feat: cache the FSST keep/drop verdict with an age bound (#472)

[jdatcmd]

Closes #472. A text load is about 2.5x faster.

Deciding whether an FSST symbol table pays for itself costs a whole-corpus encode plus a compression pass, and the answer cannot be sampled: on a training prefix FSST can look 24% worse while over the whole column it is 23% better. So the question was asked once per column per row group, and for a column whose data does not change character that re-derived the same answer for the whole load.

Re-measured first, because the issue says to

Its numbers predated #155's distinct probe and fsst_min_gain_percent. Instrumented build, PG18.4, 2,000,000 rows, 20 row groups:

shape	load	deciding	verdict	share
md5(g)	5319 ms	2482 ms	hurts 20/20	47%
email-shaped	2081 ms	843 ms	hurts 20/20	41%
'label-' || (g%40)	533 ms	0 ms	build skipped by #155	0%
low-card then md5	3021 ms	1269 ms	hurts 10/10	42%

Agrees with #499's independent profile. The low-cardinality row is the control: #155 already skips the build there and that path costs nothing, so this is about the other one.

The change

The verdict is cached on PgColumnarColumnDef, which is palloc0ed, so UNKNOWN is the natural zero and a fresh write state always asks once. The cache lives exactly as long as the write state, which is one statement. Nothing is persisted; no on-disk structure changes.

pgcolumnar.fsst_verdict_reuse bounds it, default 16, 0 meaning ask every time. It exists because the byte-identical arm needs an off switch to compare against, and it doubles as the escape hatch.

The two verdicts do not save the same work, and the code says so rather than pretending:

• reused HURTS skips the build and the question, since the vectors then take their ordinary encoding, which is what a fresh HURTS would have produced
• reused HELPS still builds the table, because it is trained on this row group's corpus and stored with the chunk, so reusing the table would change the stored bytes. Only the question is skipped, and that is the expensive half.

The risk is silence, not speed

A stale verdict does not corrupt anything. It compresses worse, correctly, and nothing would notice. So the headline checks are byte equality of the stored chunks, via the catalog's encoding descriptor, block codec and page length per chunk. A checksum of the relation file would have been useless here: columnar pages carry LSNs, so two identical loads differ.

Five of six candidate corpora return HURTS (md5, urls, emails, JSON, log paths); only prose keeps FSST. A suite built on the common case would have exercised one branch and let a wrongly cached HELPS through, so there is a fixture for each and the premise asserts they take different branches, using the descriptor decoder from write_fsst_compressed.sh and its documented header/stride trap.

The age bound's assertion was wrong first, and the measurement corrected it

I asserted a bounded cache stores no more than an unbounded one. It stores more: 5778575 against 5628054. PgColumnarFsstHelpsCompressed keeps FSST only when the win clears fsst_min_gain_percent, so a marginal win is declined deliberately and a stale HELPS takes that margin back. Smaller, and still the wrong call, because the margin pays for decode.

The check now measures distance from the uncached decision:

-- changing column stored bytes: uncached=5778575 bounded=5778575 unbounded=5628054
-- distance from the uncached decision: bounded=0 unbounded=150521

Verification

• Proved by removal: with only the reuse condition disarmed and every other line identical, the speedup disappears (1601 vs 1639 ms) and the age-bound premise fires with no (the fixture does not change character) — exactly what an absent cache should look like. Fingerprinted bede85aa fixed, 23d6f010 armed, bede85aa restored byte-identical.
• The byte-equality arms still pass when disarmed, correctly: they are safety checks, not feature checks. Said here so a reader does not mistake them for the discriminating ones.
• 11 suites clean, zero failures: fsst_verdict_cache (16), write_fsst_compressed, fsst_margin, encode_effort, encode_invariants, native_writer, native_encoding, native_roundtrip, differential (201), parallel_copy (77), harness_selftest.
• New suite registered in sorted position; count verified through the runner (132 → 133), not by parsing the array.

Plan in design/FSST_VERDICT_CACHE_PLAN.md.

🤖 Generated with Claude Code

[ChronicallyJD]

Approving. This is the strongest PR I have reviewed on this repo: it re-measured before acting because the issue said to, it found the branch asymmetry that makes HELPS and HURTS save different amounts and said so instead of claiming a uniform win, and it caught its own wrong assertion about the age bound with a measurement rather than shipping it.

The self-correction is the part worth naming. You asserted a bounded cache stores no more than an unbounded one, measured 5778575 against 5628054, and worked out why the smaller number is the worse call — fsst_min_gain_percent declines a marginal win deliberately and a stale HELPS takes that margin back. Then you changed the check to measure distance from the uncached decision rather than raw size. That is the right invariant and it is not the obvious one.

Checked rather than assumed

I went looking for a specific bug and did not find it. The build is skipped entirely on a reused HURTS, so that path exits through a different arm than the if (fsstTable != NULL) block, and I expected the age never to advance on the common path — which would have made the bound apply to HELPS only and a HURTS cache immortal. It is handled, at the right place, and the comment names exactly that failure:

The build was skipped on the strength of the cached verdict, so this row group counts as a reuse too. Without this the age would never advance on the common path and the bound would never re-take the verdict.

I also expected to find that the shipped default of 16 was never exercised. It is: fv_prose_on runs at 16. Withdrawn.

One finding: that increment is not covered by a test that would fail without it

The line is correct. Nothing in fsst_verdict_cache.sh would notice if it were deleted, and this repo's standard is proof by removal.

Walking the fixtures against it:

fixture	reuse	does the HURTS-aging branch decide anything?
fv_prose_off / on / one	0, 16, 1	prose is HELPS throughout, so the branch is never taken
fv_chg_off	0	never reuses
fv_chg_unbounded	1000000	HELPS cached and never re-taken, so the branch is never reached
fv_chg_bounded	2	branch is taken, ages out, re-decides — and gets HURTS again

So on the only fixture that reaches it, the verdict it ages out into is the same verdict it was already caching. Delete def->fsstVerdictAge++ and the md5 half simply reuses HURTS forever, which is the correct answer for md5, so every stored-byte comparison still passes.

The gap is a direction, not a value: every changing fixture runs HELPS → HURTS. The reverse, HURTS → HELPS, is what makes that increment observable — a cache stuck on HURTS never applies FSST to the half that wants it, and the stored bytes diverge materially from uncached.

Concretely, the mirror of CHANGING:

CHANGING_UP="CASE WHEN g <= $((ROWS / 2)) THEN $MD5 ELSE $PROSE END"

with the same three arms. Without the increment the bounded arm should collapse onto the unbounded one; with it, it should land near uncached, which is the invariant you already assert in the other direction.

Worth adding because that increment is load-bearing for the common path — a text load is mostly HURTS — and it is the one line in the change whose removal the suite is blind to.

A question, not a request

Why 16? The plan explains what the knob does and why 0 exists, but not why the default sits there rather than at 4 or 64. I raise it having got exactly this wrong today: I shipped a 0.25 threshold in an earlier branch that was bracketed by two points and turned out to be on the wrong side of the real crossover by a wide margin.

The cases are not equivalent, and I do not think this one needs a sweep. Being wrong about 16 costs somewhat worse compression for a bounded number of row groups and is capped by construction; being wrong about my threshold cost a 1.9x regression. But if 16 was chosen rather than measured, saying so in the plan costs one sentence and stops the next person treating it as derived.

Smaller things, all correct

• palloc0 giving UNKNOWN as the natural zero, so a fresh write state always asks once, is the right way to avoid a separate "initialised" flag.
• Six candidate corpora tried and five returning HURTS is exactly the fixture-selection discipline that would otherwise have left the HELPS branch untested. Five of six is also a useful fact in itself.
• Saying explicitly that the byte-equality arms still pass when disarmed, because they are safety checks rather than feature checks, stops a reader mistaking them for the discriminating ones. That sentence prevents a real misreading.
• Rejecting a file checksum because columnar pages carry LSNs, and comparing catalog descriptors instead, is correct and not obvious.