Skip to content

The ledger key omits the PostgreSQL major, so a check that exists on only some majors cannot be represented -- and orphan-scan will call it deleted #1010

Description

@OffgridwithJD

#982 found that (suite, name) was a key of check NAMES rather than of checks, because
harness_selftest sources 40-odd parts into one shell and phrases its premises to be
copied. The fix added the part. This is the same defect one field further out: the key
omits the PostgreSQL major, and a check's existence depends on it.

The key cannot represent what the tree actually emits

Two shapes, both on main at 4d7c75ae, both verified by reading the suite.

1. A whole suite collapses to one record on the older majors.
test/analyze_differential.sh:61:

if [ "$PGC_MAJOR" -lt 18 ]; then
	check_skip "the differential analyze path" "SKIP  pgcolumnar.analyze() needs pg_restore_attribute_stats (PG18+); this server is $PGC_MAJOR" "..."
	pgc_summary
fi

pgc_summary exits. So this suite emits 1 record on PG15-17 and N on PG18+. Same for
test/analyze_function.sh:70.

2. The two branches emit DIFFERENT CHECK NAMES, so the key sets are disjoint.
test/fk_referencing.sh:287:

if [ "$PGC_MAJOR" -ge 17 ]; then
	check "a partitioned table with no foreign key still converts" ...
	check "and its partitions inherit the columnar access method" ...
else
	check "before 17, core refuses this for every partitioned table" ...

On PG17+ the ledger sees two names; on PG15-16 it sees a third that cannot appear on
PG17+. Nothing in the key says which majors each belongs to, so the ledger cannot hold
"this check exists on 17+ and not below" -- it can only hold "this check exists".

A grep for PGC_MAJOR does not find all of these, which is why the fix needs the
dimension rather than a convention. test/native_repack.sh:48, test/pg19_vacuum_options.sh:34
and test/native_dml.sh:61 gate on server_version_num instead and never mention
PGC_MAJOR. Any rule phrased over one idiom misses the other.

What it costs today, stated precisely

Nothing, yet, and that is the point. All 1,155 committed rows come from three suites,
and I checked each for version gating:

suite rows version gates
harness_selftest 907 0 -- never references PGC_MAJOR at all
differential 204 0
native_join_runtime_filter 44 0

So this is not a live defect in the committed ledger. It is a blocker on seeding the
suites that do diverge, and every one of those is currently among the 249 uncovered:
analyze_differential, analyze_function, fk_referencing, unique_conc,
native_repack, pg19_vacuum_options, native_dml.

The direction that breaks first is orphan-scan, not gate

gate refuses a check in the log that the ledger has not seen. A PG18-only check simply
does not appear in a PG15 log, so the gate stays correct by never being asked.

orphan-scan asks the opposite question -- a ledger row no record in its own part matches
-- and that is exactly what a PG18-only row looks like in a PG15 run. It is saved today
only by the skip guard:

skipped_parts = {(s, p) for (s, p, _), v in verdicts.items() if "SKIP" in v}
orphans = [k for k in absent if (k[0], k[1]) not in skipped_parts]

analyze_differential on PG15 emits a check_skip, so its part lands in skipped_parts
and its absent rows are classified unprunable rather than orphans. That is luck, not
design:
fk_referencing's older-major branch emits check, not check_skip, so once
that suite is seeded, a PG15 run reports its two PG17+ checks as orphans -- rows naming
checks that have been deleted, when they have not.

The fix, and the one thing it cannot honestly do

Put the major in the key: (major, suite, part, name).

Two costs, both real:

  1. The census changes meaning. checks_never_observed_red would count (check, major)
    pairs, not checks -- roughly 1,155 -> ~5,800 at today's coverage. 5800 under a name
    that says "checks" is a number that lies by its own name, so the budget file has to
    restate it and the gate should print pairs and distinct checks side by side. It already
    prints two quantities, so that part is cheap.

  2. The 1,155 existing rows cannot be given a major. Their last_red and mutations
    were observed on some major and nothing recorded which. Regenerating loses the attack
    history, which is the ledger's whole value; copying one observation into five majors
    claims five observations from one. So they carry an explicit "no major recorded"
    sentinel, and that is a true statement rather than a migration stopgap -- harness_selftest
    never reads PGC_MAJOR, so 907 of them have no major to record even in principle.

That second point is worth stating plainly, because it is the part that surprised me: the
row's two columns are not the same kind of thing. Existence is per-(check, major) and
cheap to re-derive from any run. History is per-check and cannot be re-derived at all.

Prerequisite: the log must name its own major

RESULT records carry suite, part, name, verdict, reason and no major, so the tool can
only learn the major from whoever invokes it. That is the --date not-a-date failure
waiting to happen: a PG15 log merged as PG18 is silently misattributed, and the ledger's
subject is provenance. The record should carry the major and the runner should still pass
the one it ran, so the two reconcile -- the same fail-closed shape read_records
already applies to checks run:.

That splits cleanly and in dependency order:

  • Step 1. The record names its major; the tool validates it and ignores it. Green on
    its own, independently useful (a log becomes self-describing), no ledger change.
  • Step 2. The ledger keys on it: format, migration to the sentinel, every subcommand
    widened, budget restated.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions