Skip to content

fix: a rewrite's own projection ids are not foreign reservations (#345) - #347

Merged
jdatcmd merged 1 commit into
mainfrom
fix/345-projection-stripe-ids
Aug 3, 2026
Merged

fix: a rewrite's own projection ids are not foreign reservations (#345)#347
jdatcmd merged 1 commit into
mainfrom
fix/345-projection-stripe-ids

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Fixes #345.

pgcolumnar.sort_status reported ~90% decay on a table that had just been fully reclustered, whenever the table had a projection. The rows were correctly ordered; only the reporting was wrong.

Cause

record_online_sorted_extent walks the rewrite's own reserved stripe ids and stops at the first gap, reasoning that a gap means another session took that id. That is false within the rewrite's own transaction.

A projection writes through its own inner write state but calls ColumnarWriteRow with the base relation, so it reserves from this relation's stripe counter. Its groups are then recorded under the projection's own storage id and never appear in the base relation's group list. ColumnarWriteStateStripeIds returned only the base write state's reservations, so base and projection draws alternated, ours came back as 17, 19, 21, ... and the run walk broke at the first step.

The rewrite was competing with itself.

Fix

The ids are unioned with the projection fan-out's, taking only those at or above the rewrite's own first id -- so anything drawn before this rewrite began is still excluded and #342's guarantee is untouched. A projection id landing inside the run is harmless: sort_status counts base-relation groups, and a projection's group is recorded under a different storage id.

Reproduction, with a control

Both arms identical except for add_projection:

arm sorted_rows appended_rows
no projection 200000 0
with a projection 20000 180000

Removal proof

noproj (control) withproj
fix reverted 200000 / 0 PASS 20000 / 180000 FAIL
fix in place 200000 / 0 PASS 200000 / 0 PASS

The control staying green in both runs is the point: it pins the test to this mechanism rather than to breakage in general.

Relationship to #342 / #344

Opposite errors in the same function. #342 was the mark claiming groups it should not; this is the mark refusing groups it should. #344 changed the mark to a range [ours[0], runEnd], which fixed the over-claim but left runEnd computed by the same consecutive-run walk, so this survived it untouched.

Direction of error here is the safe one -- more decay reported than exists, so at worst an unnecessary re-sort. But a table reporting 90% decay right after a successful full recluster makes sort_status useless for the one thing it exists to do.

Gate

Full 15-19 matrix, ALL VERSIONS PASSED, all suites.

🤖 Generated with Claude Code

https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8

pgcolumnar.sort_status reported ~90% decay on a table that had just been
fully reclustered, whenever the table had a projection. The rows were
correctly ordered; only the reporting was wrong.

record_online_sorted_extent walks the rewrite's own reserved stripe ids and
stops at the first gap, on the reasoning that a gap means another session
took that id. That reasoning is false within the rewrite's own transaction.
A projection writes through its own inner write state but calls
ColumnarWriteRow with the BASE relation, so it reserves from this relation's
stripe counter; its groups are then recorded under the projection's own
storage id and never appear in the base relation's group list.
ColumnarWriteStateStripeIds returned only the base write state's
reservations, so base and projection draws alternated, ours came back as
17, 19, 21, ... and the run walk broke at the first step. The rewrite was
competing with itself.

The ids are now unioned with the projection fan-out's, taking only those at
or above the rewrite's own first id, so anything drawn before it began is
still excluded. A projection id inside the run is harmless to sort_status:
it counts base-relation groups, and a projection's group number is not one.

Reproduced with a control, which is what establishes the projection as the
cause rather than assuming it. Both arms are identical except for
add_projection:

  noproj   sorted 200000, appended 0        (correct)
  withproj sorted  20000, appended 180000   (wrong)

Removal proof: with the union reverted, the withproj arm fails at
20000/180000 against 200000/0 while the noproj control stays green, so the
test is pinned to this mechanism and not to breakage in general.

This is the opposite error to #342 in the same function. #342 was the mark
claiming groups it should not; this is the mark refusing groups it should.
The range fix in #344 addressed the first and left runEnd computed by the
same consecutive-run walk, so this survived it. Direction of error here is
the safe one -- more decay reported than exists, so at worst an unnecessary
re-sort -- but a table reporting 90% decay right after a successful full
recluster makes sort_status useless for deciding when a re-sort is worth
its cost.

Full 15-19 matrix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011miCFRSatixeNRw3w5yNq8
@ChronicallyJD

Copy link
Copy Markdown
Collaborator

Reviewed — correct and mergeable. This is the fix I'd scoped for #345 (include the rewrite's own projection reservations in ours), and the details hold up:

  • ColumnarWriteStateProjStripeIds mirrors the base accessor — copies each innerWs->reservedStripeIds[0..nReservedStripeIds), sets *n, returns NULL/*n=0 when there are no projection writers; the caller's if (nProj > 0) / NULL-guarded pfree handles both.
  • The projIds[j] >= lo filter is the load-bearing detail: it folds in the projection draws that interleave within the run while still excluding anything drawn below the rewrite's own first id, so recluster_extent fails intermittently: extent mark claims concurrently inserted rows (sorted 1520000 > base 1500000) #342's "a group numbered below the run is not ours" property is preserved.
  • A genuinely foreign gap still truncates. Traced base={17,19,23}, proj={18,20}, foreign={21,22}: merged→{17,18,19,20,23}, the walk runs 17→20 and breaks at 23 (expected 21), so the run stops at the foreign interleave and the rewrite's own group 23 is conservatively reported as decayed. Safe direction intact.
  • Scope is right: only record_online_sorted_extent needed it. The offline record_sorted_extent derives first/last from the live base group list rather than the reservation walk, so it never had this bug — no change there is correct, not an omission.
  • Test is control-based (noproj vs withproj, identical but for the projection) and asserts exact counts (200000/0), so it reproduces sort_status reports ~90% decay immediately after a full recluster when the table has a projection #345 and can't pass vacuously.

No objections.

@jdatcmd
jdatcmd merged commit b7301d1 into main Aug 3, 2026
19 of 21 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.

sort_status reports ~90% decay immediately after a full recluster when the table has a projection

2 participants