fix: a suite must not gate on a random draw (#487)

[jdatcmd]

Closes #487.

analyze_function.sh required core's SAMPLED null_frac to differ from the truth before asserting ours is exact, and failed the whole suite as vacuous when it did not. It did not, on PG19, during #484's matrix:

FAIL  premise: core's sampled null_frac differs from the truth, so this suite
      can discriminate: got [no (sample landed exactly on truth; suite is vacuous)]
-- core sampled null_frac = 0.1, truth = 0.100000

Every other check passed, including the one that premise guards.

Why it is not rare enough to ignore

k is NULL for exactly one row in ten and core samples 300 * statistics_target = 30,000 rows, so its null_frac is (nulls drawn)/30000 and the truth is 0.1 = 3000/30000. The premise required a binomial with n=30,000, p=0.1 not to land on its own mode — about 0.8%, one run in roughly 130. The suite runs on PG18 and PG19, so that is two draws per matrix.

Six consecutive PG19 re-runs after the failure all passed, at 0.0977 through 0.1041.

The concern was right; the instrument was wrong

The premise exists because if core's number and the exact number are the same here, an implementation that merely called core ANALYZE would pass. That is kept, and restated as arithmetic instead of luck.

A new column k7 is NULL for one row in seven, so the truth is 71428/500000 = 0.142856, and core's estimate is always a whole number of sampled rows over 30,000. No such whole number exists — it would need 4285.68. Core cannot report this fraction whatever it draws.

Measured, three consecutive core runs on this fixture:

	value	as a fraction of the sample
core run 1	0.1442	4326/30000
core run 2	0.1425	4275/30000
core run 3	0.13893333	4168/30000
ours	0.142856	not expressible

Both facts the argument rests on are asserted rather than assumed: that core samples fewer rows than the table holds, and that no whole number of sampled rows yields the fraction. Core's actual draws are now printed, not asserted — which is what #475 already did for the histogram outlier in this same file, for this same reason.

The n_distinct premise had the identical shape and is demoted the same way. It was far less exposed, since a sampled distinct estimate comes off a formula rather than a binomial's mode, but "much less likely" is still the wrong thing to gate a suite on. What makes that check discriminating is that its expected value comes from an independent SELECT count(DISTINCT k), not from core being wrong.

The premise read the wrong statistics target at first

Forcing the census case is what found it. attstattarget overrides default_statistics_target, and computing the sample size from the global default left the premise reporting a 30,000-row sample while core had read the whole table — so the contradiction surfaced two checks later as a confusing failure instead of here as a clear one. That is the same mistake slice 3b already fixed once in this file.

It now reads the column's effective target, and forcing a census fails the premise itself, naming the numbers:

FAIL  premise: core samples fewer rows than the table holds, so its number is
      an estimate: got [no (sample 3000000 covers all 500000 rows)]

Verification

Proved both directions on the same one-line simulation of the real draw (core_nullfrac forced to the truth, nothing else changed):

suite	result
as it is on main	FAILED — the exact message from the matrix
this branch	PASSED

Green on PG18 and PG19, 42 checks each.

🤖 Generated with Claude Code

[ChronicallyJD]

Approving. This is the right fix and it is more rigorous than what I suggested on the issue — reading the sample size from the column's effective attstattarget rather than writing 30000, and asserting both underlying facts instead of just the conclusion, are the parts I did not think to ask for. Finding the global-vs-per-column target bug by forcing the census case is the kind of thing that only turns up when you try to make your own premise fail.

I checked the arithmetic independently rather than taking it:

nulls from g % 7 = 0 over 500,000 rows = 71,428
truth = 71428/500000 = 0.14285600
0.142856 x 30000     = 4285.68        -> not an integer, unrepresentable
nearest reachable    4285/30000 = 0.14283333  (gap 2.3e-5)
                     4286/30000 = 0.14286667  (gap 1.1e-5)

Both gaps are ~1e-5 against a float4 spacing of ~1e-8 near 0.14, so real cannot collapse the truth onto a reachable value. The premise holds with margin.

One fragility, in the arithmetic check itself

awk -v t="$true_nullfrac7" -v n="$core_sample_rows" \
	'BEGIN { p = t * n; print (p == int(p)) ? "no ..." : "yes" }'

t is round(count(*) FILTER (...)::numeric / count(*), 6) — a decimal rounded to six places, and the test then asks whether a float product is an integer. It is correct here only because 71428/500000 happens to terminate at exactly six decimal places, so the rounding is lossless. That is a property of this fixture, not of the check.

ROWS is overridable via PGC_ANALYZE_ROWS, and the comment says the arithmetic must "fail honestly instead of quietly stop applying" if the configuration changes. With a non-terminating fraction it would not: round(...,6) would hand awk a value that is not the truth, and p == int(p) would then be answering about the rounded number. It could report "unrepresentable" for a fraction that is reachable, or the reverse.

The exact form needs no floats at all — representability is a divisibility question over integers:

K/N is reachable as m/S  <=>  K*S is divisible by N

check "premise: and no whole number of sampled rows gives that fraction, so core cannot report it" \
	"$(q "SELECT CASE WHEN (count(*) FILTER (WHERE k7 IS NULL))::bigint * $core_sample_rows
	                       % count(*) = 0
	                 THEN 'no (reachable)' ELSE 'yes' END FROM af_c")" "yes"

On this fixture: 71428 x 30000 = 2,142,840,000, and mod 500,000 = 340,000, non-zero, unrepresentable — same verdict, by exact integer arithmetic, for any ROWS. It also drops the awk dependency and with it any locale that formats decimals with a comma.

Not a blocker: today's check gives the right answer on today's fixture.

Merge with #488 — clean, but they jointly leave a stale line

I trial-merged this branch into #488 (which rewrites null_frac's source for #485 and adds the stride checks). They merge with no conflicts, and the rename in mine survives intact:

check_num "pgcolumnar.analyze() reports null_frac exactly, from reading the column"

But the merged file keeps this at line 31, in the suite header, which neither PR touches:

Slice 1 asserts null_frac comes from the zone maps and is EXACT where core's is sampled.

After #488 that is false. It is my change that invalidates it, so I have fixed it on my branch rather than asking you to — pushed as c19f9b0. Flagging it here only so the interaction is on the record: git had nothing to say about it, because a stale comment is not a conflict.

No sequencing needed on my account. Land this whenever; I will rebase.

On the demotion of the n_distinct premise

Agreed, and the reason you give is the load-bearing one: what makes that check discriminating is that its expected value comes from an independent SELECT count(DISTINCT k), not from core being wrong. That is the same property the histogram checks got in #475 and the same one I leaned on for the stride oracle in #488. Three places in this file now, which is probably enough to state it as the file's convention rather than re-deriving it each time.