feat(tables): fractional order keys for O(log n) row insert/delete (flag-gated, default off)#4890
Conversation
…uery ordering, add backfill Flag off (default) = identical behavior. Single-insert assigns a fractional order_key; queryRows orders by order_key when the flag is on; deletes skip the O(N) reindex when on. Per-table-atomic backfill script populates existing rows.
…lace, import, create, copilot) Completes the always-write-keys prerequisite: every row insert now assigns a fractional order_key consistent with position order, so the flag can be flipped safely after backfill. Flag off (default) still = identical behavior.
…-by-key Inserts express intent as afterRowId/beforeRowId (O(1) key mint via the (table_id,order_key,id) index); orderKey is returned on every row; client reconcile/undo place by orderKey (no neighbor bump) with position fallback. Flag off = unchanged. 205 table tests pass.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
… flag Position-based callers (mothership tool, v1 API, undo fallback, transient old clients) resolve their insert neighbor by order_key ordinal (OFFSET) when the flag is on — positions are gappy then, so WHERE position=N would miss. Flag off keeps the indexed position lookup. The mothership tool itself is unchanged.
Status update — insert half + verification doneAll planned work for this PR is now in:
10 commits; Activation runbook (when ready to flip)
Follow-up PRDrop the |
|
@greptile review |
Greptile SummaryThis PR replaces contiguous integer
Confidence Score: 4/5Safe to deploy with the flag off (the default); the flag-on path has a client-side sort inconsistency that would produce wrong row ordering in some locales. The flag-off path is purely additive and behaviorally identical to today. The flag-on path has a mismatch in apps/sim/hooks/queries/tables.ts — the Important Files Changed
Reviews (5): Last reviewed commit: "docs(triggers): drop rowNumber from tabl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…n + safe reconcile - resolveInsertByNeighbor throws when the anchor row is missing (was silently inserting at the front) and when its order_key is null under the flag. - insert contract: afterRowId/beforeRowId are mutually exclusive (refine). - reconcileCreatedRow only key-sorts when every cached row is keyed, so mid- backfill un-keyed rows aren't yanked to the front.
|
Thanks @greptile — all four addressed in c6b8375:
(Also fixed an unrelated |
|
All four look solid. A few notes from reviewing the commit: The The null anchor-key guard — the "flag-on throws, flag-off is best-effort" split is the right invariant. One question: when The "every cached row has orderKey" check in Everything else looks well-reasoned. The expand/contract rollout (deploy → backfill → flip → future drop) is clean. Marking the previous concerns resolved. |
- Extract maxOrderKey(executor, tableId) helper; replaces three identical max(order_key) selects (single/batch insert append + import). - Import: read the append anchor once up front and thread each batch's last key forward (nextImportStartOrderKey + afterOrderKey) instead of re-scanning max(order_key) per batch over a growing table — one scan per import, not one per 1k-row batch.
The afterRowId/beforeRowId mutual-exclusion .refine() turned the schema into a
ZodEffects, which Zod forbids .omit() on — v1's insertTableRowBodySchema.omit({
position }) threw at module load (runtime-only; tsc misses it). Split the plain
object base out, apply the shared refine on top, and have v1 omit from the base
then re-apply it.
A single UPDATE … FROM (VALUES …) over a whole large table overflows the JS call stack while drizzle assembles the VALUES list (and would blow past Postgres's 65535 bound-param limit at ~32k rows) — large tables failed with 'Maximum call stack size exceeded'. Write in 1000-row chunks inside the same per-table transaction so keying stays atomic.
|
@greptile review |
PR SummaryHigh Risk Overview Server: Inserts always persist an Client: Grid insert/duplicate uses neighbor ids instead of integer positions; React Query cache merge sorts by Triggers/docs: Reviewed by Cursor Bugbot for commit d85448d. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
The single-row and batch insert handlers dropped orderKey from the JSON response even though the service returns it, so reconcileCreatedRow always fell back to position-sorting and could place neighbor inserts wrong under the fractional-ordering flag. Serialize orderKey alongside position.
A saved position is the gappy column value, but under the flag insert reads position as a visual rank (OFFSET) — so position-based restore misplaces rows. - create-row redo now goes through the batch path carrying the saved orderKey (the single-insert API has no orderKey field); drop the now-unused single create mutation. - resolveBatchInsertOrderKeys appends under the flag instead of feeding gappy positions to resolveInsertOrderKey; positions remain the flag-off path.
5x fewer round-trips per table; ~10k bound params stays well under Postgres's 65535 ceiling and far below the single-statement size that overflows the stack.
position is gappy under the fractional-ordering flag, so rowNumber (= row.position) no longer reflects a contiguous visual rank. Rather than compute-on-read, remove it from the trigger payload, output schema, and column-execution input. Also pin isTablesFractionalOrderingEnabled=false in update-row.test.ts so its flag-off position-shift assertions are deterministic regardless of local env.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21ea14c. Configure here.
|
@greptile review |
biome check . flagged the drizzle-generated _journal.json and 0226_snapshot.json; apply the formatter so packages/db lint:check passes in CI.
rowNumber was removed from the table trigger payload; remove it from the documented output fields to match.
|
@greptile review |
TheodoreSpeaks
changed the title
Flag-on behavior is covered by manual large-table verification; the heavily- mocked DB-chain suite added little signal.
TheodoreSpeaks
changed the title
1 participant
Summary
Replaces the contiguous integer
positionordering ofuser_table_rowswith fractional string order keys so insert and delete stop the O(N) position reshift/recompact. Gated behind theTABLES_FRACTIONAL_ORDERINGenv flag (default off → identical behavior today), rolled out expand/contract.order-key.tsutil (keyBetween/nKeysBetween) overfractional-indexing; nullableorder_keycolumn +(table_id, order_key, id)index (migration0226); per-table-atomic, idempotent backfill script (chunked writes).queryRowsorders byorder_key, id; deletes skip the reindex; inserts mint a key between neighbors (afterRowId/beforeRowId, O(log n) via the index, no O(N) reshift). All 8 insert paths always write a key, so the flag can flip safely after backfill.orderKey; insert intent isafterRowId/beforeRowId; client reconcile/undo place by key (no neighbor bump), withpositionfallback for un-backfilled rows.rowNumberfrom the table-trigger output — it wasposition-based and goes gappy under the flag. Workflows referencingrowNumberlose the field.Rollout
Deploy (flag off) → run backfill → flip
TABLES_FRACTIONAL_ORDERINGon + restart (reversible). A later PR drops thepositioncolumn.Type of Change
Testing
Manual verification on a local DB with large seeded tables, flag on:
0226; ran the backfill across 16 tables incl. 500k–1M rows — per-table-atomic, idempotent re-runs, chunked writes (1M-row tables key without overflowing the JS stack / the 65535 param limit).TABLES_FRACTIONAL_ORDERING+ restarted; reads via the grid and the table-query block both order byorder_key, id.EXPLAINconfirmsORDER BY order_key, iduses the(table_id, order_key, id)index (Index Scan, no Sort) — sub-ms on 500k+ rows.max_rowscap still rejects inserts on a full table.Automated: table / copilot / route unit tests,
type-check,check:api-validation:strict, and fulllint:checkall pass.Follow-ups (separate PRs)
positioncolumn once the flag has baked on.OFFSETis O(offset); cursor on(order_key, id)makes it O(log n).Checklist