fix(tables): match unique JSON values exactly and validate row writes against the live schema - #8394
Conversation
… against the live schema
- Unique checks and the upsert conflict probe matched JSON objects and arrays by containment, so
`{"a":1}` counted as a duplicate of `{"a":1,"b":2}` and upsert could overwrite a row that only
contained its target. They now also require exact jsonb equality, keeping containment as the
GIN-indexed leading clause; JSON values take per-value locks like other types
- Row writes validated against a schema snapshot read before their transaction, so a concurrent
make-unique, make-required, retype, or column delete could commit violating data. Each write
now reads the live schema under the table's schema lock (shared) through
`user_table_schema_for_write`, in the statement it already runs first, and validates against it
- The background update runner derives each batch's patch from the raw payload against the live
schema, through the same helper as the inline bulk update, and refuses unique and required-null
patches under the lock
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
…w input - user_table_schema_for_write moves from Drizzle migration 0391 to script migration 0026, which db:push runs too. A db:push database (local dev, the CI push provision) never applied 0391, so every guarded row write failed there. The journal ends at 0390 again; the function body and its comments are unchanged. - A writer that finds the schema moved rebuilds the row from the caller's raw input instead of the value it coerced against its snapshot, so a "007" sent while a column changed from number to text is stored as "007", not "7". This covers insert, upsert, update, batch update and the import batch. - The refit re-checks the row's size, which a coercion to a wider type can grow past the limit after the pre-lock check passed.
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 24 files
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
- The background update runner re-reads a batch's rows inside the batch transaction and re-validates them merged with the re-derived patch when the schema moved since the page was checked, as the inline bulk update does; an unchanged schema adds no query. updatePageByIds takes an async per-batch hook with the transaction for this. - batchInsertRowsWithTx returns the definition it validated against, and batchInsertRows dispatches its insert triggers with it. - Row cells and patch keys are read as own properties (Object.hasOwn), so a legacy column keyed by a prototype name such as `constructor` is not seen in rows or patches that do not hold it. - The long schema-wait test asserts the write is waiting on the schema lock before the holder commits.
…on, replace dedupe, and the upsert probe
|
@cubic-dev-ai review this PR |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 24 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
This branch was previously deployed
1 participant
Summary
Unique JSON columns matched by containment. The write-time unique checks and the upsert conflict probe used
data @> {col: v}. For objects and arrays that isn't equality:{"a":1}was rejected because a row held{"a":1,"b":2}, and[1]because a row held[1,2,3]A new
uniqueValuePredicatekeeps containment as the GIN-indexed leading clause and addsdata -> $key = $v::jsonbfor objects and arrays. It's used by the single and batch unique checks and the upsert probe. In-batch duplicate keys useuniqueValueKey, which ignores key order and keeps array order, the same as jsonb equality. JSON values now take per-value locks like other types, so the exclusive table-lock fallback is gone.Row writes validated against a stale schema. Writers validated against a definition read before their transaction. A concurrent make-unique, make-required, retype, or column delete could therefore commit violating data (a duplicate in a unique column, an empty required cell, a stale-typed value, a cell in a deleted column).
0026_user_table_schema_for_writeinstallsuser_table_schema_for_write(table_id). It takes the table'suser_table_schemaadvisory lock shared, the keywithLockedTableholds exclusively, then returns the live schema. It'sVOLATILE, so the read sees everything committed before the lock was granted. It's a script migration rather than a Drizzle SQL migration because it has to exist under bothdb:migrateanddb:push, the same pattern as script migration 0025.statement_timeout, not the 3 slock_timeout, so a write behind a long schema change waits it out instead of failing. Writes stay parallel with each other.withLockedTable/guardBatchdon't take it twice. Deletes and executions-only writes don't depend on the schema.Background bulk update derives each batch's patch from the live schema. It uses the same helper as the inline bulk update (
deriveBulkUpdatePatch), under the schema lock. A column made unique or required mid-job is refused without retrying, a retyped column is coerced to its new type, and a deleted column is dropped. A retry now does exactly what the first attempt would.Behaviour changes:
updateRowis unchanged apart from the guardType of Change
Testing
row-writes.integration.tscovers:Each guard was reverted on its own and its tests went red. Making the function
STABLEfails the in-flight test.The lock-waiter helper is now scoped to the test's table. It had counted waiters database-wide, which flaked under parallel runs. Stress: 4 parallel instances, 0 failures.
A/B against staging on fresh databases, real service functions, alternating runs:
updateRowRaw-input refit and the size re-check are covered on insert, upsert, update, batch update, and the import batch.
Verified on both provisions CI uses:
db:migrateanddb:push. Ondb:push, therows_versiontests and the TTL-cleanup file skip as before; they need migration-only triggers.Type-check, lint,
check:audits, andcheck:migrations origin/stagingpass. Table unit suites: 1212 tests. Table integration suites pass.Follow-up
Checklist