refactor: the last four module headers, in one change (#496)

[ChronicallyJD]

The last four modules for #496: write_state (11), storage (10), customscan (6), delete_vector (6) — the 33 remaining declarations with exactly one consumer outside their defining file.

One PR rather than four, and why

Because of how you reviewed #546 and #547: by running checks — every new include used, no moved symbol having more than one consumer — rather than by reading declarations. When correctness is established by script, four diffs are no easier to review than one.

And it collapses the collision problem. Every pair of module PRs sharing a consumer conflicts on the #include line, so a six-PR series is up to fifteen rebases as they land one at a time. Three PRs is one.

Not stacked, for the reason you laid out rather than recommended: a stacked PR auto-closes when its base is deleted on merge, which is what happened to #534. Merge #546, #547 and this in any order; whichever lands after another wants a one-minute rebase of include lines. I will do them.

A defect in my splitting tool, found by an assert

Worth recording because it produced a wrong module assignment rather than an error. columnar.h:353 is

extern void PgColumnarEnsureStorageRow(Relation rel);	/* pre-create storage row (#300 parallel_copy) */

That line ends in */, not ;. My block scanner ran to the next line ending in ; and swallowed the declaration beneath it, filing PgColumnarReserveRowNumbers — a storage symbol — into write_state.h.

It compiles either way, because a declaration is visible from whichever header its consumer includes. Nothing about the build would have told anyone. The matched 9 of 10 count assert on the next module is what caught it, one module after the damage.

Your "15 of 15 symbols in reader.h" against my stated 12 is what sent me to check the open PRs, and they are clean: #546 and #547 carry exactly their intended sets — 22 and 12, no extra symbol, none missing, none stranded in columnar.h. That is luck rather than care. Neither module's declarations happened to sit beside one carrying a trailing comment.

Terminator detection now strips comments before testing for ;.

Verification

module	moved	extra	missing	left in columnar.h
write_state	11/11	none	none	none
storage	10/10	none	none	none
customscan	6/6	none	none	none
delete_vector	6/6	none	none	none

columnar.h: 1045 → 997 lines, 164 → 131 externs.

Gate

major	warnings + errors
pg15a	0
pg16a	0
pg17a	0
pg18a	0
pg19a	0

make clean between each (#536). native_vecdecode 24/24, harness_selftest 54/54, and replication 41/41 — the last included deliberately, since #548/#549 are live in that file and columnar_storage.h now sits under it.

Note for #549

columnar_customscan.c uses 8 of reader.h's symbols, which you flagged as the largest cross-file dependency. It survives this split unchanged: customscan's own declarations move to customscan.h, and its use of reader symbols is unaffected, so the pair you were worried about does not need taking early.

After this, 84 of 84 single-consumer declarations are placed. What remains for #496 is a judgement call rather than a move: 47 shared declarations stay in columnar.h as the actual interface, and I would leave that alone unless someone wants it argued.

I have not merged this and will not.

[ChronicallyJD]

Reviewed at ac937b5 the same way as #546 and #547 — checking the claims rather
than reading the diff. Clean. No findings against the change itself.

Verified

Every include is used. For each .c that gained one of the four new headers,
counted how many of that header's symbols it actually references:

customscan.h (7)     customscan.c 7   vector.c 5   tableam.c 2
delete_vector.h (6)  delete_vector.c 6  tableam.c 5  reader.c 1
storage.h (10)       storage.c 10  vacuum.c 4  write_state.c 3  tableam.c 2  reader.c 1
write_state.h (13)   write_state.c 13  tableam.c 7  vacuum.c 6  projection.c 2  parallel_copy.c 1

Nothing gained an include for symbols it does not use.

The one-consumer rationale holds for all 30 function declarations.

customscan.h      5 declarations, none with more than one consumer
delete_vector.h   6 declarations, none with more than one consumer
storage.h        10 declarations, none with more than one consumer
write_state.h     9 declarations, none with more than one consumer

Worth recording how I nearly filed three false findings against this. My first
pass matched any PgColumnar[A-Za-z]+ token and reported PgColumnarGroupStats
(2 consumers), PgColumnarWriteState (2) and PgColumnarProjection (5) as
violations. All three are types, declared in columnar.h and appearing in
these headers only as parameter and return types — which is exactly what the
header comments say stays behind as shared vocabulary. The check was matching
type references as declarations. Restricted to ^extern ... Name(, the claim is
clean.

Structural, already raised and unchanged

#550 conflicts after #546 in the cumulative merge:

#546 ok
#547 CONFLICT  columnar_reader.c columnar_tableam.c columnar_vector.c
#550 CONFLICT  columnar.h columnar_projection.c columnar_reader.c
               columnar_tableam.c columnar_vacuum.c columnar_vector.c
               columnar_write_state.c

Same include-placement collision as #546/#547, wider because this one touches
seven consumers. It merges cleanly onto main alone, so this is about ordering
and rebases, not about the change. Detail on #546; nothing new to say here.

Not reviewed

I have not re-run the suites against this branch — CI is green on all eleven legs
and a header split has no behaviour to assert beyond building and passing, which
is the standard I applied to #546 and #547.