Refactor database tracking and safety proxy architecture#15
Conversation
Add SQL parsing for granular table and WHERE ID tracking. Replace separate mutation/select proxies with unified builder proxy. Extract row-level tracking to support future invalidation by specific IDs. - New `createSafetyProxy` with centralized guard enforcement - New `createBuilderProxy` unifying select/insert/update/delete handling - `parseSqlTracking` for AST-based table and WHERE clause extraction - `createTrackedRawDb` to track raw SQL execution - `buildPkMap` to collect primary key info from schema - New `tracking.ts` module for cascading writes and row ID recording - Remove `createTrackedDb`, `createSelectProxy`, `createMutationProxy`, `createQueryProxy`, and `checkResultWarnings` - Updated README to clarify raw SQL automatic tracking
Greptile SummaryThis PR replaces the split
Confidence Score: 4/5The core refactor is functionally correct and well-tested, but the relation-name-vs-table-name subscription gap in trackWithRelations means sessions subscribing through nested with options whose relation key differs from the physical table name will silently miss invalidations. Cascade isolation, the .withoutWhere() escape hatch, addListener deduplication, and Drizzle SQL template handling are all correctly addressed and verified by a comprehensive integration test suite. The remaining gap is trackWithRelations using relation property names instead of physical table names, causing with: { user: true } to hash 'user' rather than 'users' and miss invalidations when the users table is mutated. packages/server/src/tools/createSafetyProxy.ts (trackWithRelations) and packages/server/src/tools/createBuilderProxy.ts (isBuilder predicate) Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const config = getTableConfig(table as any); | ||
| const pkCols = config.columns.filter((c: any) => c.primary).map((c: any) => c.name); |
There was a problem hiding this comment.
c.primary may not be the correct Drizzle column property
config.columns.filter((c) => c.primary) uses the .primary field to detect primary-key columns. In the Drizzle ORM SQLite adapter the canonical property exposed by getTableConfig is c.primaryKey (a boolean), not c.primary. If the property name is wrong, every pkCols array will be empty and pkMap will be populated but useless for future consumers. Worth verifying against the installed Drizzle version before pkMap is wired into invalidation logic.
There was a problem hiding this comment.
Proof that .primary is correct and .primaryKey is wrong:
- Constructor source (drizzle-orm/column.cjs:36):
this.primary = config.primaryKey;
The constructor maps config.primaryKey (config-time property) to this.primary (instance property). - Runtime verification:
const config = getTableConfig(users);
for (const col of config.columns) {
console.log(col.name, '| primary:', col.primary, '| primaryKey:', col.primaryKey);
}
Output:
id | primary: true | primaryKey: undefined
name | primary: false | primaryKey: undefined
- col.primary → true/false (correct, works)
- col.primaryKey → undefined always (wrong, broken)
Greptile confused the config-time property name (primaryKey in the builder API: integer("id").primaryKey()) with the runtime instance property name (primary on the column object returned by getTableConfig). The code correctly uses the runtime property .primary.
| const { sql, params } = builder.toSQL(); | ||
| const parsed = parseSqlTracking(sql, params); | ||
| for (const t of parsed.tablesRead) addListener(t, ctx); | ||
| for (const t of parsed.tablesWritten) cascadeWrite(t, ctx.tablesWritten, ctx.cascadeGraph); |
There was a problem hiding this comment.
The old code only applied the cascade graph to DELETE: createInsertProxy passed new Map() as the cascade graph, and createUpdateBuilderProxy also passed new Map(). Only createMutationProxy for "delete" received the live cascadeGraph. trackExec now calls cascadeWrite(t, ctx.tablesWritten, ctx.cascadeGraph) for every table in parsed.tablesWritten, which includes INSERT and UPDATE operations.
Concrete failure: with cascadeGraph.get("users") = Set{posts} (because posts has onDelete: "cascade"), calling db.insert(users).values({...}) marks posts as written, so every subscriber of posts receives an invalidation even though no posts row was touched. The deleted integration tests (proxy.integration.test.ts) explicitly asserted expect(tablesWritten.has("posts")).toBe(false) for both insert and update, and those assertions would now fail.
Only cascade writes on DELETE operations; INSERT and UPDATE only mark the directly affected table.
The `.withoutWhere()` call was being invoked twice — once before passing to `wrap()` and implicitly again when the proxy intercepts it. Pass the unwrapped builder instead and let the proxy handle method interception normally. Also add `inArray` import and `posts` to pkMap in test setup, and improve row ID tracking to infer table from single-table writes and filter by primary key columns only.
1 participant
Add SQL parsing for granular table and WHERE ID tracking. Replace separate mutation/select proxies with unified builder proxy. Extract row-level tracking to support future invalidation by specific IDs.
createSafetyProxywith centralized guard enforcementcreateBuilderProxyunifying select/insert/update/delete handlingparseSqlTrackingfor AST-based table and WHERE clause extractioncreateTrackedRawDbto track raw SQL executionbuildPkMapto collect primary key info from schematracking.tsmodule for cascading writes and row ID recordingcreateTrackedDb,createSelectProxy,createMutationProxy,createQueryProxy, andcheckResultWarnings