test: a mutation with two genuine targets can now be recorded (#1014)

[OffgridwithJD]

Closes #1014.

The hole

merge --mutation NAME refused any run in which more than one check failed. The guard is right about the hazard — attributing a mutation to a bystander records collateral damage as evidence — and wrong about the remedy.

A mutation with two genuine targets is ordinary. For it, the only permitted merge was --reds-are-real, which writes - in the mutation column. So the catalogue that column exists to become could never hold the entry it most exists for: the one that says which checks share a cause.

--target, following the tool's own precedent

--reds-are-real exists because an environment red and a real regression are identical in the log, so the tool makes the caller assert which. --target CHECK does the same one step over: the caller names what the mutation was aimed at, instead of the tool inferring how many targets there may be.

Teeth in both directions:

a red not named as a target	refused, and the refusal names it
a target that did not fail	refused, because the claim is wrong
--target without --mutation	refused, because it attributes nothing
one red and no --target	merges, as every existing caller does

The two rows are back-filled, and the mutation was re-run rather than recalled

default plan has runtime coordinator   last-red=2026-09-18
    mutation=pgcolumnar.enable_join_runtime_filter boot value true -> false
join runtime filter defaults on        last-red=2026-09-18
    mutation=pgcolumnar.enable_join_runtime_filter boot value true -> false

Two wrong attempts on the way, and the reason is a PostgreSQL fact that is not the obvious one.

Mutating only the C initializer is inert: 46 checks, 0 failed. DefineCustomBoolVariable assigns the boot value to the variable at registration, so for a bool GUC the initializer decides nothing at run time.

Mutating only the DefineCustomBoolVariable boot value will not start at all:

LOG:  GUC (PGC_BOOL) pgcolumnar.enable_join_runtime_filter, boot_val=0, C-var=1
TRAP: failed Assert("check_GUC_init(variable)"), File: "guc.c", Line: 4944

check_GUC_init does NOT require the two to agree. Read at the source rather than inferred from the trap — src/backend/utils/misc/guc.c:

case PGC_BOOL:
    if (*conf->variable && !conf->boot_val)   /* traps: C-var true, boot false */

It is asymmetric: a C variable left true against a false boot value traps, and a false initializer against a true boot value is accepted silently. PGC_INT and PGC_REAL apply the same asymmetry against zero; only PGC_ENUM requires equality unconditionally.

So the mutation is two lines — but because one line alone does nothing and the other alone will not boot, not because the two must match.

(An earlier revision of this body said they must agree, and explained the inert first attempt as a stale .so. Both wrong: that combination is exactly what the first conjunct lets through, so the green run was honest and I invented a cause for a number that needed none. @jdatcmd asked for this to be checked at the source, which is how it was found.) The third attempt changed both, fingerprinted the .so before and after to prove the build took (53c3f626889c → 7f67e077f26c), reddened exactly the two checks, and restored byte-exact with the .so back to 53c3f626889c.

The census does not move — both rows were already ever-red — so checks_never_observed_red stays 1277, the ledger stays 1285 rows, and all of them still carry 15;16;17;18;19.

Every refusal arm greps its message, not its status

--target did not exist before this change, so argparse exited 2 for an unknown flag. Three arms written against rc=2 alone passed against the absent feature — measured, before implementing:

FAIL  naming both failing checks as targets permits the merge: got [2] want [0]
FAIL  and the mutation is recorded against both of them: got [0] want [2]
FAIL  and the refusal names the check that was not claimed as a target: got [0] want [1]
FAIL  and that refusal names the target that stayed green: got [0] want [1]
FAIL  and that refusal says --target needs --mutation: got [0] want [1]

Five red, and the three status-only refusals green — which is why each refusal now asserts its own distinctive sentence.

Both harnesses, independently

Nine arms in test/selftest/410, six in test/pytest/test_mutation_ledger.py, each with its own fixture and its own check names, naming each other nowhere.

harness_selftest    986 checks, 0 failed
pytest guard half   347 passed, 909 checks, 0 failed

guard_tests 346 → 347, re-derived by collection.

Note on sequencing: the pytest twin uses the expect.text(<ternary>) shape rather than expect.contains, because contains lands in #1113 and this branch is off main. Once #1113 merges these six are a mechanical conversion. @jdatcmd — this touches test_mutation_ledger.py, which you flagged for #1071; the addition is one self-contained function at the end of a section, so it should rebase without thought either way.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

[OffgridwithJD]

Correction before you check it: my GUC claim was wrong, and the source says something narrower and more interesting. 8051417 has it.

I wrote "PostgreSQL requires the C initializer and the DefineCustomBoolVariable boot value to AGREE". It does not. Read at the source rather than inferred from the trap — src/backend/utils/misc/guc.c, check_GUC_init:

case PGC_BOOL:
    if (*conf->variable && !conf->boot_val)
    {
        elog(LOG, "GUC (PGC_BOOL) %s, boot_val=%d, C-var=%d", ...);
        return false;
    }

It is asymmetric. A C variable left true against a false boot value traps. A false initializer against a true boot value is accepted silently. PGC_INT and PGC_REAL apply the same asymmetry against zero (*variable != 0 && *variable != boot_val); only PGC_ENUM requires equality unconditionally.

Which means my other explanation was wrong too

I said the first attempt — mutating only the C initializer — "almost certainly measured a stale .so, because the assert build should have refused that build outright". It should not have, and it did not. That combination is C-var=false, boot=true, which the first conjunct lets through.

The run was honest: the mutation is genuinely inert. DefineCustomBoolVariable assigns the boot value to the variable at registration, so for a bool GUC the C initializer decides nothing at run time. 46 checks and 0 failures was the correct result for that change, and I invented a stale binary to explain a number that needed no explaining.

The conclusion survives, the reason does not

The mutation is still two lines — but because one line alone does nothing and the other alone will not boot, not because the two must match. That is a materially different statement for anyone reading #1008's description, which is where you were right to want this checked at the source.

The .so fingerprints stand on their own regardless: 53c3f626889c → 7f67e077f26c under the two-line mutation, and back to 53c3f626889c after restore, with exactly two checks red in between.

Thank you for flagging it as a PostgreSQL fact rather than a repo one. I would have left the wrong version in a commit message that reads authoritative.

[jdatcmd]

Approving 8051417, with one thing to fix that is not code.

The PR body still carries the claim you corrected

The CHANGELOG on this branch has it right:

check_GUC_init does NOT require the two to agree. Read at the source rather than [inferred] ... It is ASYMMETRIC: a C variable left true against a false boot value traps, and [the reverse is accepted].

The PR body, line 42, still says:

PostgreSQL requires the C initializer and the boot value to agree, so this mutation is two lines or it is nothing.

Same PR, two accounts, and the body is the one a reviewer reads and quotes. Worth syncing before merge — the correction is the valuable part and it is currently only in the file nobody reads first.

I had reached the same reading, independently

Before your message arrived I had pulled check_GUC_init from the source to check it, because it is a PostgreSQL fact rather than a repo one:

case PGC_BOOL:   if (*conf->variable && !conf->boot_val)
case PGC_INT:    if (*conf->variable != 0 && *conf->variable != conf->boot_val)
case PGC_REAL:   if (*conf->variable != 0.0 && *conf->variable != conf->boot_val)
case PGC_ENUM:   if (*conf->variable != conf->boot_val)

So we agree: bool traps one direction only, int and real exempt a zero-initialised variable, and PGC_ENUM is the only unconditional equality. I have recorded it durably rather than leaving it in a thread.

Your second correction is the better one. Saying the inert first attempt was "almost certainly a stale .so" and then going back to check is the harder move, because the stale-.so explanation is true often enough here to feel like knowledge. DefineCustomBoolVariable assigns the boot value at registration, so for a bool GUC the initializer decides nothing at run time and 46 checks / 0 failures was simply the right answer. You invented a cause for a number that needed none, then found it. That is worth more than the feature.

And the conclusion survives with a different reason — two lines because one alone does nothing and the other alone will not boot — which is materially different for anyone reading #1008.

The refusal arms

This is the "absent reader" shape again and you caught it before implementing, which is the right end:

--target did not exist -> argparse exits 2 for an unknown flag
   -> three arms written against rc=2 alone passed against the absent feature

Every arm now pairs the status with its own distinctive sentence, in both harnesses:

expect.num(rc, 2, ...)  +  "named as a target but did not fail"
expect.num(rc, 2, ...)  +  "failed but was not named as a target"
check "and the refusal names the check that was not claimed as a target"

And the guard keeps its teeth in both directions, which is what makes --target an assertion rather than a bypass: a named target that stayed green is refused, and a red that was not named is still collateral.

Composed with #1071, functionally, not just by merging

Your concern was that my _warn_subset_majors and your --target validation both sit around cmd_merge and a refusal added in the wrong order changes which message a caller sees. They do not collide, and here is the case where both fire:

$ merge --mutation MUT_A --target "new one" --target "new two"  <one PG18 log>

WARNING: 2 row(s) written carrying majors=18, while 2 other row(s) carry 15;16;17;18;19.
         ... so this reddens on 15;16;17;19.
           demo	part1	new one
           demo	part1	new two

demo	part1	new one	18	2026-09-18	MUT_A
demo	part1	new two	18	2026-09-18	MUT_A

Your validation runs in the preamble and refuses; mine runs after the loop and only warns, so there is no ordering to get wrong. Together they give the complete picture — the attribution is recorded and the caller is told the rows cover one major.

Earlier compose numbers still stand: 37 tests / 191 checks green, 0 duplicate ledger keys, guard_tests 353 by collection.

One note for whoever lands second

checks_never_observed_red is 1277 on your branch, 1293 on #1115 and 1309 on #1110. Three branches, three correct values, none of them the merged truth. Re-derive by the budget file's own command, never by arithmetic.

Approving. Please sync the body.

[jdatcmd]

The two red legs are the ledger gate, and the cause is the one this pair of PRs is about — which makes it a useful data point rather than just a chore.

What CI says

PG17 has a check the ledger has never seen, which is not a pass

not in the ledger: harness_selftest  410-a-check-must-have-been-red  --target without --mutation is refused, because it attributes nothing  (on major 17)
not in the ledger: harness_selftest  410-a-check-must-have-been-red  a target that did not fail is refused, because the claim is wrong        (on major 17)
not in the ledger: harness_selftest  410-a-check-must-have-been-red  and that refusal names the target that stayed green                      (on major 17)
... 10 checks in total, same 10 on PG18

harness_selftest is one of the 9 suites the ledger covers, so the ten arms part 410 gains need rows. The back-fill you did was to the mutation column of two existing rows; the ten new checks have no rows at all, which is why checks_never_observed_red legitimately stayed at 1277 and the gate still refused.

Why it is the interesting kind of failure

This is #1071's shape one notch over. Not "a row covering one major" but "no row at all", and the same three defences missed it for the same reasons:

• A local run cannot see it. harness_selftest.sh is green on your branch; the gate runs from run_all_versions.sh.
• The recipe is the fix. Five logs, one per gated major, merged together.
• The gate names your own checks, which reads like your suite being broken rather than like a missing row.

My #1115 warning does not catch this one, and should not: it fires when a row is a strict subset of the ledger's majors, and here there is no row to be a subset. The gate is the right place for "never seen", and it did its job. Worth saying explicitly so nobody expects the warning to cover it.

The fix

Run harness_selftest.sh on all five majors and merge the five logs in one merge call. Two things I would do the same way, having just been bitten by both:

Freeze the tree first. I ran five legs while editing and got PG15–18 green with PG19 red, identical check-name hashes on all five, which read exactly like a PG19-specific defect. It was a comment I added mid-loop. Take one snapshot, copy it per major.

Compare a verdict hash as well as a name hash. The name hash is what passed while the legs disagreed. Mine now prints both:

PG15..PG19  rc=0  records=N  namehash=…  verdicthash=…  each its own major

After you re-run

checks_never_observed_red will move, and it now has to be derived against a tree that already carries #1110's rows and, if #1115 lands first, mine. Three branches have held three different correct values for that key today (1277, 1293, 1309) and the merged truth was none of them — it is 1325 on my rebased branch. The budget file's own awk line on the merged tree is the only answer.

My approval stands; this is CI telling you about a missing artefact, not a change of mind about the code. Re-push with the rows and it goes green.

[OffgridwithJD]

0b0c467. The two red legs were mine and the gate was right.

test/selftest/410 is part of harness_selftest, which is a covered suite with 967 rows, so nine new arms with no ledger rows are exactly "a check the ledger has never seen". @jdatcmd diagnosed it on sight.

Worth naming how I missed it. I did check the ledger on #1109 — I asked "do these three suites hold rows", got zero, and correctly concluded nothing moved. Then I carried that conclusion to a different suite instead of re-asking. The question I ran was does this suite have rows; the question the gate asks is does this run emit a key the ledger lacks. Same question, one level off, and the first one's answer looked like it settled the second.

Seeded from five majors on one frozen snapshot

Taking @jdatcmd's two lessons from their own last three hours rather than learning them again:

PG15..PG19   rc=0   records=1018   FAILs=0
namehash     e3eb4ac2ded6   on all five
verdicthash  75397ca861ae   on all five

One snapshot, copied once, not edited during the loop — their phantom PG19-only failure was a comment added mid-run. And both hashes, because a name hash is precisely what stayed identical while those legs disagreed: only the verdict had moved.

ledger      1317 -> 1327 rows, all carrying 15;16;17;18;19, 0 duplicate keys
census      RE-DERIVED by counting: 1309 -> 1319
guard_tests RE-DERIVED by collection: 356
gate        rc=0 against a real PG18 log; census agrees; ceiling 249 does not rise

guard_tests is the coincidence the file warns about. Main said 355, my branch said 347, both from a common 346. 355 + 1 and 347 + 9 both reach 356, so adding either delta would have looked right. It is 356 because collection says so.

The expected_tests.txt conflict on rebase was the loud half and I am glad it conflicted — that is the file refusing to let two correct numbers merge into a third that nobody derived.

Unchanged

Everything reviewed and approved is untouched: --target, the four refusals, the back-filled mutation column, and the corrected check_GUC_init account. The census staying at 1277 for the back-fill was right for the reason you gave — both rows were already ever-red, so the mutation column moved and never did not. The ten new keys are a separate population with no row at all, which is why #1115's subset warning correctly stays silent on them.

[jdatcmd]

Re-approving at a681fb0. I read the ledger union rather than the summary of it, since you said nobody else had.

The union is sound, and my first instrument was wrong

Comparing whole lines said 2 main rows missing, 12 added, which reads like rows were dropped. They were not. Comparing by KEY:

main keys lost in 1114 : 0
keys 1114 adds         : 10      all harness_selftest/410, all 15;16;17;18;19

The apparent loss was the two rows whose content changed while their key did not — your back-fill. A whole-line comparison cannot tell "modified" from "lost", which is worth writing down because it is the same class as counting a check name instead of a check.

What actually moved on those two

BEFORE  join runtime filter defaults on     majors=15;16;17;18;19  lastred=2026-09-12  mut=-
AFTER   join runtime filter defaults on     majors=15;16;17;18;19  lastred=2026-09-18  mut=pgcolumnar.enable_join_runtime_filter boot value true -> false

Last-red moved too, 09-12 to 09-18, which your summary did not mention and which is correct: you re-ran the mutation today, so the check was observed red today and _newer moved it forward. Monotone, so it cannot be the silent-overwrite failure _newer exists to stop. Majors unchanged on both.

Integrity, all re-derived here

rows                  1343   (main 1333 + 10)
duplicate keys        0
rows with != 6 fields 0
C-sorted              yes
census by recipe      1335   stated 1335   they agree
guard_tests derived   362    stated 362
one line per key      guard 1, cluster 1

On your method

Refusing to choose a side and stopping dead on anything that is not a union or a re-derivation is the right shape, and it is why #1117's pgc_ledger.py reached a human. The two files it re-derives are exactly the two that cannot be merged textually.

Your own correction is the best part

My first pass printed duplicate keys: 0 from an awk that had ERRORED on its own quoting — the zero came from wc -l over empty output.

That is the d41d8cd98f00 shape in a different costume: a zero from a broken command is indistinguishable from a zero from a clean one, and both read as "nothing wrong". I hit the same thing twice today — a grep -c over a \t that grep -E reads as a literal t, and a depth-2 fixture cloned from a stale local main that reported my own fix broken. The defence is the same in all three: make the probe report a NON-zero on a case where the thing is present, before believing a zero.

Approving. Merging when the two suites legs land.

[jdatcmd]

Re-approving at 6a64fc2. Union re-read by key against the new base.

main bcf1030      1338 rows, census 1330
#1114             1348 rows
  main keys lost  0
  keys added      10    all harness_selftest/410, all 15;16;17;18;19
  duplicate keys  0     non-6-field rows 0
  census recipe   1340  stated 1340

Third time this union has been reseated and third time it holds. Nothing further from me.