feat: add drizzle persistence plugin #122
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request introduce a persistence layer for the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jaipaljadeja
force-pushed
the
feat/drizzle-pglite
branch
from
2cb90f8 to
fc8449e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (2)
examples/cli/indexers/2-starknet.indexer.ts (2)
16-22: Ensure Proper Disposal of Database ConnectionsWhile setting up the SQLite database, ensure that database connections are properly closed when no longer needed to prevent potential resource leaks.
Consider adding code to close the database connection when the indexer stops or encounters an error.
Line range hint
43-46: Clean Up Commented-Out CodeThe
writer.insertcode block is commented out. If it's not needed, consider removing it to keep the codebase clean. If it's intended for future use, add a comment explaining its purpose.
🛑 Comments failed to post (8)
packages/indexer/build.config.ts (1)
15-15:
⚠️ Potential issuePossible Typo in Filename 'dirzzle-persistence.ts'
It appears that the filename
dirzzle-persistence.tsmay be misspelled. Considering that you're integrating with Drizzle ORM, the correct filename should likely bedrizzle-persistence.ts.Apply this diff to correct the filename:
"./src/index.ts", "./src/sinks/sqlite.ts", "./src/sinks/csv.ts", "./src/sinks/drizzle/index.ts", "./src/testing/index.ts", "./src/vcr/index.ts", "./src/plugins/index.ts", "./src/plugins/kv.ts", "./src/plugins/logger.ts", "./src/plugins/persistence.ts", - "./src/plugins/dirzzle-persistence.ts", + "./src/plugins/drizzle-persistence.ts", ],Committable suggestion skipped: line range outside the PR's diff.
examples/cli/indexers/2-starknet.indexer.ts (3)
63-87: 🛠️ Refactor suggestion
Consider Using Migrations for Database Schema Management
Directly executing SQL statements for schema creation within the code is not recommended. Using a migration system ensures consistency and easier maintenance of the database schema.
Consider implementing migrations using Drizzle ORM's migration capabilities or another migration tool to handle table creation and schema updates. This approach allows for versioning of schema changes and better collaboration among team members.
2-2:
⚠️ Potential issuePossible Typo in Import Path 'dirzzle-persistence'
The import path
@apibara/indexer/plugins/dirzzle-persistencemay contain a typo. The correct path should likely be@apibara/indexer/plugins/drizzle-persistence, reflecting the Drizzle ORM integration.Apply this diff to correct the import statement:
import { defineIndexer, useSink } from "@apibara/indexer"; -import { drizzlePersistence } from "@apibara/indexer/plugins/dirzzle-persistence"; +import { drizzlePersistence } from "@apibara/indexer/plugins/drizzle-persistence"; import { useLogger } from "@apibara/indexer/plugins/logger";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { drizzlePersistence } from "@apibara/indexer/plugins/drizzle-persistence";
24-26:
⚠️ Potential issueAvoid Hardcoding Database Credentials
The database connection string contains hardcoded credentials, which is a security risk. It's recommended to use environment variables to manage sensitive information.
Apply this diff to use environment variables for the connection string:
// Persistence Database const client = new Client({ - connectionString: "postgres://postgres:postgres@localhost:5432/postgres", + connectionString: process.env.DATABASE_URL, });Ensure that the
DATABASE_URLenvironment variable is properly set in your environment.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const client = new Client({ connectionString: process.env.DATABASE_URL, });packages/indexer/package.json (1)
72-77:
⚠️ Potential issueFix Typo in Export Path for
drizzle-persistencePluginThere's a typo in the export path
"./plugins/dirzzle-persistence"; it should be"./plugins/drizzle-persistence"to correctly reference the plugin and maintain consistency.Apply this diff to fix the typo:
}, - "./plugins/dirzzle-persistence": { + "./plugins/drizzle-persistence": { "types": "./dist/plugins/drizzle-persistence.d.ts", "import": "./dist/plugins/drizzle-persistence.mjs", "require": "./dist/plugins/drizzle-persistence.cjs", "default": "./dist/plugins/drizzle-persistence.mjs" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.}, "./plugins/drizzle-persistence": { "types": "./dist/plugins/drizzle-persistence.d.ts", "import": "./dist/plugins/drizzle-persistence.mjs", "require": "./dist/plugins/drizzle-persistence.cjs", "default": "./dist/plugins/drizzle-persistence.mjs"packages/indexer/src/plugins/dirzzle-persistence.ts (3)
1-207:
⚠️ Potential issueCorrect Typo in Filename and Ensure Consistency
The file is named
dirzzle-persistence.ts, but the exported function isdrizzlePersistence(line 42). This inconsistency can lead to confusion and import errors. Please rename the file todrizzle-persistence.tsto match the function name and update any related import statements and export paths accordingly.
22-22:
⚠️ Potential issueUse
bigintData Type fororderKeyto Prevent Precision Loss
- Table Definition (Line 22):
orderKeyis defined asinteger("order_key").notNull(). SinceorderKeyrepresents aBigInt, usingintegermay cause overflow issues for large values.- Data Insertion (Lines 147-148, 153-154): Converting
cursor.orderKey(aBigInt) toNumbermay result in precision loss.Recommend using
bigintin your table definition and storingorderKeyasstringorBigInt.Apply these diffs:
Update Table Definition:
export const checkpoints = pgTable("checkpoints", { id: text("id").notNull().primaryKey(), - orderKey: integer("order_key").notNull(), + orderKey: bigint("order_key").notNull(), uniqueKey: text("unique_key") .$type<`0x${string}` | undefined>() .notNull() .default(undefined), });Update
_putCheckpointMethod:.values({ id: this._indexerName, - orderKey: Number(cursor.orderKey), + orderKey: cursor.orderKey, uniqueKey: cursor.uniqueKey, }) .onConflictDoUpdate({ target: checkpoints.id, set: { - orderKey: Number(cursor.orderKey), + orderKey: cursor.orderKey, uniqueKey: cursor.uniqueKey, }, });Update
_getCheckpointMethod (Lines 137-139):No change needed if
orderKeyis retrieved directly asBigInt.Also applies to: 147-148, 153-154
184-184:
⚠️ Potential issueUse Consistent Data Types for
fromBlockandtoBlockinfiltersTable
- Table Definition (Lines 34-35):
fromBlockandtoBlockare declared asinteger. Since block numbers can be large, consider usingbigintto accommodate larger values.- Data Handling (Lines 184, 193, 199): Converting
endCursor.orderKeytoNumbermay lead to precision loss.Apply these diffs:
Update Table Definition:
filter: text("filter").notNull(), - fromBlock: integer("from_block").notNull(), - toBlock: integer("to_block"), + fromBlock: bigint("from_block").notNull(), + toBlock: bigint("to_block"), },Update
_putFilterMethod:// Update existing filter's to_block await this._db .update(filters) - .set({ toBlock: Number(endCursor.orderKey) }) + .set({ toBlock: endCursor.orderKey }) .where(and(eq(filters.id, this._indexerName), isNull(filters.toBlock))); // Insert new filter await this._db .insert(filters) .values({ id: this._indexerName, filter: serialize(filter as Record<string, unknown>), - fromBlock: Number(endCursor.orderKey), + fromBlock: endCursor.orderKey, }) .onConflictDoUpdate({ target: [filters.id, filters.fromBlock], set: { filter: serialize(filter as Record<string, unknown>), - fromBlock: Number(endCursor.orderKey), + fromBlock: endCursor.orderKey, }, });Also applies to: 193-193, 199-199
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
examples/cli/indexers/2-starknet.indexer.ts (2)
35-40: Consider adding type safety to plugin configurationThe plugin configuration is functional but could benefit from explicit type definitions.
Consider adding type safety:
plugins: [ drizzlePersistence({ database: persistDatabase, - indexerName: "2-starknet", + indexerName: "2-starknet" as const, }), ],
66-71: TODO Comments Indicate Incomplete ImplementationThe comments suggest that table definitions should be imported from the plugin package instead of being created directly with SQL.
Would you like me to help implement proper schema imports and migrations setup?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
change/@apibara-indexer-3ab2f0f6-4d43-4499-a0a3-55188fe8eb53.json(1 hunks)examples/cli/indexers/2-starknet.indexer.ts(2 hunks)examples/cli/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- change/@apibara-indexer-3ab2f0f6-4d43-4499-a0a3-55188fe8eb53.json
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/cli/package.json
🔇 Additional comments (2)
examples/cli/indexers/2-starknet.indexer.ts (2)
2-2: LGTM: Import statements are properly structured
The new imports for Drizzle ORM and PostgreSQL client are correctly added and align with the PR's objective of adding persistence functionality.
Also applies to: 8-10
Line range hint 1-91: Verify Plugin Integration
Let's verify that this implementation matches the expected usage of the drizzle persistence plugin.
✅ Verification successful
Based on the plugin implementation from the source code, I can now generate the final response:
The drizzle persistence plugin implementation is correct
The implementation matches the expected usage pattern of the drizzle persistence plugin:
-
Proper plugin configuration with required parameters:
database: Correctly passed as a drizzle PgDatabase instanceindexerName: Optional parameter with default value "default"
-
Correct table structure creation in the
run:beforehook:checkpointstable with required fields (id, order_key, unique_key)filterstable with required fields (id, filter, from_block, to_block)
-
Proper connection handling:
- Database client connection established in
run:beforehook - Tables created with correct schema matching the plugin's exported table definitions
- Database client connection established in
The implementation follows all the plugin's requirements and best practices for persistence management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other implementations or tests of the drizzle persistence plugin
# to ensure consistent usage patterns
# Look for other files using the drizzle persistence plugin
rg -l "drizzlePersistence" --type ts
# Look for test files related to the plugin
fd -e test.ts -e spec.ts | xargs rg "drizzlePersistence"
# Look for documentation or example usage
fd README.md | xargs rg -l "drizzlePersistence"
Length of output: 237
Script:
#!/bin/bash
# Let's examine the plugin implementation to understand the expected usage
cat packages/indexer/src/plugins/dirzzle-persistence.ts
# Also look for any related configuration types or interfaces
ast-grep --pattern 'interface $_PersistenceConfig {
$$$
}'
# Search for any exports from the plugin file
ast-grep --pattern 'export const $_ = $_'
Length of output: 100794
| // Persistence Database | ||
| const client = new Client({ | ||
| connectionString: "postgres://postgres:postgres@localhost:5432/postgres", | ||
| }); | ||
| const persistDatabase = drizzle(client); |
There was a problem hiding this comment.
Security & Configuration Issues in Database Setup
Several critical issues need to be addressed:
- Hardcoded database credentials in the connection string pose a security risk
- No error handling for database connection failures
- Missing configuration options for different environments (dev/staging/prod)
Consider implementing the following changes:
- const client = new Client({
- connectionString: "postgres://postgres:postgres@localhost:5432/postgres",
- });
+ const client = new Client({
+ connectionString: runtimeConfig.persistenceConnectionString ??
+ process.env.PERSISTENCE_DB_URL ??
+ throw new Error("Database connection string not configured"),
+ });Also, consider adding error handling:
try {
await client.connect();
} catch (error) {
logger.error("Failed to connect to persistence database:", error);
throw error;
}| await persistDatabase.execute(sql` | ||
| CREATE TABLE IF NOT EXISTS checkpoints ( | ||
| id TEXT NOT NULL PRIMARY KEY, | ||
| order_key INTEGER NOT NULL, | ||
| unique_key TEXT | ||
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS filters ( | ||
| id TEXT NOT NULL, | ||
| filter TEXT NOT NULL, | ||
| from_block INTEGER NOT NULL, | ||
| to_block INTEGER, | ||
| PRIMARY KEY (id, from_block) | ||
| ); | ||
| `); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve Database Schema Management
Several improvements are recommended for the schema management:
- Use Drizzle's schema definitions instead of raw SQL
- Implement proper migrations
- Add indexes for better query performance
Consider refactoring to use Drizzle's schema definitions:
import { pgTable, text, integer } from 'drizzle-orm/pg-core';
export const checkpoints = pgTable('checkpoints', {
id: text('id').primaryKey(),
orderKey: integer('order_key').notNull(),
uniqueKey: text('unique_key')
});
export const filters = pgTable('filters', {
id: text('id').notNull(),
filter: text('filter').notNull(),
fromBlock: integer('from_block').notNull(),
toBlock: integer('to_block'),
// Add composite primary key
}, (table) => ({
pk: primaryKey(table.id, table.fromBlock)
}));| hooks: { | ||
| async "run:before"() { | ||
| await client.connect(); | ||
|
|
||
| // Normally user will do migrations of both tables, which are defined in | ||
| // ``` | ||
| // import { checkpoints, filters } from "@apibara/indexer/plugins/dirzzle-persistence" | ||
| // ```, | ||
| // but just for quick testing and example we create them here directly | ||
|
|
||
| await persistDatabase.execute(sql` | ||
| CREATE TABLE IF NOT EXISTS checkpoints ( | ||
| id TEXT NOT NULL PRIMARY KEY, | ||
| order_key INTEGER NOT NULL, | ||
| unique_key TEXT | ||
| ); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS filters ( | ||
| id TEXT NOT NULL, | ||
| filter TEXT NOT NULL, | ||
| from_block INTEGER NOT NULL, | ||
| to_block INTEGER, | ||
| PRIMARY KEY (id, from_block) | ||
| ); | ||
| `); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add Cleanup Hook
Consider adding a cleanup hook to properly close the database connection.
Add a run:after hook:
hooks: {
async "run:before"() {
await client.connect();
// ... table creation
},
+ async "run:after"() {
+ try {
+ await client.end();
+ } catch (error) {
+ logger.error("Error closing database connection:", error);
+ }
+ },
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hooks: { | |
| async "run:before"() { | |
| await client.connect(); | |
| // Normally user will do migrations of both tables, which are defined in | |
| // ``` | |
| // import { checkpoints, filters } from "@apibara/indexer/plugins/dirzzle-persistence" | |
| // ```, | |
| // but just for quick testing and example we create them here directly | |
| await persistDatabase.execute(sql` | |
| CREATE TABLE IF NOT EXISTS checkpoints ( | |
| id TEXT NOT NULL PRIMARY KEY, | |
| order_key INTEGER NOT NULL, | |
| unique_key TEXT | |
| ); | |
| CREATE TABLE IF NOT EXISTS filters ( | |
| id TEXT NOT NULL, | |
| filter TEXT NOT NULL, | |
| from_block INTEGER NOT NULL, | |
| to_block INTEGER, | |
| PRIMARY KEY (id, from_block) | |
| ); | |
| `); | |
| }, | |
| }, | |
| hooks: { | |
| async "run:before"() { | |
| await client.connect(); | |
| // Normally user will do migrations of both tables, which are defined in |
| public async del() { | ||
| await this._delCheckpoint(); | ||
| await this._delFilter(); | ||
| } |
There was a problem hiding this comment.
Not used so I think it can go away.
| private async _delCheckpoint() { | ||
| await this._db | ||
| .delete(checkpoints) | ||
| .where(eq(checkpoints.id, this._indexerName)); | ||
| } |
| private async _delFilter() { | ||
| await this._db.delete(filters).where(eq(filters.id, this._indexerName)); | ||
| } |
packages/indexer/package.json
Outdated
| @@ -69,6 +69,12 @@ | |||
| "import": "./dist/plugins/persistence.mjs", | |||
| "require": "./dist/plugins/persistence.cjs", | |||
| "default": "./dist/plugins/persistence.mjs" | |||
| }, | |||
| "./plugins/dirzzle-persistence": { | |||
There was a problem hiding this comment.
| "./plugins/dirzzle-persistence": { | |
| "./plugins/drizzle-persistence": { |
packages/indexer/package.json
Outdated
| "types": "./dist/plugins/dirzzle-persistence.d.ts", | ||
| "import": "./dist/plugins/dirzzle-persistence.mjs", | ||
| "require": "./dist/plugins/dirzzle-persistence.cjs", | ||
| "default": "./dist/plugins/dirzzle-persistence.mjs" |
There was a problem hiding this comment.
| "types": "./dist/plugins/dirzzle-persistence.d.ts", | |
| "import": "./dist/plugins/dirzzle-persistence.mjs", | |
| "require": "./dist/plugins/dirzzle-persistence.cjs", | |
| "default": "./dist/plugins/dirzzle-persistence.mjs" | |
| "types": "./dist/plugins/drizzle-persistence.d.ts", | |
| "import": "./dist/plugins/drizzle-persistence.mjs", | |
| "require": "./dist/plugins/drizzle-persistence.cjs", | |
| "default": "./dist/plugins/drizzle-persistence.mjs" |
There was a problem hiding this comment.
Should be drizzle-persistence.ts
| @@ -1,26 +1,43 @@ | |||
| import { defineIndexer, useSink } from "@apibara/indexer"; | |||
| import { drizzlePersistence } from "@apibara/indexer/plugins/dirzzle-persistence"; | |||
There was a problem hiding this comment.
| import { drizzlePersistence } from "@apibara/indexer/plugins/dirzzle-persistence"; | |
| import { drizzlePersistence } from "@apibara/indexer/plugins/drizzle-persistence"; |
|
|
||
| // Normally user will do migrations of both tables, which are defined in | ||
| // ``` | ||
| // import { checkpoints, filters } from "@apibara/indexer/plugins/dirzzle-persistence" |
There was a problem hiding this comment.
| // import { checkpoints, filters } from "@apibara/indexer/plugins/dirzzle-persistence" | |
| // import { checkpoints, filters } from "@apibara/indexer/plugins/drizzle-persistence" |
packages/indexer/build.config.ts
Outdated
| @@ -12,6 +12,7 @@ export default defineBuildConfig({ | |||
| "./src/plugins/kv.ts", | |||
| "./src/plugins/logger.ts", | |||
| "./src/plugins/persistence.ts", | |||
| "./src/plugins/dirzzle-persistence.ts", | |||
There was a problem hiding this comment.
| "./src/plugins/dirzzle-persistence.ts", | |
| "./src/plugins/drizzle-persistence.ts", |
jaipaljadeja
force-pushed
the
feat/drizzle-pglite
branch
from
fc8449e to
76f1e94
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
examples/cli/indexers/2-starknet.indexer.ts (1)
16-17: Consider Consistency in Database UsageThe indexer is using both SQLite (for the sink database) and PostgreSQL (for persistence). Evaluate whether it's necessary to use two different database systems. Consolidating to a single database could simplify the architecture and reduce overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
change/@apibara-indexer-3ab2f0f6-4d43-4499-a0a3-55188fe8eb53.json(1 hunks)examples/cli/indexers/2-starknet.indexer.ts(2 hunks)examples/cli/package.json(2 hunks)packages/indexer/build.config.ts(1 hunks)packages/indexer/package.json(3 hunks)packages/indexer/src/plugins/drizzle-persistence.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/indexer/build.config.ts
- packages/indexer/package.json
- examples/cli/package.json
- change/@apibara-indexer-3ab2f0f6-4d43-4499-a0a3-55188fe8eb53.json
🔇 Additional comments (7)
examples/cli/indexers/2-starknet.indexer.ts (6)
2-2: Import Statement Added Correctly
The drizzlePersistence plugin is correctly imported from @apibara/indexer/plugins/drizzle-persistence.
88-88: Suggestion: Add a Cleanup Hook to Close the Database Connection
Consider adding a run:after hook to properly close the database connection when the indexer stops running. This ensures that resources are released properly and prevents potential connection leaks.
Apply this diff to add the cleanup hook:
hooks: {
async "run:before"() {
// ...existing code...
},
+ async "run:after"() {
+ try {
+ await client.end();
+ } catch (error) {
+ const logger = useLogger();
+ logger.error("Error closing database connection:", error);
+ }
+ },
},35-40: Integration of drizzlePersistence Plugin
The drizzlePersistence plugin is correctly integrated into the indexer configuration. This enables efficient persistence of checkpoint and filter data using Drizzle ORM and PostgreSQL.
24-26:
Security Risk: Hardcoded Database Credentials
The hardcoded database connection string in the Client constructor poses a security risk and lacks flexibility across different environments (development, staging, production). It's recommended to use environment variables or configuration files to manage database credentials securely.
Apply this diff to address the issue:
- const client = new Client({
- connectionString: "postgres://postgres:postgres@localhost:5432/postgres",
- });
+ const client = new Client({
+ connectionString: runtimeConfig.persistenceConnectionString ??
+ process.env.PERSISTENCE_DB_URL ??
+ throw new Error("Database connection string not configured"),
+ });64-64:
Lack of Error Handling for Database Connection
The call to await client.connect(); lacks error handling. If the database connection fails, it could lead to unhandled exceptions and application crashes. Wrap the connection logic in a try-catch block to handle potential errors gracefully.
Apply this diff to add error handling:
async "run:before"() {
+ try {
await client.connect();
+ } catch (error) {
+ const logger = useLogger();
+ logger.error("Failed to connect to persistence database:", error);
+ throw error;
+ }
// Normally user will do migrations of both tables...72-86: 🛠️ Refactor suggestion
Refactor Suggestion: Use Drizzle's Schema Definitions Instead of Raw SQL
Using raw SQL statements to create tables is less maintainable and bypasses the benefits of using an ORM. Consider using Drizzle ORM's schema definitions and migration system to manage your database schema. This approach enhances maintainability and leverages Drizzle's features.
Here's how you can define your tables using Drizzle:
import { checkpoints, filters } from "@apibara/indexer/plugins/drizzle-persistence";
// Use Drizzle's migration system to create tables
await persistDatabase.runMigrations();Ensure you have proper migration files set up according to Drizzle's documentation.
packages/indexer/src/plugins/drizzle-persistence.ts (1)
1-192: LGTM: Implementation of drizzlePersistence Plugin
The drizzlePersistence plugin is well-implemented, providing a robust persistence layer using Drizzle ORM. The definitions of the checkpoints and filters tables are appropriate, and the plugin correctly integrates with the indexer's lifecycle hooks. The methods for managing checkpoints and filters are clearly defined and utilize Drizzle's capabilities effectively.
| .$type<`0x${string}` | undefined>() | ||
| .notNull() | ||
| .default(undefined), |
There was a problem hiding this comment.
Inconsistency in Column Definition: uniqueKey
The uniqueKey column in the checkpoints table is defined with .notNull().default(undefined). Setting a default value of undefined for a NOT NULL column may not behave as intended. In SQL, undefined is not a valid default value.
Consider adjusting the column definition to allow uniqueKey to be nullable if appropriate:
- .$type<`0x${string}` | undefined>()
- .notNull()
- .default(undefined),
+ .$type<`0x${string}` | null>()
+ .default(null),Alternatively, if uniqueKey should always have a value, provide a valid default or ensure it's populated correctly when inserting records.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .$type<`0x${string}` | undefined>() | |
| .notNull() | |
| .default(undefined), | |
| .$type<`0x${string}` | null>() | |
| .default(null), |
|
Looks good now! |
No description provided.