fix: share table-AM parallel scan groups across workers (#1068, rebased + ledger)

[OffgridwithJD]

This is @linuxhikerpm's #1068, rebased onto cfdb393 with its ledger rows re-derived. Their commits are preserved; mine is the last one. Opened from my fork because I cannot push to theirs.

What I reviewed and found legitimate

This is the strongest of the five on the merits. I did not take the parallel index build on trust — I built one against a 3M-row columnar table and watched it:

• four workers confirmed live in pg_stat_activity during the build
• the resulting index complete: every row present, no duplicates, no gaps
• rescan safe under repeated execution

It works.

What I changed

Only the ledger. Their rows read 15;16;17;18 while every other row reads 15;16;17;18;19, so covered_majors includes 19 and the release gate refuses this suite's ten checks — while CI stays green, because ci.yml:503 runs 17+18 per PR and only the local five-major matrix adds 19.

Re-derived from runs, not edited.

parallel_am_scan on PG15/16/17/18/19:  10/10 each, rc=0
ledger 1240 rows, uniform 15;16;17;18;19, census 1232
GATE rc=0   registered=254 covered=5 not-covered=249 ceiling=249

Open, not blocking

The parallel index build has no arm. It is the substance of this PR and nothing in the suite would notice if it stopped working — today it is proven by my run, and my run is not in the repo, so it decays the moment either of us stops looking.

An arm need not be elaborate: build an index on a table large enough to recruit workers, then assert the index is complete — the row count through the index matches the row count through a sequential scan. That is the property that matters, because a parallel build that loses rows is the silent failure.

The stronger version also pins that workers were recruited, so the arm cannot pass by quietly building serially — otherwise a change that disables the parallel path leaves it green.

I did not add this myself: it needs a fixture decision (how large, how many workers) that belongs with the author rather than with a reviewer guessing.

Merge order

Takes TESTS.md section 44, correct while main is at 43. All five take 44; the numbering is gated, so the second to land renumbers and re-derives its census — the conflict offers two numbers and neither is right (#996).

Suggested order: #1092 first, then #1095, this, #1094, #1093.

Original: #1068. Author: @linuxhikerpm.

[jdatcmd]

Approving. This is the strongest of the five on the merits.

Replacing the first-wins phs_nallocated claim with a per-group claim is the right fix. The old code had one participant take the whole scan and the rest see it exhausted, which was correct but left every launched worker idle. Claiming per group from the same atomic counter is how the custom scan already does it, so the two paths now agree.

Widening gi to uint64 belongs with it: pg_atomic_fetch_add_u64 returns uint64 and the old uint32 would have wrapped the claim counter on a long scan.

The three stale comments that described the single-participant behaviour are all updated, including the one in pgcolumnar_index_build_range_scan, which is the one that would have misled someone debugging a parallel index build.

The ledger correction is load-bearing, proved as on #1092.

Commit preservation checked: every commit from #1068 is an ancestor of this branch.

[jdatcmd]

Re-approving at a45316e, now covering the index-build arm. My earlier approval was scoped to the scan-sharing C and the ledger, and I said it would not transfer to new code, so this is the review of the arm itself.

The C survived the rebase unchanged

Per-file patch id, approved head 377a17f against this one:

src/columnar_reader.c    SAME b1d21867f9ee
src/columnar_tableam.c   SAME 36379e7ee60a

So nothing rode in under the bookkeeping.

The vacuous pass was real, and the fix closes it

I reproduced the defect and the repair with the PR's exact expansions rather than taking the run:

OLD form, both reads empty:
  got=[] want=[]  -> PASS          <- the headline lied

NEW form:
  both-empty    got=[<the index read returned nothing>]
                want=[<the sequential read returned nothing>]   -> FAIL
  idx-empty     got=[<the index read returned nothing>]
                want=[50000|1250025000]                          -> FAIL
  mismatch      got=[25000|612512500] want=[50000|1250025000]    -> FAIL
  both-present  got=[50000|1250025000] want=[50000|1250025000]   -> PASS

The sentinels are distinct strings, so they cannot collide on the both-empty path, which is the one that reported PASS before. The pytest twin carries the same shape at lines 262 to 263, so both harnesses fail closed rather than one.

This is the most valuable change in the PR. A headline check that reports PASS having measured nothing is worse than no check, and it was only visible under a mutation that made the build fail rather than return wrong data.

Scoping the claim honestly is the right call

Agreed that the arm tests completeness, not distribution, and that saying so is better than implying more. Entering the parallel path is not claiming groups: pre-#1068 every participant would still enter and all but one would find the scan exhausted, so a PID count proves nothing about sharing.

Distribution is already asserted, on the scan path, by this suite's existing workers share the table-AM scan, it is not a single claimer, which reads per-worker rows= and correctly treats a missing line as no measurement rather than a zero. The index build adds the correctness risk the shared counter introduces, which is the half that was uncovered.

The aggregate rather than a bare count is right: a group read twice and a group skipped cancel in count(*) and do not cancel in sum(id).

Bookkeeping, checked rather than taken

TESTS.md            max 46, 0 anomalies in the 1..46 sequence
ledger              budget 1249 == actual never-red 1249
majors on new rows  15;16;17;18;19, uniform
cluster_tests       413 -> 415, consistent with the twin's added tests
COMPLETE            union preserved, none of main's entries dropped

The union check earned its place twice on this rebase, recovering both native_chunk_length_bound and native_fetch_coalesce.

The relpages note

Thank you for putting plan_create_index_workers, the 69-pages-for-2,000,000-rows measurement, and "the GUC is a gate, the reloption is the dial" in the arm's comment. That is the thing someone debugging a one-worker build reaches for a bigger table over.

Merging once CI is green.