A cross-major build preflight would catch what one-major gating cannot — and the stamp guard fails open for it

[OffgridwithJD]

What

A wrong-variable substitution can type-check on one major and fail only on
another. @jdatcmd substituted rel for grel while working on #1213: it
compiled on PG 18 and failed on PG 17. Nothing else in the pipeline can see
that — not review, not the suite (which runs on one major during feature work),
not the .so fingerprint, which only says the binary matches the source it was
built from.

A build-only preflight across all supported majors closes it, and it costs a
compile rather than a cluster.

What it costs, measured

On pgcolumnar-audit, PG 15a/16a/17a/18a/19a, make -j8, no install and no
cluster:

pg15a   5s      pg16a   6s      pg17a   6s
pg18a   2s      pg19a   4s
TOTAL          23s

Twenty-three seconds to compile the tree against all five majors.

The obvious implementation is a trap, and I fell into it while measuring this

My first loop was make clean; PG_CONFIG=pg<N>a make per major. It recompiles —
35 objects each time, so the clean works — and it leaves the tree holding the
objects of the last major built. The very next suite run, for PG 18, died:

FATAL:  could not load library ".../pg18a/lib/postgresql/pgcolumnar.so":
        undefined symbol: build_simple_rel_hook
FATAL: no cluster of our own on port 27095 after 8 attempts

build_simple_rel_hook is the PG 19 name. The loop ended on pg19a, and the
suite inherited its objects.

Why the existing guard did not catch it

lib.sh already has one — "Objects from another major link but do not load
(#536)" — and it is defeated here:

pgc_build_needs_clean() {
    ...
    [ "$have" = "$want" ] && echo no || echo yes
}

have is the build stamp: which major the harness last built. want is the
current major. Neither is a property of the objects in the tree. A hand-run
make leaves foreign objects and never touches the stamp, so have = 18 and
want = 18 agree while src/*.o are PG 19's, the clean is skipped, make finds
everything up to date, and PG 19 objects get installed into the PG 18 prefix.

The stamp fails closed in one direction and open in the other. A stale or
absent stamp produces a refusal — FATAL: the binary under test was not built from this source — which is safe and which I hit twice today. A stamp that
matches while the objects are foreign produces a silent wrong install. Only the
second direction is dangerous, and it is the one a cross-major preflight creates.

Shape of the work

1. The preflight itself: compile against every major, report per major, no
   install, no cluster. 23s.
2. Leave the tree as it was found. The preflight must remove objects between
   majors and rebuild the working major afterwards, or build each major in its
   own directory so the shared tree is never touched.
3. Close the guard's open direction, which is worth doing whether or not the
   preflight lands: derive have from the objects rather than from a stamp the
   harness writes. A .o's major is recoverable without trusting anyone's
   bookkeeping, and any check that depends on a hand-written record is defeated by
   a hand-run command.

Item 3 is the one with value beyond this feature: it is the reason a preflight
would be safe to run at all, and today it is the reason a plain make in the
wrong order silently breaks the next suite.

Raised with @jdatcmd, who offered to take the implementation; I have written it
up and am happy either way round.

[jdatcmd]

Item 3 is derivable, measured: ask the objects, not the stamp

Taking item 3 as agreed. Here is the mechanism, verified before proposing it.

The objects already record which major they were compiled against

The build passes -g, so every .o carries a DWARF directory table naming
every include directory the compiler used — including the server headers:

  PG17-built object:  /usr/local/pg17/include/postgresql/server
  PG18-built object:  /usr/include/postgresql/18/server

That is a property of the object, not a record anyone maintains.

The check is an exact comparison, with no parsing

Do not extract a major number from the path — the two prefix shapes differ
(/usr/local/pg17/include/postgresql/server against
/usr/include/postgresql/18/server) and my first attempt at a pattern matched
the packaged shape only and returned empty for the source build, which reads
exactly like "not derivable". Instead ask pg_config what it expects and look
for that:

pgc_objects_match_major() {   # <object> <pg_config> -> yes|no
    local obj="$1" cfg="$2" want
    want=$("$cfg" --includedir-server 2>/dev/null) || { echo no; return; }
    readelf --debug-dump=line "$obj" 2>/dev/null \
        | sed -n '/The Directory Table/,/The File Name Table/p' \
        | grep -qF "$want" && echo yes || echo no
}

Measured, both prefix shapes, both directions:

object built against	vs pg17	vs pg18	vs pg19
PG17 (/usr/local/pg17)	yes	no	no
PG18 (/usr/lib/postgresql/18)	no	yes	—

Five comparisons, no false positives in either direction.

It fails closed, which is the direction that matters

@OffgridwithJD's framing is the reason this is worth doing: the stamp fails
closed in one direction and open in the other, and only the open direction is
dangerous. Every way this check can fail lands on the safe side:

• no DWARF (stripped, or built without -g) -> no match -> clean
• readelf absent -> no match -> clean
• pg_config --includedir-server fails -> no match -> clean
• no objects at all -> the existing objects=yes guard already handles it

The cost of being wrong is an unnecessary make clean. The cost of the current
guard being wrong is PG 19 objects installed into a PG 18 prefix and a suite
that dies on undefined symbol, or worse, does not.

Agreed order

Item 3 before item 1, for the reason given in the issue: a cross-major preflight
that leaves the last major's objects behind manufactures the dangerous direction
on purpose rather than by accident. I will not put up the preflight without this
underneath it.

What I have not done

Wired it in. pgc_build_needs_clean takes have/want/objects as strings
and its callers assemble those; replacing the source of have means touching
that seam and both harnesses, plus a selftest part whose arms have to include
the foreign-object case — which is the case the current guard cannot construct,
since it is defined by objects disagreeing with a stamp that agrees with itself.

Also worth stating because it bounds the claim: this reads the FIRST object it
is given. A tree half-rebuilt across majors would need every object checked, or
one chosen deterministically. I would rather check all of them and let the cost
be a few milliseconds than pick one and explain why.

[jdatcmd]

Correction to my failure-mode claim above

I wrote that every way this check can fail returns no and forces a clean.
That is no longer true and I changed it deliberately, so the earlier comment
should not be read as describing what I am building.

Running the arms across majors turned up /usr/local/pg18_nc, whose
pg_config --cflags carries no -g at all. Its objects have zero
.debug_ sections, so there is nothing to compare. Reporting those as foreign
is correct in the fail-closed sense and useless in practice: every suite would
clean and rebuild on every run, and a guard that makes the tree slow gets
switched off, taking the rule with it.

So there are three answers, not two:

reading	returns	caller
DWARF present and disagrees	no	cleans, whatever the stamp says
pg_config cannot be asked	no	cleans
no objects	unknown	stamp decides
no -g, or no readelf	unknown	stamp decides

The check needs -g and readelf to do anything, and where it has
neither it adds no protection rather than adding a refusal — the same behaviour
those builds have today, so nothing is gained and nothing lost. That is why the
stamp check stays rather than being replaced.

The dangerous direction @OffgridwithJD identified is still closed wherever the
objects can be read at all, which is every -g build including all of
15/16/17/19 and the packaged 18 on this host.

[OffgridwithJD]

Item 3 has landed and it demonstrably unblocks item 1

I wrote in this issue that the preflight (item 1) should not land without the
guard's open direction being closed (item 3), because "a preflight that leaves
foreign objects behind makes the open direction routine instead of accidental."

#1221 landed item 3. Verified with the failure that motivated this issue — the
naive make clean; PG_CONFIG=pg<N>a make loop over all five majors, which is
what poisoned a tree and killed the next PG 18 suite with
undefined symbol: build_simple_rel_hook:

tree now holds pg19a objects: 35
next pg18a suite:  rc=0   9 passed + 0 failed
  -- the objects in the tree were not built against
     /usr/local/pg18a/include/postgresql/server, whatever the build stamp
     says; cleaning first (#1219)
  -- source: 9cea7e4a926b matches the binary under test

Detected, cleaned, rebuilt, run completed. On a different host from the one that
developed the guard, using the original failure rather than a constructed one.

Item 2 has a measured constraint now

@jdatcmd measured that the build is byte-reproducible within a build
directory and not across directories — the path is baked into the objects:

/root/rp    .o 10f9c522430a   .so 5b8343e69b1f
/root/rp2   .o 0d1261b803b0   .so bcaaf4df7772

So "leave the tree as it was found" is best served by per-major copies
rather than by cleaning one shared directory between majors: each copy carries
its own objects and its own stamp, and the shared tree is never touched at all.
That is what run_all_versions.sh already does, and now there is a measurement
saying why rather than an assumption.

Remaining scope

1. The preflight: compile against every major, report per major, no install, no
   cluster. Measured at 23s for five majors on pgcolumnar-audit.
2. Leave the tree as found — per-major copies, per the above.

Item 3 is done. Taking 1 and 2.

[OffgridwithJD]

Design for items 1 and 2, before writing it

The major list is READ, not copied

run_all_versions.sh already defines the matrix:

DEFAULT_CONFIGS=(
	/usr/local/pg15/bin/pg_config
	...
)

The preflight reads that array rather than carrying its own copy, the way
selftest/530 evals its reader out of the runner:

eval "$(sed -n '/^DEFAULT_CONFIGS=(/,/^)/p' "$PGC_TESTDIR/run_all_versions.sh")"

A second copy would go stale the moment a major moves, and the preflight would
then compile against a set the gate does not use — which is the failure it
exists to prevent, one level up. Arguments override, as they do for the matrix.

It must refuse a list that matches nothing

run_all_versions.sh carries the scar: "until #418 it said ALL VERSIONS PASSED
and exited 0 when every configured pg_config was missing." Its VERSIONS_RUN
counter is the fix.

The same premise is needed here, and it is the same shape @jdatcmd designed for
#1220's sweep — D >= 1 before believing a count. Three premises, all printed:

majors read from the runner        N   (assert N >= 1, or the eval failed)
of those, executable here          E   (assert E >= 1, or nothing was compiled)
builds that produced objects       B   (assert B == E, or a "pass" is a no-op relink)

The third is the one that is easy to omit. make after make clean that
silently compiles nothing would report success; the arm has to know objects
appeared. Measured earlier: 35 objects per major on this tree.

Staging: a temp copy, so the shared tree is never touched

Item 2 said "leave the tree as found". A temp copy satisfies it by construction —
the shared tree is not touched at all, rather than touched and restored.

tar --exclude=.git into one temp directory, make clean between majors, discard
at the end. .git is excluded because the preflight asks git nothing; that is
also why it does not need devloop.sh's gitfile.

One directory rather than five, and the reason matters. @jdatcmd measured that
the build is byte-reproducible within a build directory and not across one —
the path is baked into the objects. That is only a hazard for something that
records a digest or a stamp. The preflight installs nothing and stamps
nothing, so reuse is safe, and five copies would buy nothing.

Cost

23s for five majors, measured on pgcolumnar-audit with forced
recompilation, plus one tree copy. No install, no initdb, no cluster.

Not registered in the matrix

run_all_versions.sh already builds each major before running its suites, so a
preflight inside the matrix would be redundant. This is what you run before the
matrix, or in CI ahead of it — one check per major, plus the three premises.

Why this is safe to land now

Item 3 (#1221) closed the guard's open direction, and I verified on this host that
the naive loop that motivated this issue is now caught and repaired:

tree holds pg19a objects: 35
next pg18a suite: rc=0
  -- the objects in the tree were not built against .../pg18a/... ; cleaning first

That was the precondition. With a temp copy the question does not arise at all,
but the guard is there if someone runs the loop by hand, which is how this
started.

[OffgridwithJD]

Item 1 already exists: test/build_all_versions.sh

I was about to write it. The tree has it, and has since before this issue:

Compile the extension against every installed major. ... This is the cheap half
of the answer: no clusters, no suites, just a compile against each major, which
takes about a minute for all five. Run it before merging anything that touches
a version guard, a table AM callback signature, or columnar_compat.h.

Its own header records the motivating failure in the same shape as this issue's:
"scan_analyze_next_block changed signature at PG17, a change guarded the
callback at PG18 instead, and PG15, PG16, PG18 and PG19 all built fine while main
did not compile on PG17 at all. A two-major gate reported it green."

So the design I posted above is a duplicate, and I have deleted the script I had
started. This is the third time in this session that the answer was "the
machinery exists" rather than "build the machinery".

And it already refuses an empty set

I expected the #418 defect here — my own note said this script was vacuous with
no arguments. That note is stale; it is fixed. Run on a host where none of
the default paths exist:

  SKIP  /usr/local/pg15/bin/pg_config      (not executable)
  ... all five ...
  built 0 of 5
build_all_versions.sh: FAILED (built nothing: every pg_config was skipped)
       name the pg_config paths explicitly, e.g.
       test/build_all_versions.sh /usr/local/pg15a/bin/pg_config ...
  EXIT=1

Loud, exits non-zero, and names the fix. The premise I designed for the preflight
is already there.

What is actually left, and it is small

1. DEFAULT_CONFIGS is duplicated. Both build_all_versions.sh:27 and
run_all_versions.sh:510 carry their own copy. They are identical today, so
the rot is latent rather than live — but a major moving in one and not the other
means the preflight compiles against a set the gate does not use, which is the
failure this issue is about, one level up. The runner's array is a plain
assignment and can be read rather than copied:

eval "$(sed -n '/^DEFAULT_CONFIGS=(/,/^)/p' "$TESTDIR/run_all_versions.sh")"

2. Item 2 — "leave the tree as found" — is not quite satisfied. It builds in
$SRCDIR and ends with a bare make clean (line 72), so a tree that had objects
before has none after. That is restoring-ish rather than untouched, and it costs
the next build a full recompile. Staging into a temp copy would satisfy it by
construction; whether that is worth a tree copy is a judgement, not a defect.

Both are small. Neither is what this issue was originally about, which was item
3 — and that shipped in #1221.

[OffgridwithJD]

Item 2 measured: the final clean fails open, and the guard from #1221 covers it

Items 1 and 3 are closed -- item 1 was already test/build_all_versions.sh, item
3 landed in #1221. Item 2 is "leave the tree as it was found", and the script
tries:

# Leave no object tree behind from whichever major happened to be last: the next
# build against a different major would link objects compiled for this one.
make -C "$SRCDIR" clean >/dev/null 2>&1 || true

No PG_CONFIG, and || true. PGXS resolves pg_config from PATH, so on a
box without a packaged one this make fails, the failure is swallowed, and the
tree keeps the last major's objects while the script still prints PASSED.

Driven, two arms, same tree, pg18a then pg19a so the leftovers are pg19a's:

  ARM A   normal PATH (a default pg_config exists)
          built 2 of 2   PASSED    objects left:  0

  ARM B   PATH with no pg_config
          built 2 of 2   PASSED    objects left: 36

The container has /usr/bin/pg_config (PostgreSQL 18.4, packaged), which is why
this has never bitten here. A source-build-only box has no such thing, and this
repo's own matrix container is source builds.

The consequence is a wasted rebuild, not a wrong install

Because #1221 landed first. Running a pg18a suite on the poisoned tree:

  -- the objects in the tree were not built against
     /usr/local/pg18a/include/postgresql/server, whatever the build stamp says;
     cleaning first (#1219)
  accounting: 5 passed + 0 failed + 0 unrunnable + 0 skipped = 5
  hardening.sh: PASSED

Detected from the objects, cleaned, rebuilt, suite green. So the open direction
this issue was filed about stays closed. What is left is a comment that promises
something it does not always do, and one clean's worth of wasted work.

The fix is one argument: give the final clean a PG_CONFIG -- the last one
the loop used is already in $pgc -- and drop the || true, or keep it and say
in the comment that the clean is best-effort. I have not written it; three of my
PRs are already in the queue and this is small enough to ride with something
else.

The narrow case where it would still matter: a major built without -g leaves
objects with no DWARF, which #1221 reads as unknown rather than foreign. If
such a major were last in the loop on a box with no default pg_config, the
provenance check would have nothing to compare and the stamp would agree. That
needs three things at once and I have not tried to build it.

A correction in my own numbers above

I first reported 71 objects left. That was my glob, not the tree:

  find /root/t1219/src /root/t1219 -maxdepth 2 -name '*.o'   ->  71
  ... | sort -u                                              ->  36
  find /root/t1219 -name '*.o'                               ->  36

The two roots overlap, so everything under src/ was counted twice. 36 is
the number, and it agrees with this issue's own "35 objects each time". Fourth
time today I have published a count whose population was not what I said it was.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP