fix(query,realtime): a live query no longer ships the raw table row, or mis-orders a projected window - #235
Conversation
…or mis-orders a projected window Fixes #230 — two defects with one cause, found by making `examples/dummy`'s live suite run. A `ChangeEvent` carries the whole TABLE row: that is what logical replication emits, and what `setRowObserver` emits. A live query's result set is whatever its `sql` returned. Nothing bridged the two. ## The leak Every patch forwarded the change row unnarrowed, so a column the projection dropped went out on the socket the moment it CHANGED. `examples/dummy`'s feed projects ten columns and one publish delivered `updatedAt`; a salary or a private note would have gone the same way. The per-subscriber gate cannot help — it decides whether a ROW is delivered, never which of its columns. `narrowRow` restricts a patch row to the columns the query actually returned. `id` always survives: it is the row's identity on the wire. An unknown projection narrows nothing, because "nothing read yet" is not "the result set has no columns". The projection is LEARNED from the query's own reads (`live-definition.ts`) rather than declared — it lives inside the `sql` provider's closure and there is nothing static to read it from. Learned and kept, because the case the window's own rows cannot answer is an EMPTY window: the first row to arrive would otherwise go out whole. ## The mis-ordering `match()` decides position against the rows the WINDOW holds, so an `orderBy` on a column the projection omits measured a real value against nothing — never equal on the update path, so every change read as a move; arbitrary on the INSERT path, so a row created last landed wherever `undefined` sorted. That second half was not in the issue: this suite's own test found it. A position the window cannot answer for is now `refill` — one re-read and a re-snapshot — rather than a guess. A DELETE never reaches the rule: it is addressed by the index its id was found at, so a projected query still removes rows incrementally. The discriminator is `Object.hasOwn`, never a value check. A nullable column that IS null still carries its key, and everywhere else in this package an absent key and a SQL NULL are one absence. Here they are different facts, and only the key tells them apart. ## The reference app `PostSummary` now carries `createdAt`, the key `liveFeed` orders by. The rule generalises what `assertSeekable` already applies to a cursor — a sort key has to be readable on the row — and it puts the feed back on the incremental path: `a publish arrives as one incremental patch, not a refetch` is the assertion that file was written with, and it is true for the first time. The digest's hand-built fixture row carries it too, because `digestEmail` parses that shape. ## Verification - Both halves mutation-checked: removing the narrowing reds the leak guard in `examples/dummy` AND in `@ultimat3/testing`'s end-to-end node test; the ordering rule is covered by six cases in `matcher.test.ts`. - The end-to-end leak guard had to be rewritten once: an update patch carries only CHANGED columns, so a non-projected column left alone is absent whether or not anything narrows. It now moves. - `bun run verify` 14/18 (4 skipped); `reference-app-gate` every pin holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 seconds Limit details: You’ve used the included review currently available. Your 76 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (14)
Comment |
1 participant
Fixes #230 — two defects with one cause.
A
ChangeEventcarries the whole table row: that is what logical replication emits, and whatsetRowObserveremits. A live query's result set is whatever itssqlreturned. Nothing bridged the two.1. The leak
Every patch forwarded the change row unnarrowed, so a column the projection dropped went out on the socket the moment it changed.
examples/dummy's feed projects ten columns and one publish deliveredupdatedAt; a salary, a private note or a hashed token would have gone the same way. The per-subscribervisiblegate cannot help — it decides whether a row is delivered, never which of its columns.narrowRowrestricts a patch row to the columns the query actually returned.idalways survives — it is the row's identity on the wire, andapplyToWindowand every client store key by it.sqlprovider's closure and there is nothing static to read it from. Learned and kept, because the case the window's own rows cannot answer is an empty window — the first row to arrive would otherwise go out whole.2. The mis-ordering
match()decides position against the rows the window holds, so anorderByon a column the projection omits measured a real value against nothing.remove+insertpositionForcompared against rows that cannot answer → a row created last landed at position 0That second half was not in the issue. I wrote a test asserting the insert path was unaffected, and it failed —
positionFor's own doc says a wrong position is "an order the database would never return", and that is what it was returning.A position the window cannot answer for is now
refill— one re-read and a re-snapshot, machinery the fanout already has — rather than a guess. A delete never reaches the rule: it is addressed by the index its id was found at, so a projected query still removes rows incrementally.The discriminator is
Object.hasOwn, never a value check. A nullable column that is null still carries its key, and everywhere else in@ultimat3/queryan absent key and a SQL NULL are one absence (isNullsays so). Here they are different facts — "this row's value is nothing" versus "this shape cannot answer" — and only the key tells them apart.3. The reference app
PostSummarynow carriescreatedAt, the keyliveFeedorders by. The rule generalises whatassertSeekablealready applies to a cursor — a sort key has to be readable on the row — and it is the honest shape besides: a client handed a feed sorted by creation time can neither re-sort nor resume without the value it was sorted on.That puts the feed back on the incremental path, so
a publish arrives as one incremental patch, not a refetch— the assertion that file was written with, which had never been true — passes.What I got wrong on the way, since it changed the tests
My first end-to-end leak guard could not fail. An update patch carries only the columns that changed, so a non-projected column left alone is absent whether or not anything narrows. Mutation-checking caught it; the write now moves
secretas well astitle.Verification
examples/dummyand the end-to-end node test in@ultimat3/testing; the ordering rule has six cases inmatcher.test.ts, including that a delete still patches and that a NULL value is answerable.bun run verify— 14 of 18, 4 skipped at the framework root.bun run scripts/reference-app-gate.ts— every pin holds, both apps.Cost, stated plainly
A live query whose rows omit their sort key now re-reads instead of patching. That is correct and slower, and the fix is to project the key. It is not silent:
wiki/Known-Gaps.md, both packageCLAUDE.mds and the CHANGELOG all say so, and the reference app is the worked example.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.