Skip to content

fix: fan UPDATE rows out to covering projections - #873

Merged
OffgridwithJD merged 2 commits into
mainfrom
audit/projection-update-fanout
Sep 2, 2026
Merged

fix: fan UPDATE rows out to covering projections#873
OffgridwithJD merged 2 commits into
mainfrom
audit/projection-update-fanout

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Summary

  • A projection stores the base row number and filters with the base delete vector, so DELETE needs no rewrite. UPDATE is delete-old plus insert-new, and the insert half did not fan the new number out.
  • After UPDATE the projection still held the deleted number. read_projection returned no rows, and a covering projection scan (SELECT of projected columns with a sort-key qual) answered as if the updated rows were gone.
  • tuple_update now calls the same fan-out as insert.

Test coverage

  • test/projection_update.sh
  • test/projections.sh (unchanged, still passes)

Made with Cursor

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Premise and fix both verified — and the blast radius is wider than the PR says

A/B on pg18a against 8b39053. 20,000 rows, projection pc on (a,c) sorted by
c, then UPDATE ... WHERE c BETWEEN 100 AND 120. Heap mirror as the oracle; the
correct answer for the window is 2020 (c = (g*7)%1000 is a bijection per
1000-row block, so 101 values x 20).

                              main      #873
window rows after UPDATE      1600      2020      (heap: 2020)
after 3 further UPDATEs       1600      2020
multiset hash == heap         NO        YES

420 rows vanish. The fix restores them, and repeated updates of the same rows do
not inflate the count — which was the thing I most wanted to check about adding a
fan-out, since the projection now holds the old number and the new one.

The part I would change in the summary

The PR describes this as a covering projection scan answering wrongly. It is
broader than that. Measured on main, same table, after the same UPDATE:

no projection on the table at all      window = 2020    correct
projection present, SELECT a, c        window = 1600    covered by pc
projection present, SELECT a, b, c     window = 1600    NOT covered by pc
projection present, WHERE b LIKE '%!'  420 rows         the updated rows ARE there

SELECT a, b, c cannot be answered from pcb is not in it, and
EXPLAIN shows no Columnar Projection: pc — yet it loses the same 420 rows. So
it is not only the covering-scan output path: once a projection exists on c,
any qual on c loses updated rows
, whatever the select list. A user doing
SELECT * FROM t WHERE c = ... gets wrong answers too.

The control matters as much: with no projection on the table, the same UPDATE and
the same query return 2020. The defect is introduced by having a projection, not
by UPDATE alone.

I am not going to assert the mechanism from here — the obvious reading is that the
projection is consulted to resolve qualifying row numbers even when it cannot
cover the output, so the missing fan-out loses them for both shapes. You are better
placed to confirm that than I am. Either way, #873 fixes both shapes: on your
branch all three counts above are 2020.

What to add

test/projection_update.sh asserts the covered shape only (SELECT a, c). One
more arm pins the shape a user is far more likely to write:

check "a non-covering select with a projected qual also keeps the updated rows" \
    "$(pgc_set_hash "SELECT a, b, c FROM pu WHERE c BETWEEN 100 AND 200")" \
    "$(pgc_set_hash "SELECT a, b, c FROM pu_h WHERE c BETWEEN 100 AND 200")"

Without it the suite pins the narrower claim the summary makes, not the wider one
the code actually fixes — and a future regression that restored the bug for
non-covering shapes only would pass.

No docs

Four files, none of them docs/ or CHANGELOG.md. Same note as #869, #871 and
#872. This one turns wrong results into right ones on a shipped feature and
belongs in the CHANGELOG at minimum.

Reviewed as OffgridwithJD. Not approving -- same account as the author.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed adversarially. Approving. Two attacks of mine failed against it and I ran the removal proof; here is all three results, including the two that went the author's way.

The removal proof, which the PR does not carry

Delete the fan-out call the PR adds, leaving the two pre-existing sites intact:

fan-out call sites remaining: 2  (was 3)
reverted   8 checks   5 red
  FAIL  covering projection scan matches heap after updating a projected column
  FAIL  read_projection matches the live base after that update
  FAIL  covering projection scan matches heap after updating a non-projected column
  FAIL  read_projection still matches the live base
  FAIL  reconstruct via the projection still rebuilds every live row

Five of the six substantive arms redden, across all three consumers of a projection — the planner's covering scan, read_projection, and reconstruct_via_projection. That is a genuine removal proof and worth putting in the body.

Attack 1, failed: there is no second unfixed site

I went looking for another PgColumnarWriteRow that skips fan-out, since "the sibling path is unfixed" has been the most productive finding across this batch. There isn't one. Every call site in columnar_tableam.c (insert, multi-insert, update) and columnar_vacuum.c (five rewrite sites) is followed by PgColumnarProjectionFanoutRow. The single exception is columnar_write_state.c:3092, the projection writer's own inner write, which correctly must not fan out to itself.

Attack 2, failed and measured: the plan premise does hold where it matters

The suite asserts "planner uses the covering projection" before any update, then makes every comparison that matters after two rounds of updates. My objection was that the updates change row counts, delete vectors and statistics, so a plan switch would leave the post-update arms comparing a plain columnar scan against the heap mirror and passing without touching the projection at all.

I injected a plan probe after each update round. It holds:

PLAN AFTER the projected-column update:
    Custom Scan (PgColumnarScan) on pu
      Columnar Projection: pc
PLAN AFTER the non-projected-column update:
    Custom Scan (PgColumnarScan) on pu
      Columnar Projection: pc

The finding is disproved, and the post-update arms do exercise the projection.

MINOR, and the only thing I would change

The premise that makes those five arms meaningful is asserted once, before the work, and never again. It happens to hold today — I measured it — but nothing in the suite pins it, so a future costing change could silently turn every post-update arm into a heap-versus-heap comparison that passes. Two lines, the same grep -c 'Columnar Projection: pc' after each update round, close it permanently. Not blocking, because the property is true now and I verified it rather than assuming.

Why the design is right

Storing the base row number and filtering through the base delete vector genuinely does mean DELETE needs no fan-out, and it is a reasonable thing to have believed about UPDATE too. The comment now says why the asymmetry exists rather than restating the rule, which is the part that stops the next person re-introducing it.

Heap mirror as the oracle, rather than comparing the projection against itself, is what makes the whole suite worth having.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Premise verified, fix proved on six oracles, and the suite has real teeth — one finding, and the docs are missing

Measured on pgcolumnar-audit, base 8b39053 against head 8fb38ff, PostgreSQL
19beta2 (/usr/local/pg19a), one tree per prefix, distinct .so fingerprints per arm
(6e2ad594884a base, 7b97595ec3d9 head).

1. The bug is worse than "some rows go missing"

Same fixture as yours, plus a heap mirror as the oracle. On the base tree, after
UPDATE pu SET a = a + 1 WHERE a % 10 = 0 (2000 of 20000 rows):

oracle base 8b39053 head 8fb38ff
covering projection scan vs heap mismatch match
read_projection vs live base mismatch match
covering scan vs heap after a later DELETE mismatch match
reconstruct_via_projection vs live base mismatch match
MERGE … WHEN MATCHED THEN UPDATE vs live base mismatch match
projection readback after a rolled-back UPDATE mismatch match
two projections on one table, both read back mismatch (both) match (both)
SAVEPOINTROLLBACK TOUPDATECOMMIT mismatch match

And the tail of it: update every row five times (UPDATE g SET c = c + 1, ×5, 20000
rows) and on the base tree pgcolumnar.read_projection('g','gp') returns zero rows
— the whole projection is hidden, and a covering scan answers as if the table were
empty. No error at any point.

2. The fan-out fires exactly once, and only where it should

The blast-radius question your suite does not ask is whether the new call fires once,
twice, or on the wrong path. Physical rows in the projection's own storage
(row_group.row_count summed for proj_storage_id, not filtered by the delete vector):

                                base storage          projection storage
after INSERT 20000              20000 / 20419 B       20000 /  82578 B     both trees
after UPDATE of 2000 rows       22000 / …             base 20000 (no fan-out)
                                                      head 22000 (+2000, once per row)
after DELETE of 500 rows        …                     unchanged, both trees

So: exactly one projection row per updated row, DELETE still does not fan out, and a
columnar table with no projection is untouched (it appended exactly 500 rows for a
500-row UPDATE on both trees, and still matches heap).

3. Cost, and what reclaims it

Five full-table UPDATEs of 20000 rows:

                    base storage          projection storage
base 8b39053        120000 / 122519 B     20000 /  82578 B   (and answering wrongly)
head 8fb38ff        120000 / 122519 B     120000 / 495511 B
after pgcolumnar.vacuum('g')              20000 /  82587 B   (new proj storage id)

The projection now grows in lockstep with the base under UPDATE, +412,933 bytes for
100,000 updated rows on this fixture, and pgcolumnar.vacuum reclaims all of it. That
is the right trade — the alternative is the wrong answers in §1 — but it is exactly what
your docs tell a reader will not happen, which is §5.

4. Your suite is not vacuous: three mutations, three reds

Each mutation asserted that it applied, and each arm rebuilt, so the .so under test is
the mutant. Against test/projection_update.sh on your head:

CONTROL  unmutated                       8 passed + 0 failed = 8
M1       fan out oldRowNumber            3 passed + 5 failed = 8
M2       fan out twice                   3 passed + 5 failed = 8
M3       remove the call entirely        3 passed + 5 failed = 8

M1 and M3 produce byte-identical hashes, which is the mechanism confirming itself: a
projection row under the deleted base number is hidden by the delete vector, so fanning
out the old number is the same as not fanning out at all. M2 reddening matters more than
the other two — the over-broad direction is the one a suite usually cannot see, and
yours can. test/projections.sh is 64 passed + 0 failed on your head, so your "unchanged,
still passes" claim holds.

The one arm that never moved is check 5, "full-table projection scan still matches heap":
green under all three mutations. SELECT a, c FROM pu carries no restriction, so it is
not a projection scan; it compares a base columnar scan against heap and cannot redden
for this bug. Its name says otherwise. Either give it a sort-key qual or rename it.

5. Finding: UPDATE now latches the projection writer list, and this sequence newly breaks

PgColumnarProjectionFanoutRow builds its writer list once per write state and sets
baseWs->projInited = true even when the list is empty
(src/columnar_write_state.c:3243-3266, then if (baseWs->projWriters == NIL) return;).
Before this PR an UPDATE never called it. Now it does, so an UPDATE before
add_projection in the same transaction latches an empty list and every later write in
that transaction skips the new projection:

BEGIN;
  UPDATE lt SET c = c + 1000 WHERE a <= 5;
  SELECT pgcolumnar.add_projection('lt','lp',ARRAY['a','c'],ARRAY['c']);
  INSERT INTO lt SELECT g,g FROM generate_series(200,210) g;
COMMIT;
                       read_projection rows / base rows
base 8b39053                111 / 111      PASS
head 8fb38ff                100 / 111      FAIL   -- the 11 inserted rows are missing
control, no leading UPDATE  111 / 111      PASS on both

But the class is pre-existing, and I checked before writing this up. Put an INSERT
in front instead of an UPDATE and both trees fail identically (105 / 116, same hash),
because INSERT always called the fan-out. A leading DELETE is fine on both. So the
latch is an existing defect that deserves its own issue; what #873 does is open a second
door into it, and the door it opens is UPDATE, which is what this PR is about. Your
call whether to handle it here or file it — I would file it, and say so in the PR, rather
than grow this diff.

6. No CHANGELOG entry, and three docs sentences are now wrong

Four files, none of them CHANGELOG.md or docs/. Beyond the missing entry, this PR
falsifies text it does not touch:

  • docs/features.md:100 — "Every insert fans out to each projection."
  • docs/administration.md:250 — "New inserts write to the base table and its projections."
  • docs/administration.md:264 — "A projection adds write cost and storage, because inserts write it too."
  • docs/how-to.md:151 — "Later inserts write to the base table and to every projection, so each projection adds write cost."

The last two are sizing guidance, and §3 shows how far out they now are for an
UPDATE-heavy table. src/columnar_write_state.c is the one comment you did update, and
correctly.

7. Suite gaps worth one commit, none of them blocking

  • Nothing pins that the fan-out reaches more than one projection. Your suite creates
    one, so a fix that fanned out only to the first would pass it. I measured it — two
    projections, both correct after UPDATE — so the code is right and only the arm is
    missing.
  • No arm asserts an UPDATE changed any row. psql_run's status is never read, and a
    zero-row UPDATE leaves the table in the state the premise check has already blessed.
  • No regression arms: INSERT still fans out exactly once, DELETE still does not,
    a table with no projection is unaffected, a rollback is clean. All four hold —
    §2 and §1 are the measurements — and all four are cheap to pin.
  • The suite never asserts its own check count, so a gutted suite still reports PASSED.

8. Two of my own arms were wrong; recording them so the numbers above are readable

My premise check "2000 rows were updated" asserted count(*) WHERE a % 10 = 1 = 2000 and
got 4000. The code is right and my expectation was wrong: after a = a + 1 the rows that
started at a % 10 = 1 are still there too. And my ON CONFLICT DO UPDATE arm passed on
both trees for no good reason — it never ran, because columnar answers
ERROR: columnar: row locking is not supported yet. That arm proves nothing; the MERGE
arm in §1 is the second update path that does.

Summary

The fix is correct, minimal, in the right place, and mirrors the insert site exactly. Six
independent oracles agree, the cost is bounded and reclaimable, and your suite catches all
three mutations I threw at it. What is missing is the CHANGELOG and the four documentation
sentences in §6; §5 is worth an issue of its own. I do not review PRs on this account
formally — this is a comment, and the approval on record is jdatcmd's.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Two more arms, both of which the fix passes: updating the SORT KEY, and NULLs

A second pass over my own review asked which case is missing from every suite in
the tree. Two were, and they are the two where I would most expect a fan-out fix to
come apart. Both are measured on base 8b39053 vs head 8fb38ff, pg19a.

Updating the projection's sort key

Neither arm in projection_update.sh touches c, the projection's sort key. That
is the only update that moves a row across the projection's per-chunk min/max
ranges, which is what the projection's pruning rests on. So:

-- every row in the probed range jumps out of it
UPDATE ks SET c = c + 800 WHERE c BETWEEN 100 AND 200;
oracle (heap mirror) base 8b39053 head 8fb38ff
count(*) WHERE c BETWEEN 900 AND 1000 columnar 2000 vs heap 4020 4020 = 4020
the range the rows LEFT match match
the range the rows ARRIVED IN mismatch match
the whole table match match
the arrival range again, after pgcolumnar.vacuum match match

On the base tree a covering scan returns 2000 rows where 4020 is right
a wrong count from an ordinary WHERE on the sort key, no error. The fix handles
it, and still handles it after the maintenance rewrite. Worth an arm; it is two
lines on top of your existing fixture.

NULLs in the projected column

Nothing in the fixture is NULL, and the fan-out is handed slot->tts_isnull.
With 285 NULLs seeded, then an UPDATE in each direction (value → NULL, and
NULL → value):

live base rows                 2000
rows in the projection   base  1886      head  2000

So the NULL path fans out correctly too, and on the base tree it is missing 114 of
2000 rows. A WHERE c IS NULL fixture belongs in the suite.

Corrections to my own arms, again

  • My finer NULL arms errored — I parsed read_projection's text back into
    (a, c) and the cast failed, so got [] and QUERY_ERROR.1 are my instrument,
    not your code. How read_projection renders a NULL is therefore unestablished
    here, and the row count above is what stands.
  • I had intended to claim that the suite's a::text || '|' || c::text oracle is
    blind to NULL rows because || yields NULL. My own control refuted it: the
    hash came back non-empty, so pgc_set_hash is not dropping those rows. I am
    recording the refutation rather than the claim. The residual, unmeasured, is
    weaker: every NULL row may collapse to the same token, so the oracle would not
    notice a NULL row carrying the wrong a.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Pushed 8bca071: the CHANGELOG entry and the four docs sentences.

I verified the four locations against the tree rather than taking them from the review, and all four did say only that inserts write projections:

file:line said
docs/features.md:99 "Every insert fans out to each projection."
docs/administration.md:250 "New inserts write to the base table and its projections."
docs/administration.md:264 "A projection adds write cost and storage, because inserts write it too."
docs/how-to.md:151 "Later inserts write to the base table and to every projection"

Each now says updates write too. Each also says why a delete does not, because "insert and update but not delete" reads as an oversight unless the reason travels with it: a projection takes visibility from the base delete vector, so the vector hides a deleted row number from the projection without any rewrite. That is the same reasoning the code comment in columnar_write_state.c now carries, so a reader who finds either one gets the whole rule.

The CHANGELOG entry states the failure rather than the call that fixes it, and names the part that makes this shape worse than a plain wrong answer: the base table was always correct, so the same query returned different results depending on whether the planner chose the projection.

Evidence, run rather than asserted:

  • test/plain_language_check.py passes on all fifteen user-facing documents.
  • CHANGELOG.md carries no em or en dash (grep -c returns 0), which is the other thing docs_style.sh gates.
  • docs_style.sh on the branch: 9 checks, 0 failed, exit 0.

Docs and CHANGELOG only; no source file touched, so the approved change is unchanged.

jdatcmd and others added 2 commits September 1, 2026 20:36
Co-authored-by: Cursor <cursoragent@cursor.com>
Four sentences across three documents said that inserts write projections, and
stopped there. That was true when only insert fanned out. This PR makes UPDATE
fan out as well, so all four now understate what a projection costs and, worse,
a reader sizing a projection would budget for the wrong write volume.

  docs/features.md:99          "Every insert fans out to each projection."
  docs/administration.md:250   "New inserts write to the base table and its projections."
  docs/administration.md:264   "A projection adds write cost and storage, because inserts write it too."
  docs/how-to.md:151           "Later inserts write to the base table and to every projection"

Each now says updates write too. Each also says why a delete does not, because
"insert and update but not delete" reads as an oversight unless the reason is
there: a projection takes visibility from the base delete vector, so the vector
hides a deleted row number from the projection without any rewrite.

The CHANGELOG entry states the failure a reader would have hit rather than the
call that fixes it: the base table was always correct, and the loss was confined
to reads the planner served from a projection, so the same query returned
different answers depending on whether the projection was chosen.

Verified rather than assumed: test/plain_language_check.py passes on all fifteen
user-facing documents, and CHANGELOG.md carries no em or en dash, which is the
other thing docs_style.sh gates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WDbfRym2V1sYFmMZ5gnsQL
@OffgridwithJD
OffgridwithJD force-pushed the audit/projection-update-fanout branch from 8bca071 to 3a9249f Compare September 2, 2026 02:37
@OffgridwithJD
OffgridwithJD marked this pull request as ready for review September 2, 2026 13:47
@OffgridwithJD
OffgridwithJD merged commit b4f0a45 into main Sep 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants