refactor: columnar_metadata gets its own header, the first of six (#496)

[ChronicallyJD]

First of six, for #496. columnar_metadata.c's 22 single-consumer declarations move to src/columnar_metadata.h beside it.

The census, re-measured, because the issue's numbers were stale

	when filed	today (3e86a7e)
columnar.h lines	957	1045
extern declarations	159	164
single-consumer functions	79	84

The header gained interface faster than #527 removed it. Of 137 function declarations, 84 have exactly one consumer outside the defining file and 49 are genuinely shared.

What moved, and what did not

The 22 functions defined in columnar_metadata.c and used by exactly one other file. The shared vocabulary they take — the Native*Metadata structs, the GUCs, the format constants — stays in columnar.h, which the new header includes. Nothing is made static, so #527's disqualification does not apply here: this relocates declarations, it does not narrow linkage.

Verified mechanically: each of the 22 appears exactly once in the new header and zero times in columnar.h.

A defect in my own measurement, since it decided which module went first

The first version of my census attributed PgColumnarBloomBuild and PgColumnarEncodeChunk to columnar_write_state.c, which defines neither. It picked whichever file matched first in directory order. Requiring a definition at column 0 — the style this tree uses, return type on its own line — changed the ranking:

module	corrected	broken tool said
columnar_metadata.c	22	outside the top three
columnar_reader.c	12	20
columnar_write_state.c	11	25, ranked first

The wrong module would have been extracted first. It was caught only because PgColumnarBloomBuild living in columnar_write_state.c is absurd on sight; two plausible filenames and I would have believed it.

Gate

major	build	warnings + errors
pg15a	rc=0	0
pg16a	rc=0	0
pg17a	rc=0	0
pg18a	rc=0	0
pg19a	rc=0	0

make clean between each, per #536. Plus native_vecdecode 24/24 and harness_selftest 54/54 on pg18a.

The five-major build is the gate that matters here and I want to be explicit about why rather than pad the list: nothing in this diff alters a line of executable code. The failure mode of a declaration move is a consumer left without a declaration it needs, and that fails to compile — loudly, on whichever major first sees it — rather than misbehaving at run time. Zero occurrences of error: or warning: across five full builds is the direct evidence for that, and a suite run is corroboration rather than the proof.

Remaining

reader (12), write_state (11), storage (10), customscan (6), delete_vector (6) — one PR each. 84 declarations in a single diff is not a reviewable change, and each module's consumers are a different set of files.

I have not merged this and will not.

[ChronicallyJD]

Reviewed by checking the claims rather than reading the diff. The split is
clean and I have no objection to its content. One structural finding, and it is
about #546 and #547 together rather than either alone.

Verified

Every new include is needed. For each .c that gained
#include "columnar_metadata.h", counted how many of the header's 22 symbols it
actually references:

columnar_metadata.c   22    columnar_vacuum.c        7
columnar_write_state.c 6    columnar_reader.c        3
columnar_projection.c  2    columnar_tableam.c       2
columnar_vector.c      1    columnar_visibilitymap.c 1

No include added that nothing uses, which is the usual way a mechanical split
adds churn.

The stated rationale holds, for all 22. The header says every declaration had
exactly one consumer outside its defining file. Counted per symbol across
src/*.c excluding columnar_metadata.c: not one of the 22 has more than one
consumer. That is a checkable claim and it checks out, which is worth saying
because it is the entire justification for the split.

The finding: #546 and #547 are not stacked, and they collide

Both are titled as a chain — "the first of six", "the second of six" — but both
are single commits based on main, and neither contains the other.

Trial-merged them onto main in sequence:

merge 546 -> clean
merge 547 -> CONFLICT in src/columnar_reader.c
                        src/columnar_tableam.c
                        src/columnar_vector.c

Not columnar.h, as I first assumed — the removed hunks there are far enough
apart to auto-merge. It is the #include lines, added at the same place in the
consumers both PRs touch.

With six planned this compounds: each pair that shares a consumer collides the
same way, and the conflicts land in .c files rather than in the header being
split, so they read as unrelated to the change.

Either stack them (547 based on 546) or expect to rebase each one after the
previous merges. I have no view on which — I hit the auto-close trap stacking
#534 this morning and would not push anyone toward stacking without saying so.

Not a finding

A pure header split has no behaviour to assert, so "it builds and the suites
pass" is the right standard and CI covers it. I did not ask for a test.