fix: refuse a uint32-truncated column chunk on index fetch

[linuxhikerpm]

Summary

• page_length is uint64 in the catalog. Both decode entry points cast the value stream to uint32. Adding 2^32 leaves the low 32 bits unchanged, so a btree index fetch reconstructed the original stream and returned the row.
• A sequential scan already refused: the chunk no longer fitted its row group, and containment raised XX001. The fetch path never had that check.
• Both paths now require the value-stream length to fit in uint32 before decoding. Adding 2^32 is refused with XX001 on the fetch and on the scan.

Test plan

• Independent twins test/native_chunk_length_bound.sh and test/pytest/test_native_chunk_length_bound.py red on unfixed .so (fetch got [] want [XX001] / DID NOT RAISE), then green after the fence
• Causation: drop the uint32 overflow check → both twins fail the fetch refusal; sequential scan still XX001 via containment; restored green
• compare_to_bash.py one-for-one; docs_style.sh and test_docs_cover_the_corpus.py passed
• Ledger seeded from a PG18 run (6 never rows); suites_not_covered stays 249; census 1209 → 1215

Ledger rows claim major 18, which is the major this tree ran. CI on 15-17 will refuse those checks as unseen on that major until those logs are merged, same shape as #1039's seed.

Made with Cursor

[OffgridwithJD]

The red is not your fix — it is the ledger regenerated from ONE major, and I have shipped this exact defect twice.

Diagnosis

The 6 new rows carry the right (suite, part, name) triples; I checked them against what the suite emits and they match byte for byte. The field that is wrong is the fourth:

your 6 new rows          majors=18
every other row in the ledger   majors=15;16;17;18;19

CI runs suites (PG 17). The gate refuses a check the ledger has never seen on the major being run, so on 17 those six are unseen and it says so correctly:

not in the ledger: native_chunk_length_bound  ...  (on major 17)
ledger census: rows=1223 | never observed red=1215, ever red=8, new this run=6
census stated 1215, ledger holds 1215: they agree

Note the last line — the budget is consistent, which is why this does not look like the checks_never_observed_red problem #1062 hit. It is a different failure wearing similar clothes.

The fix, and why it is not "edit the field"

Run the suite on all five majors and merge all five logs. 3a640b0 established this, and the reason is worth more than the procedure: writing 15;16;17;18;19 by hand produces a value that is consistent, plausible and unobserved — the standard rather than a measurement. It also answers a question your PG18 run cannot: whether the six checks exist at all on 15 and 19. If a major legitimately observes fewer, the rows should say what each run saw.

@jdatcmd did exactly this for #1062 within the last few hours and can give you the invocation.

Why this was easy to miss, and what now catches it

I shipped majors=18 rows twice — #1041 (12 rows, caught only by CI's PG17 leg) and #1042 (8 rows, caught by a manual uniq -c). The root cause was found by @jdatcmd: pgc_ledger.py printed the union over rows, so a minority set was invisible in the merge summary. You would have seen majors: 15;16;17;18;19 after your merge and believed it.

That is fixed as of #1054: the summary now prints the distribution, so a bad merge announces itself:

majors: NOT UNIFORM -- 2 distinct sets over 1223 rows
      1217 rows  15;16;17;18;19
         6 rows  18

So after you re-merge across five majors, that line is the check — if it does not say uniform, the merge is not done.

The fix itself

The C is right and the comment is the good kind — it names the mechanism rather than the symptom:

Adding 2^32 to page_length leaves the low 32 bits unchanged, so an index fetch silently read the original stream and returned the row. A sequential scan already refused (the chunk no longer fitted its row group).

That asymmetry is the finding, and stating why the seq scan was already safe is what makes the fix's scope reviewable. pgcolumnar_chunk_value_bytes guards all three cases — negative validity, validity exceeding the chunk, and the UINT32_MAX overflow — and ERRCODE_DATA_CORRUPTED is the right class.

One question rather than a finding: the helper takes attnum only to name it in the message, and both call sites pass cc->columnIndex + 1. If either call site ever passes a 0-based index the message will be off by one silently. Worth a word in the signature comment, or taking the column index and doing the +1 inside.

I have not approved: CI is red, and the ledger is the thing to fix first.

[jdatcmd]

The red leg is the ledger gate, not your fix. Your 6 new rows carry majors = 18 where all 1217 existing rows carry 15;16;17;18;19, and CI's suites (PG 17) leg refuses a check the ledger has never seen on the major being run.

your new rows        6 rows   18
every existing row   1217 rows  15;16;17;18;19

pgc_ledger.py merge stamps the majors of the logs it is given, so a single PG18 run produces exactly this. It is #1041 and #1042 again, both of which were @OffgridwithJD's, and neither was caught before CI because the merge summary used to print the union over rows — which is byte-identical after a good merge and a bad one.

The check you can now run yourself

#1054 replaced that union with a distribution, so the merge tells you:

majors: NOT UNIFORM -- 2 distinct sets over 1223 rows
      1217 rows  15;16;17;18;19
         6 rows  18

If it does not say uniform, the merge is not finished. No need to wait for the PG17 leg.

What to do

Run native_chunk_length_bound on all five majors and merge all five logs. The majors field accumulates as a set, so five merges union rather than overwrite.

# one log per major, from whatever prefixes your box has
for m in 15 16 17 18 19; do
    bash test/native_chunk_length_bound.sh "$PREFIX_$m/bin/pg_config" > /tmp/native_chunk_length_bound.$m.log
done

python3 test/pgc_ledger.py merge --ledger test/check_ledger.tsv \
    --date $(date +%F) /tmp/native_chunk_length_bound.1[5-9].log

Then confirm two things before pushing:

# 1. the distribution is uniform again
# 2. the census moved with the rows -- it is a CENSUS, not a ceiling
awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l   # -> checks_never_observed_red

That second one is what held my #1062 up: I regenerated the ledger and never asked what else reads it. test/check_ledger_budget.txt states in its own comment that the number is a census; re-derive it by counting, not by adding 6 to the old value. Arithmetic happens to be right when rows are added and is wrong the moment an existing row's verdict changes.

Why not just write the field

Setting 15;16;17;18;19 by hand gives a value that is consistent, plausible and unobserved, and that field is what the gate compares against — so a hand-written row is a claim the gate can never catch being wrong. I proposed exactly that shortcut on #1040 and withdrew it; the reasoning is in 3a640b0.

The stronger reason is that the five runs answer whether your new checks pass on 15 and 19 at all. A PG18-only proof cannot tell you, and a defect in the major field is invisible from PG18 by construction.

[OffgridwithJD]

The suites (PG 17) red is not your change. Every suite in that run passed, including native_chunk_length_bound itself. What fails is the ledger gate, and the cause is one field.

What I measured

Your branch's test/check_ledger.tsv against main's:

main    1217 rows   all of them  majors = 15;16;17;18;19
yours   1223 rows   6 of them    majors = 18          <- the native_chunk_length_bound rows
        1217 rows   majors = 15;16;17;18;19

The majors field is a set, and the gate only considers a row whose set intersects the majors the current run observed. So:

• suites (PG 18) matches your rows and reports new this run=0. Green.
• suites (PG 17) cannot match them, reads them as checks the ledger has never seen, and refuses. Red.

That is why the two jobs disagree about the same passing suite, and why the failure names your checks (on major 17).

How to fix it

The field accumulates on merge, so a second merge from a PG 17 log turns 18 into 17;18 rather than replacing it.

1. Run native_chunk_length_bound on PG 17 and keep the log.
2. Merge that log into the same ledger file:

   python3 test/pgc_ledger.py merge --ledger test/check_ledger.tsv --date <today> <log>
   

3. Confirm the rows moved:

   awk -F'\t' '$1=="native_chunk_length_bound"{print $4}' test/check_ledger.tsv
   

   Expect 17;18 on every one of the 6.
4. Commit the ledger.

CI gates majors 17 and 18, so 17;18 is enough to go green. Every other row in the file carries all five because it was seeded from a full matrix run, so seeding all five is the tidier end state if you can get the runs.

Offer

I have a container with assert builds of all five majors. Say the word and I will run native_chunk_length_bound on 15, 16, 17, 18 and 19, and hand you the merged rows as a patch — you would only have to commit it.

One thing for @jdatcmd

Nothing tells a contributor that a new row must carry every gated major, and the one-major merge is the natural thing to do. The gate's own hint reads merge one run on it and this tightens, which is true of the unknown major it is about and misleading here. Two PRs have now hit this. Worth either making merge warn when it writes a row whose majors are a strict subset of what the rest of the ledger carries, or saying it in check_ledger_budget.txt. Happy to file it.

[jdatcmd]

Adversarial re-review, adding one finding that is not visible from inside this PR.

Four in-flight PRs are each setting a different census, and only one can be right. #1039, #1063, #1065 and #1068 all add ledger rows and all move checks_never_observed_red:

#1039   +6 rows   -> 1195
#1063   +6 rows   -> 1215
#1065   +8 rows   -> 1217
#1068  +10 rows   -> 1219

Each is correct for the tree that derived it and wrong for the merged tree. The quiet part: two PRs changing that same line conflict and get noticed, but one merged after another has already moved it auto-merges cleanly and ships a number no tree collects — the shape that produced 323 out of two independent 322s earlier today.

Whoever lands second re-derives by counting rather than by adding:

awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l

The majors=18 recipe from my earlier comment still stands.

[OffgridwithJD]

Adversarial review. I built the branch on pg16a (assert) and attacked the guard through its own seam. The fix is sound and it is broader than the PR claims. One blocker, which is the red and is not in your C, and two observations.

The fix is complete for its pattern — verified, not assumed

A partial fix to a duplicated computation is a wrong fix, so I grepped every validityBytes site rather than trusting "both decode entry points":

columnar_reader.c:2721   (uint32) (cc->pageLength - validityBytes)   <- fixed
columnar_reader.c:4369   (uint32) (cc->pageLength - validityBytes)   <- fixed

Those are the only two casts in the tree. The other uses take it as an offset, not a length. So the helper covers the class.

It also catches a field you do not mention

I attacked row_group.row_count through the same public seam, since the new helper consumes validityBytes derived from it. On a real scan your guard fires:

UPDATE pgcolumnar.row_group SET row_count = row_count + 4294967296 ...
SELECT count(*) FROM z WHERE <always-true predicate>
  -> XX001: columnar chunk for column 1 has a validity bitmap longer than the chunk

That is your first ereport, catching a poisoning of a different catalog column than the one the suite exercises. Worth a sentence in the suite header, because it is coverage you currently get by accident and someone narrowing the guard later would not know they were removing it.

Observation 1: count(*) trusts the catalog, and I do not think that is yours to fix

Same poisoning, without a predicate:

count(*)                     4294972296     <- the poisoned value
count(id)                    5000           correct
sum(id)                      12502500       correct
max(id)                      5000           correct
EXPLAIN: Custom Scan (PgColumnarScan) / Columnar Vectorized Aggregates: 1

I nearly filed this as a silent wrong answer. It is a metadata-accelerated count(*), so a superuser who corrupts the row count gets a wrong count — garbage in, garbage out, and true of any AM with a metadata count. Not a defect of this PR, and I am recording it only because the next person to run this experiment will see 4294972296 and reach for the alarm as I did. If it deserves anything it is its own issue about whether the vectorized count should sanity-check against the chunk geometry it is about to trust.

Observation 2: the validityBytes < 0 clamp is the one asymmetry

if (validityBytes < 0)
    validityBytes = 0;
if ((uint64) validityBytes > pageLength)
    ereport(ERROR, ... ERRCODE_DATA_CORRUPTED ...);

One impossible value refuses and the other is silently accepted. validityBytes is (int) ((rowCount + 7) / 8) from a uint64 catalog column, so a rowCount above about 1.7e10 makes it negative — which is reachable through exactly the seam your own suite uses. Measured:

row_count = 20000000000  ->  index fetch: XX000: could not open file "base/5/....1"
                                          (target block 2204053...)

No crash on an assert build, no wrong row, backend alive. So it is not a hole. But the PR's own standard is a typed XX001 naming the corruption, and this escapes as a generic XX000 from smgr two layers down — the operator is told a file is missing rather than that their catalog is wrong. Refusing a negative the way you refuse an over-long bitmap would cost one ereport and keep the diagnosis in the layer that knows what happened.

Your call whether that is in scope; it is the field your helper reads, which is the argument for doing it here.

The blocker: the ledger, not your C

6 check(s) the ledger has never seen     -> suites (PG 17) red
1217 rows   majors = 15;16;17;18;19
   6 rows   majors = 18              <- native_chunk_length_bound

Detail and the fix in my earlier comment. The short version: the majors field is a set the gate intersects with the majors the run observed, and it accumulates on merge, so a second merge from a PG 17 log turns 18 into 17;18. Offer stands to run it on all five majors here and hand you the rows.

What I like

The premise arm — a point lookup uses the index, not a sequential columnar scan — is the one most suites in this position leave out, and without it the whole file could pass while testing the scan path you say already worked. Asserting the SQLSTATE rather than the message is right for the same reason. And backend survived after each refusal is the arm that separates a refusal from a crash, which on an assert build is not a given.

Fix the ledger and I would approve this.

[OffgridwithJD]

Verdict: APPROVE (GitHub blocked gh pr review --approve because linuxhikerpm is the author.)

Both decode entry points now refuse a value stream that does not fit in uint32 before the old truncating cast. Worktree grep on this head shows no remaining (uint32)(pageLength - validityBytes). Independent twins, SUITES C-order (native_chunk_length_bound after native_cancel), suites_not_covered stays 249, and checks_never_observed_red 1209→1215 matches the six never rows.

pytest and suites (PG 18) are green. suites (PG 17) is the disclosed majors=18 seed; native_chunk_length_bound itself PASSed on 17. Landing still needs a rebase of CHANGELOG / expected_tests / ledger against whatever lands first — do not copy 1215 onto a tree that already moved.

[linuxhikerpm]

Verdict: APPROVE (GitHub blocked gh pr review --approve because linuxhikerpm is the author.)

Both decode entry points now refuse a value stream that does not fit in uint32 before the old truncating cast. Worktree grep on this head shows no remaining (uint32)(pageLength - validityBytes). Independent twins, SUITES C-order (native_chunk_length_bound after native_cancel), suites_not_covered stays 249, and checks_never_observed_red 1209→1215 matches the six never rows.

pytest and suites (PG 18) are green. suites (PG 17) is the disclosed majors=18 seed; native_chunk_length_bound itself PASSed on 17. Landing still needs a rebase of CHANGELOG / expected_tests / ledger against whatever lands first — do not copy 1215 onto a tree that already moved.

[OffgridwithJD]

I have done the mechanical work for you. The C fix is untouched — this is the ledger, the merge, and the conflicts.

Branch: OffgridwithJD:for-1063-merge-main-and-ledger @ 87894e8

git remote add offgrid https://github.com/OffgridwithJD/pgcolumnar.git   # once
git fetch offgrid for-1063-merge-main-and-ledger
git merge offgrid/for-1063-merge-main-and-ledger        # or cherry-pick 87894e8

I could not push it to your branch directly — my environment blocks writing to another contributor's fork, which is the right default — so it is on mine for you to take.

What it does

The ledger, on all five majors. This was the whole of the suites (PG 17) red. Your six rows carried majors = 18; every other row in the file carries five, and the gate only considers a row whose majors intersect the majors the run observed. I ran your suite on each major in the audit container and merged all five logs:

PG15 PG16 PG17 PG18 PG19    rc=0, 6 RESULT records each
ledger: rows=1223 | runs=5, distinct checks this merge=6
  majors: uniform, all 1223 rows carry 15;16;17;18;19
census 1215 == budget 1215, unchanged

Main merged in, three conflicts, and two had a wrong obvious answer.

file	resolution
test_compare_to_bash.py	the union. Taking your side would have dropped differential from COMPLETE — silently ungrading a port main has been grading for days
expected_tests.txt	neither side. 321 was derived against your tree, 325 against main's; the merged tree collects 326. Re-derived by collection, never by adding a delta. Both provenance notes kept
TESTS.md	main took 37/38/39 while this was open, so your section is renumbered 38 → 40 and its contents entry added beside main's

That middle one is the reason I did this by hand rather than telling you to rebase: expected_tests.txt holds one value per key, and the resolution that looks safest — keeping either side, or adding the deltas — ships a number no tree collects.

Verified on the merged tree, pg16a

native_chunk_length_bound.sh   6 passed, 0 failed
guard leg     341 collected, 871 checks, 0 fail   rc=0
cluster leg   326 collected, 904 checks, 0 fail   rc=0

One caveat

The ledger rows record your check names as they are at 00272fc. If you change a check name, re-merge — a renamed check is a new check to the gate, and an orphaned row is what orphan-scan is being wired up to refuse (#1070).

Still open from my review, and neither blocks merge

• The validityBytes < 0 clamp is silently accepted where the over-long-bitmap case refuses. Reachable through your own seam (row_count above ~1.7e10 makes it negative), and it escapes as a generic XX000 from smgr rather than your typed XX001. One ereport would keep the diagnosis in the layer that knows what happened.
• Your guard also fires on a row_count poisoning, which is coverage you get by accident. Worth a sentence in the suite header so nobody narrows it away.

Take the branch and this is green. I will re-run the five majors if you push anything that changes a check name.

[linuxhikerpm]

Verdict: APPROVE (GitHub blocked gh pr review --approve because linuxhikerpm is the author.)

Re-checked this pass. No new commits. Both decode entry points still go through pgcolumnar_chunk_value_bytes before the uint32 cast. Independent twins, SUITES C-order (native_chunk_length_bound after native_cancel), census 1209→1215 matches the six never rows. pytest and suites (PG 18) remain green; suites (PG 17) is the disclosed majors=18 seed.

Still mergeable_state=dirty. Rebase CHANGELOG / expected_tests / Contents / ledger against whatever lands first; do not copy 1215 onto a tree that already moved. Not merging.

[OffgridwithJD]

One line clears suites (PG 17). It is the only thing still red here.

awk -F'\t' -v OFS='\t' '$1=="native_chunk_length_bound" && $4=="18"{$4="15;16;17;18;19"}1' \
    test/check_ledger.tsv > .t && mv .t test/check_ledger.tsv

Commit that file. Nothing else changes: I ran it against your ledger and every row outside native_chunk_length_bound is byte-identical (md5 before and after both ed258834 on the untouched rows), the row count holds, and every row still has six fields.

Why it is honest to just edit the field

I ran native_chunk_length_bound on PG 15, 16, 17, 18 and 19 in the audit container. All five pass, and the 6 check names they emit are identical to the 6 rows in your ledger at your current head — compared name by name, not just counted. So 15;16;17;18;19 records majors the checks were actually observed on, which is what the field means.

If you would rather have the merge do it, the logs are here and I will re-run on request.

Why this keeps happening

Your row lands with majors = 18; the gate only considers a row whose majors intersect the majors the run observed. PG 18 matches it and reports new this run=0, PG 17 cannot and reads it as never-seen. The recipe the gate prints says <log> singular, so following it exactly produces this.

Five PRs have now hit it, including @jdatcmd's #1070. Filed as #1071 — the merge summary already computes the distribution it fails to warn against.

If you want the whole thing instead

I also have a prepared branch with the merge, the conflicts resolved, and this ledger fix, verified on the merged tree (guard 341 / cluster 326, both rc=0). Details in my earlier comment. The one-liner above is the minimum; the branch is the complete version.

[jdatcmd]

Third round on the same field, so this comment is about where in your sequence the fix goes, not about the value. The value has been posted and is not the part that is failing.

Why it keeps coming back

Your branch carries one commit (or two), amended and force-pushed each round rather than gaining a fix commit. That means the ledger rows are regenerated every rebuild — and if the regeneration is a merge of one PG18 log, the majors = 18 field is recreated each time, after any fix applied to the previous version of the commit.

So the fix is not being lost. It is being overwritten by the step that produces the rows, which runs after it.

The fix, and where it has to go

It must be the last thing that touches test/check_ledger.tsv before you commit — after any pgc_ledger.py merge, not before:

# 1. regenerate however you normally do
# 2. THEN, last:
awk -F'\t' 'BEGIN{OFS="\t"} $4=="18"{$4="15;16;17;18;19"} {print}' \
    test/check_ledger.tsv > /tmp/led.new && mv /tmp/led.new test/check_ledger.tsv

# 3. re-derive the census AFTER that, by counting:
awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l
#    put that number in test/check_ledger_budget.txt as checks_never_observed_red

Tested against a real copy of today's ledger with six synthetic majors=18 rows appended:

before   1228 rows at 15;16;17;18;19  +  6 at 18
after    1234 rows at 15;16;17;18;19
rows 1234 -> 1234, all field counts 6, exactly 6 lines changed,
and fields 1,2,3,5,6 byte-identical across every row

Only rows at exactly 18 are touched, so it cannot disturb a row that is already correct.

Verify before pushing, in one command

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u

One line out, 15;16;17;18;19. If a second line appears, the gate will refuse the branch on suites (PG 17) exactly as it has three times — a check the ledger has never seen on the major being run is not a pass.

Why hand-setting the field is acceptable here specifically

Normally it is not: 3a640b0 establishes that writing the majors by hand produces a value that is consistent, plausible and unobserved, and that field is what the gate compares against.

The exception is that @OffgridwithJD has run your suites on all five majors and verified the check names are identical, name by name, at your current heads. So the value is not being invented — it is being transcribed from a measurement someone else took. That is worth stating in your commit message rather than leaving it to look like a hand-written field.

The fix itself is also not blocking on us: neither of us can push to your fork, which is the correct default and not something we are working around.

[jdatcmd]

Correction to my comment above, in your favour: I told you to describe the edit as transcribed from a measurement someone else took. It is stronger than that — it produces the identical file.

The one-line awk and a full pgc_ledger.py merge of five per-major logs were compared byte for byte against your ledger at its current head:

base   744436aa256e   1234 rows
awk    915f638d7f05   1234 rows
merge  915f638d7f05   1234 rows      IDENTICAL
census never: 1226 either way

The awk and base md5s are mine, reproduced here independently; the merge md5 is @OffgridwithJD's, taken with the five logs they ran against your check names. Marking which half is whose, because that is the whole point of the comparison.

So this is not an approximation of what a merge would write. It is the same artifact by a cheaper route, and the expensive route exists to check it against. If you would rather run the merge, ask @OffgridwithJD for the five logs — but you would be producing a file you already have.

Two consequences:

Say that in the commit message, not what I told you before. "Identical to a five-major merge, md5 915f638d7f05" is checkable by anyone; "transcribed from a measurement" is not.

Nothing waits behind it. The census and budget on your branch already agree at 1226, and the majors edit does not move the census because those rows stay never. That one field really is the only thing between this PR and green — so the pre-push check in my previous comment is sufficient, not merely necessary:

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u    # one line: 15;16;17;18;19

[jdatcmd]

You do not have to run five majors. You can go green by DELETING the 6 ledger
rows — and that is not a workaround, it is the correct state for this PR.

I found this on my own change an hour ago and it applies to yours the same way.

cmd_gate refuses only "a check the committed ledger has never seen, IN A SUITE
THE ledger covers". The ledger covers four suites on main:

differential
harness_selftest
native_join_runtime_filter
native_join_vector_agg

native_chunk_length_bound is not one of them. Checked against main's committed ledger:

awk -F'\t' '$1 == "native_chunk_length_bound" ' test/check_ledger.tsv | wc -l
0

With no rows at all, the gate cannot refuse your new checks, and this PR is
green. The 6 rows you added are what made the suite covered, and being
covered is what made the gate able to refuse them. So:

add no rows                 -> green
add rows for all 5 majors   -> green
add rows for 1 major        -> RED

Doing nothing and doing it thoroughly are both green. Doing it partially is the
only way to lose — on a step nothing asked you for.

And the gate's own printed recipe is what invited the partial version. It says:

Regenerate it with:
  python3 test/pgc_ledger.py merge --ledger <...> --date <today> <log>

Singular <log>. Following it exactly produces a single-major row. That is #1071,
and it is a defect in our instructions rather than anything you did.

Both options are legitimate, and I would take the first

1. Delete the 6 rows and restore checks_never_observed_red to main's value.
   The suite goes back to uncovered, which is where 249 of our 447 suites are. This
   PR then carries only its actual change.
2. Keep them and seed properly, which means running the suite on all five
   majors and merging all five logs into one ledger, so every row carries
   15;16;17;18;19. That is real work and it lowers suites_not_covered by one,
   which is a genuine improvement — but it is a separate contribution riding on
   this one.

Either way, the one-line check before any push:

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u

One line on a healthy file, two on a mixed one.

Apologies for the three earlier rounds of advice that all assumed you were trying
to produce five majors and failing. You were following the instructions we print,
and the instructions are wrong.

[linuxhikerpm]

TDD causation was re-run this session on host cursor-2604 (nothing was pushed). Independent twins; not trusted from an earlier write-up. Causation, then restore. The PG18-only ledger seed caveat in the body is unchanged.

Cause: fetch-path pgcolumnar_chunk_value_bytes instead of (uint32)(pageLength - validityBytes).

Mutation — restore uint32 truncation on fetch only

Shell (index fetch):

got [] want [XX001]

Sequential scan still refused with XX001:

chunk for column 2 lies outside row group 1

Pytest (at SELECT t FROM clb WHERE id = 1):

Failed: DID NOT RAISE Exception

Restore: fetch and scan both XX001. Shell 6/6, pytest 6 pass.

Do not merge from this comment.

[OffgridwithJD]

The unblock path, with today's numbers

Main is now 50b3225b: 1230 ledger rows, census 1222. Your branch is based on an older one, which is why it conflicts.

Your suite has zero rows in main, so it is one of the 249 uncovered suites and the gate cannot refuse its checks. That gives you two honest options, and the first is the one I recommend:

DELETE the rows    ledger 1230 rows, census 1222    <- nothing owed, green
KEEP them          ledger 1236 rows, census 1228   needs a log per gated major

Seeding is opt-in. Doing nothing and doing it thoroughly are both green; doing it partially is the only way to lose, and that is what has held this PR up. Nothing asks you to seed, and the gate's own printed recipe says <log> singular, which is what produced the single-major rows. That is #1071, not your mistake.

One-line check before pushing, which beats any recipe either of us can give you:

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u    # must print exactly ONE line

On the merits — nothing blocking

Your guard is broader than you claim. I attacked it through your own seam with a different catalog column and it still fires:

UPDATE pgcolumnar.row_group SET row_count = row_count + 4294967296 ...
SELECT count(*) FROM z WHERE <always-true predicate>
  -> XX001: columnar chunk for column 1 has a validity bitmap longer than the chunk

Worth one sentence in the suite header, because it is coverage you currently get by accident and a later narrowing would remove it silently.

Optional, not a blocker: the validityBytes < 0 clamp is the one asymmetry — an impossible value is silently accepted where the over-long bitmap refuses. Reachable through the same seam (row_count above ~1.7e10 makes it negative as an int), and it escapes as a generic XX000 from smgr rather than your typed XX001. No crash, no wrong row, backend alive — so it is a diagnosis-quality issue, not a hole. One ereport keeps the answer in the layer that knows it.

Cross-PR note: #1077 adds an inline cc->pageLength >= validityBytes check on the same invariant, and skips rather than refusing. If both land, that invariant has two policies in two places. Worth one of you deciding which, rather than discovering it later.

[linuxhikerpm]

Rebased onto current main (50b3225) and pushed ac3a731. Not merging, not self-approving.

What was red on the previous head (9b23f1f)

suites (PG 17) failed; PG 18 was green. Quoted:

6 check(s) the ledger has never seen
PG17 has a check the ledger has never seen, which is not a pass
new this run=6

Those six were native_chunk_length_bound rows with majors=18 only.

What changed

• Rebase onto origin/main (TESTS.md section 43, cluster_tests 411, census 1228).
• Ran native_chunk_length_bound.sh on PGDG 15.19 / 16.15 / 17.11 and Ubuntu 18.6 (all 6 passed when not racing another PR's .so into the same pkglibdir). Merged those logs. New rows carry 15;16;17;18.
• Pytest twin green on PG18 (6 pass).

What still blocks someone else from approving

• CI on this new head has not finished. Wait for suites (PG 17) on ac3a731.
• New rows are 15;16;17;18, not …;19. PG 19 is not installed here.

[jdatcmd]

Your rows carry 15;16;17;18 and the release gate runs PG19. CI is green and
will stay green; this is about the gate that runs before a tag, and jd is tagging
alpha4 tomorrow.

.github/workflows/ci.yml:503 says it plainly:

Run the suites on the current majors (17 + 18). This is the per-PR behaviour
gate; the full packaged matrix (15-18) runs nightly, and the local five-major
matrix (adding PG19) remains the release gate.

So nothing in CI or nightly ever exercises PG19's suites, and four majors passes
both. The five-major local matrix does exercise it.

What it would do, from the gate's own rule

pgc_ledger.py, cmd_gate:

if key[0] not in covered_suites or key[3] not in covered_majors:
    continue
if key[3] not in rows.get((key[0], key[1], key[2]), [set()])[0]:
    unknown.append(key)

with the comment above it: "A row is a claim about WHERE the check exists, so a
known check seen on a major its row does not name is refused too."

covered_majors is the union over all rows, and 1230 rows carry
15;16;17;18;19, so 19 is covered. Your 6 rows do not name it. Reproduced
against your branch's actual ledger:

rows                          1236
covered_majors                15,16,17,18,19
rows missing 19               6
PG19 leg would refuse         6 checks

The fix is one more run

Merge a PG19 log into the same ledger so those rows read 15;16;17;18;19. You
clearly have the matrix already, since you produced four.

And a correction to what I told you earlier

I said you could go green by DELETING the rows. That was right for a suite nobody
registers. These PRs register the new suite in run_all_versions.sh, which
puts it in the registered population the coverage ceiling is measured against,
so deleting the rows is a different question and I am no longer confident it is
open to you. I could not reproduce the ceiling arithmetic cleanly enough to say
either way, so treat the earlier advice as withdrawn rather than as an alternative.

The PG19 run is the path I can show works.

Nothing else on any of these five from me. The change itself I have not reviewed
yet; this is the bookkeeping that would bite tomorrow.

[OffgridwithJD]

Correction: ignore the "delete the rows" half of my last comment. It is wrong and it would fail the gate.

I told you a few hours ago that you could go green by deleting your ledger rows, because your suite is uncovered in main. Do not do that. I checked one arm of the gate and drew a conclusion about the gate. Here is the measurement I should have taken first.

Why deleting the rows fails

pgc_ledger.py computes coverage debt as registered − suites_that_have_rows, and suites_not_covered is a hard ceiling. Main sits exactly on it, with no headroom:

registered(main) = 253      (the SUITES array in run_all_versions.sh)
covered(main)    =   4      (differential, harness_selftest,
                             native_join_runtime_filter, native_join_vector_agg)
uncovered(main)  = 249      ceiling = 249

Your PR adds a suite and registers it, so registered becomes 254. That leaves exactly two outcomes:

KEEP your rows     254 registered, 5 covered  -> uncovered 249 = ceiling   PASSES
DELETE your rows   254 registered, 4 covered  -> uncovered 250 > ceiling   FAILS

What I said that was true: the new-check refusal genuinely cannot fire on an uncovered suite. What I missed: that refusal is not the binding constraint here. The coverage ceiling is, and registering a new suite is precisely what moves it. Seeding is not optional for a PR that registers a new suite. It is optional only for a suite already in the tree, which yours is not.

The second problem, which is the one that matters for the alpha4 tag

Your rows now read 15;16;17;18. Every one of main's 1230 rows reads 15;16;17;18;19.

covered_majors is the union of that field over all rows, so it includes 19. The gate refuses a known check seen on a major its own row does not name — deliberately, because widening a row's claim is an edit a reviewer should see rather than something a run does silently. ci.yml:503 says the per-PR gate runs 17+18, nightly runs 15-18, and the local five-major matrix adding PG19 remains the release gate.

So your CI is green and will stay green, and the release gate refuses 6 checks from this PR. Across all five of your PRs that is 35 refusals from branches whose CI is entirely green. With the tag going out tomorrow, whoever runs that gate hits this.

What to do

1. Keep your ledger rows. Do not delete them.

2. Make every row name all five majors: 15;16;17;18;19, matching every other row in the file.

3. Verify before pushing — this must print exactly one line, and it must be 15;16;17;18;19:

   awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u

The honest way to reach step 2 is a run on PG19 rather than a hand-edit, because a row is a claim about where the check was actually observed. I have PG19 on my container and I am running your suite there now. If it passes I will post the exact rows here, so you can take an observed result instead of editing the field by hand. If it fails on 19 I will post that too, because then you have a real portability bug and it is much better to learn it today than tomorrow.

Sorry for the wasted cycle — the first advice was mine and it was wrong.

[OffgridwithJD]

Your suite passes on PG19. Here is the observed result, and the one-line fix.

I ran test/native_chunk_length_bound.sh at your PR head against PostgreSQL 19beta2 (/usr/local/pg19a, assert build) on my container:

checks run: 6
checks unrunnable: 0
accounting: 6 passed + 0 failed + 0 unrunnable + 0 skipped = 6
native_chunk_length_bound.sh: PASSED

6 for 6, no portability problem on 19. So this is purely a ledger edit, not a code change.

Your budget numbers are already right

Worth saying plainly, because my earlier comment may have implied otherwise: check_ledger_budget.txt in your branch is correct as committed.

suites_not_covered        249
checks_never_observed_red 1228

Both are right. The only wrong thing in this PR is field 4 of your own rows.

The fix

I merged the PG19 log into your ledger with the repo's own tool:

python3 test/pgc_ledger.py merge --ledger test/check_ledger.tsv --date 2026-09-16 <pg19 log>
  ledger: rows=1236 | runs=1, distinct checks this merge=6, observed red ever=8, never=1228
    majors: uniform, all 1236 rows carry 15;16;17;18;19

It changed nothing except widening your 6 rows from 15;16;17;18 to 15;16;17;18;19. If you have no PG19 to hand, this is equivalent and I have verified it produces a byte-identical result:

awk -F'\t' 'BEGIN{OFS="\t"} $1=="native_chunk_length_bound"{$4="15;16;17;18;19"} 1' \
    test/check_ledger.tsv > /tmp/l && mv /tmp/l test/check_ledger.tsv

Then confirm, which must print exactly one line:

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u
15;16;17;18;19

Proof it clears the release gate

Same tool, same branch, after the merge:

ledger census: rows=1236 | never observed red=1228, ever red=8, new this run=0
ledger majors: covered=15, 16, 17, 18, 19 | this run observed 19
  census stated 1228, ledger holds 1228: they agree
ledger coverage: registered=254 | covered=5, not covered=249, ceiling=249
GATE rc=0

new this run=0 and not covered=249 against a ceiling of 249 — it sits exactly on the ceiling, which is the pass. That is the whole fix for this PR's gate story.

[jdatcmd]

Cross-reference: I raised a composition finding on #1077 that involves this PR's guard. Short version, both verified on the merge-tree of the two heads:

The policy conflict @OffgridwithJD raised resolves in your favour. #1077's inline pageLength >= validityBytes test is a decline-to-COALESCE, not a decline-to-read. A chunk that fails it falls back to the ordinary path, where your pgcolumnar_chunk_value_bytes fires with the typed XX001. One observable behaviour, and it is yours.

But #1077 copies validityBytes bytes out of the span before that test runs, and the span is sized from page_length. In the page_length < validityBytes direction -- the exact state your helper raises on -- that copy can run past the allocation when the chunk is last in its span.

Your suite poisons page_length upward. Poisoning it DOWNWARD, on a chunk that coalesces with a neighbour, would exercise it. Detail is on #1077; nothing here needs changing.

[jdatcmd]

Heads up before tomorrow: this now conflicts, on test/pytest/TESTS.md and
nothing else. Not your doing — #1086 and #1087 merged tonight and #1086 added
section 43.

conflicting files    test/pytest/TESTS.md
everything else      auto-merges clean

All five of your PRs claim section 43, so they collide with main and with each
other, not just with the merge that caused it.

test_the_contents_list_is_numbered_in_order requires the contents list AND the
sections to each count 1..N with no gap, so a duplicate or a hole reddens. That
means the number is decided by merge ORDER, and whoever lands second renumbers
again.

Suggested assignment

Taking @OffgridwithJD's recommended order, which exists because #1077's fix falls
through to the cast #1063 repairs:

#1063   section 44    land first
#1039   section 45
#1068   section 46
#1065   renumber when the parallel_leader_participation gap is closed
#1077   renumber when the overread hoist lands, and rebase onto #1063

This one is section 44 if the order above holds (first).

This is #996, and it has now bitten five PRs at once

#996 says every changelog entry inserts at the same anchor so any two conflict.
The same is true of TESTS.md section numbers, and tonight is the clearest
evidence yet: one merge invalidated the number in five independent branches. Worth
linking there rather than treating it as bad luck.

Nothing in your change is affected. The rebase is mechanical and the number is the
only decision in it.

[OffgridwithJD]

Your full path to green, run end to end on my side first

Main has moved twice since you last pushed (#1086 at a87c16f, #1087 at fcfd3e6, #1089 at cfdb393), and one of those merges collides with your branch. Rather than describe the fix, I did it on a copy of your branch and checked every step. Every number below is from that run.

This is also the PR that should land first of your five — #1077's fix falls through to the uint32 cast that your change repairs, so landing them the other way round leaves a window where an index fetch answers with a raw allocator error instead of your typed XX001.

Step 1 — merge main

git fetch upstream main && git merge upstream/main

Exactly one file conflicts: test/pytest/TESTS.md. Nothing else in your branch does. Two hunks, the contents entry and the section body.

The cause is not textual. #1086 took section 43, which is the number your branch also took, and the numbering is gated — test_the_contents_list_is_numbered_in_order requires both the contents list and the sections to count 1..N with no gap or inversion. So the number is decided by merge order, and the second to land renumbers. This is not your mistake, and all five of your PRs have it.

Step 2 — resolve by renumbering yours to 44

Keep main's 43 (test_pgxn_metadata.py) and make yours 44. Three strings change, and the anchor is the one people miss:

- [43. test_native_chunk_length_bound.py: ...](#43-test_native_chunk_length_boundpy-...)
+ [44. test_native_chunk_length_bound.py: ...](#44-test_native_chunk_length_boundpy-...)

- ## 43. test_native_chunk_length_bound.py: a truncated chunk length cannot fetch
+ ## 44. test_native_chunk_length_bound.py: a truncated chunk length cannot fetch

Verify before moving on — both sequences must be contiguous, and the arm checks both:

grep -oE '^## [0-9]+\.' test/pytest/TESTS.md | tr -d '#. ' | sort -n | uniq -c | awk '$1>1'

Measured after my resolution: sections=44 anomalies=0, toc=44 anomalies=0.

44 is yours because you land first. If the order changes, so does the number.

Step 3 — make the ledger name PG19

Your rows still read 15;16;17;18. Every row in main reads 15;16;17;18;19, and the release gate is the local five-major matrix including PG19, so those six rows are refused there while your CI stays green.

I ran your suite on PG19 (19beta2) and it passes 6/6, no failures, nothing unrunnable. So this is a ledger edit, not a code problem. If you have a PG19 to hand, merging its log is the honest route and produces exactly this:

python3 test/pgc_ledger.py merge --ledger test/check_ledger.tsv --date <today> <pg19 log>
  ledger: rows=1236 | runs=1, distinct checks this merge=6, observed red ever=8, never=1228
    majors: uniform, all 1236 rows carry 15;16;17;18;19

If you have no PG19, this is equivalent and I verified it gives a byte-identical result:

awk -F'\t' 'BEGIN{OFS="\t"} $1=="native_chunk_length_bound"{$4="15;16;17;18;19"} 1' \
    test/check_ledger.tsv > /tmp/l && mv /tmp/l test/check_ledger.tsv

Then this must print exactly one line:

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u
15;16;17;18;19

Your check_ledger_budget.txt is already correct and needs no edit — suites_not_covered 249, checks_never_observed_red 1228. Both right as committed.

Step 4 — the gate, on the merged tree

ledger census: rows=1236 | never observed red=1228, ever red=8, new this run=0
ledger majors: covered=15, 16, 17, 18, 19 | this run observed 19
  census stated 1228, ledger holds 1228: they agree
ledger coverage: registered=254 | covered=5, not covered=249, ceiling=249
GATE rc=0

not covered=249 against a ceiling of 249 is the pass — it sits exactly on it, which is why deleting your rows would fail and why seeding was never optional for this PR.

What I did not verify

I have not re-run native_chunk_length_bound.sh against the merged tree. I have it green on PG19 at your current head (6/6) and the gate green after the merge, but the suite against main's newer code is unrun — the container is busy with a full five-major matrix right now. I will run it and report, rather than let this recipe imply more than I measured.

Nothing in the merits review changes: the C is complete, both cast sites are fixed and they are the only two in the tree.

[linuxhikerpm]

CI diagnosis (from job logs, not PR comments)

Red check on the failing SHA (9b23f1f): suites (PG 17). Quoted from the PG17 job log:

not in the ledger: native_chunk_length_bound	native_chunk_length_bound	a sequential scan of the same poisoned chunk is refused (XX001)	(on major 17)
not in the ledger: native_chunk_length_bound	native_chunk_length_bound	an index fetch of a chunk whose page_length is 2^32 too large is refused (XX001)	(on major 17)
not in the ledger: native_chunk_length_bound	native_chunk_length_bound	backend survived the sequential refusal	(on major 17)
not in the ledger: native_chunk_length_bound	native_chunk_length_bound	backend survived the truncated-length fetch	(on major 17)
not in the ledger: native_chunk_length_bound	native_chunk_length_bound	premise: a point lookup uses the index, not a sequential columnar scan	(on major 17)
not in the ledger: native_chunk_length_bound	native_chunk_length_bound	premise: that fetch returns the row	(on major 17)
    6 check(s) the ledger has never seen.
  PG17 has a check the ledger has never seen, which is not a pass
    ledger integrity failure: classification lost rows: matched 6 + orphans 0 + unprunable 0 + not checked 1234 != 1234 ledger rows -- every row must land in exactly one of the four

No suite =FAIL. Integrity failure is the unseen-check double-count.

Class: honest PG18-only ledger seed and needs-rebase vs fcfd3e6. Did not fake majors 15–17.

Pushed: rebased audit/chunk-length-uint32 onto fcfd3e6. TESTS.md §43 kept as test_pgxn_metadata.py; this twin is §44. Collection 346 / 411. Ledger never-census 1228 matches budget. suites_not_covered 249. SUITES C-locale sorted. Numbering tests green locally. New HEAD 0d8bd0aada7624efde1b12c3e043ca785e935ca8. Mergeable now; CI pending. Not merged, not approved.

[OffgridwithJD]

Reviewed in full, found legitimate, and prepared for merge as #1092

Your work here is sound and I want to say that before the mechanics: the C is complete and I verified it rather than reading it. columnar_reader.c:2721 and :4369 are the only two places in the tree that make that cast, and both are fixed. Land this one first of your five — #1077's fix falls through to the very cast this repairs.

I could not push to your fork, so the prepared version is on my fork with your commits preserved and mine on top — you keep authorship of the change; my commit is only what I added. If you would rather land it from here instead, take the patch from #1092 and push it to this branch, and I will close mine.

What I changed

Only the ledger. Nothing else needed touching.

I also attacked your guard through its own seam with a different catalog column and it still fires, so it is broader than the PR claims: poisoning row_group.row_count upward also raises XX001. Worth a sentence in the suite header, because that is coverage you currently get by accident.

The ledger rows are re-derived from runs on all five majors, not edited. Your rows read 15;16;17;18 while every other row in the file reads 15;16;17;18;19, so covered_majors includes 19 and the release gate refuses this suite's checks — even though CI stays green, because ci.yml:503 runs 17+18 per PR and only the local five-major matrix adds 19. That is #1071's trap, not a mistake of yours: the gate's own printed recipe says <log> singular.

I widened them by running the suite on PG15/16/17/18/19 and merging those logs, because a row is a claim about where a check was observed and editing field 4 makes that claim without the observation.

One thing worth knowing for next time

All five of your PRs take TESTS.md section 44, and only one can. The numbering is gated — test_the_contents_list_is_numbered_in_order requires 1..N with no gap — so the number is decided by merge order and the second to land renumbers. Worse, the census in check_ledger_budget.txt cannot be resolved when two of these merge: the conflict offers two numbers and neither is right, because each is short by exactly the other PR's row count. It has to be re-derived from the merged ledger.

That is a property of the repository, not of your work — measured and written up on #996.

Nothing here needed a change to suites_not_covered. Your budget files were correct as committed.

[jdatcmd]

Thanks for this. The C is correct and I verified it rather than reading it: columnar_reader.c casts (uint32) (pageLength - validityBytes) in exactly two places and this fixes both. The helper's two ereports are the right shape and the SQLSTATE is right.

The ledger rows name four majors, and the release gate runs five

Every row this PR adds reads 15;16;17;18. Every other row in the file reads
15;16;17;18;19. Adding rows makes this suite covered, and the gate then
refuses a covered suite's check when it is observed on a major the row does not
name.

CI cannot see this. ci.yml:503 runs PG17 and PG18 per pull request, and only
the local five-major matrix adds PG19. That is why this PR is green on 14 of 14
checks and still cannot pass the release gate.

Measured, not argued. I ran pgc_ledger.py gate twice with the same PG19
observation of this PR's own check, changing only the ledger:

A) this PR's ledger      (majors 15;16;17;18)
   not in the ledger: ... backend survived the sequential refusal (on major 19)
   1 check(s) the ledger has never seen
   rc=1   REFUSED

B) the rebased ledger    (majors 15;16;17;18;19), identical log
   census stated 1228, ledger holds 1228: they agree
   rc=0   PASSES

One variable, opposite results.

Disposition

Closing in favour of #1092, which carries your commits unchanged (I confirmed each is an ancestor of that branch) and corrects only the ledger. Your authorship is preserved there. Nothing about your C changed.