fix: keep an encoding only when it is smaller after the block codec

[jdatcmd]

Closes #1132.

An encoding is chosen on pre-codec bytes and the chunk is stored
post-codec. PgColumnarEncodeChunk compares every candidate against
bestLen, which starts at rawLen (columnar_encoding.c:2329), and every
comparison is len < bestLen on uncompressed bytes. The block codec runs
afterwards, once, over the whole encoded region (columnar_write_state.c:1479),
defaulting to zstd level 3. Bit-packing whitens a stream the codec was
exploiting, so an encoding that shrank the bytes can enlarge the stored
chunk, and nothing in the selection path can see it.

FSST already decides this way, through PgColumnarFsstHelpsCompressed. This
asks the same question for the encoders that had no such gate.

The fix

The writer compresses the encoded region and the raw one and keeps whichever is
smaller, per column chunk -- the granularity the codec actually runs at, not
per vector. When it takes the raw one it swaps in an all-NONE descriptor built
alongside, because a descriptor that disagrees with its chunk is a decode error
rather than a size regression.

Two things bound the cost. The alternative is only built when
pgcolumnar.enable_post_codec_encoding_choice is on, so the knob costs nothing
rather than costing memory silently; and the second compression only runs when
rawRegion->len > encoded->len, which is false whenever the encoders all
declined and the two regions are the same bytes.

Measured on real data

ClickBench hits_0.parquet -- 1,000,000 rows, 105 columns -- imported twice
through pgcolumnar.import_parquet on PG17, once with each build.

stored total    81,869,112  ->  78,109,810      -4.59%
value bytes     68,744,112  ->  64,984,810      -5.47%

NONE vectors           101  ->       1,621
RLE                   5,372 ->       4,558
DICT                  3,782 ->       3,390
FOR                     934 ->         621
DELTA                     1 ->           0
FSST                    310 ->         310      unchanged

FSST is unchanged because it already had this gate, and the encoders that
did not are exactly the ones that moved. That is the result I would want to see
before believing the rest.

Biggest movers:

ClientEventTime   3,530,536 -> 1,777,051   -49.7%
HID               3,876,000 -> 3,257,411   -16.0%
EventTime         2,251,000 -> 1,755,641   -22.0%
LocalEventTime    2,249,612 -> 1,757,239   -21.9%

Same data out of both loads: row counts equal, sum(WatchID) and
sum(ClientEventTime) equal, and an md5 over URL ordered by
(WatchID, EventTime) equal.

What it costs to write

+4.40% backend instructions. Wall clock is not usable here -- six identical
loads spread 19.8 s to 27.2 s, 37% -- so this is cpu_core/instructions/ pinned
to the P-cores, three alternated reps per arm:

ON    194,674,775,778  194,669,148,754  194,618,892,582   mean 194,654,272,371
OFF   186,554,174,618  186,481,676,596  186,341,557,647   mean 186,459,136,287
                                                          +4.40%

Spread within each arm is 0.03% and 0.11%. For context, #1125 priced the FSST
verdict's equivalent at 15.5% on the keep path and 32.0% on the drop path; this
is cheaper because the precondition skips every chunk where encoding declined.

Red, then green, in both harnesses

Both halves were run against the unfixed build and both reddened on the same
arm, alone:

a chunk is not stored larger than it would be with no encoding at all:
    got 'inflated' want 'not-inflated'

with every premise, every control and the read-back invariant green in the red
state. After the fix the tail column reports encoded_vectors=0 and 440,397
bytes against an offline zstd-only prediction of 440,492, and the control column
is byte-identical at 39,906.

The fixture took three attempts, and that is the interesting part

Five synthetic shapes failed to reproduce the defect -- all showed encoding
winning. I had to characterise the real column first. ClientEventTime is a
heavy tail: 91,735 distinct values over a range of 1,707,676,369, because
rare outliers reach back to 1971 while the typical value sits in a narrow recent
band. That splits the two cost models exactly -- FOR prices by RANGE and must
size every value for the outliers, zstd prices by BYTE REDUNDANCY and the
typical value's high bytes are constant.

Repetition alone does not reproduce it (measured 0.71x, encoding winning),
so the tail is load-bearing and the first premise asserts it rather than
assuming it.

And that premise was wrong the first time. It used the 0.1st-to-99.9th
percentile spread, which with one outlier in a thousand lands exactly ON the
boundary: from one seed it read 4,994 on one run and 53,786,536 on another,
green then red, with no code change between. It reads the 1st-to-99th percentile
now, measured at 4905 / 4905 / 4904 across three runs.

What the matrix caught that I would have shipped

Full PG17 matrix, 254 suites ran, 5 skipped, 0 incomplete. Three failures, and
one of them was a design gap rather than a detail:

• harness_selftest -- the new suite was not registered in
  run_all_versions.sh.
• encode_invariants and native_dict_underfill -- controls asserting that
  the encoder under test actually ran (encode_invariants.sh:139,
  "the widths above are only meaningful if something is bit-packing"). The
  change voided them silently, so those suites would have kept passing while
  proving nothing.

pgcolumnar.enable_post_codec_encoding_choice exists because of that second
one, not because I anticipated it: a fixture chosen to exercise
frame-of-reference packing is not necessarily a fixture where packing beats the
codec. Those two suites pin the pre-#1132 behaviour and go on testing the
encoders; the selection policy has its own suite.

Everything correctness-critical passed unchanged, including
native_encdesc_golden, differential, corruption, fuzz, recovery and
native_roundtrip.

Ledger

encode_post_codec is a new suite, so leaving it unseeded would raise
suites_not_covered, which is a monotone ceiling the gate refuses to let rise.
Rows are seeded from five real runs merged in a single call, so every row
carries 15;16;17;18;19 rather than a neighbour's major set.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[OffgridwithJD]

The C is right and I went looking for four ways it could be wrong. One blocking defect,
in a derived number, and CI has not run yet so nothing has caught it.

Blocking: cluster_tests is not updated, and the whole half refuses to run

test_encode_post_codec.py collects two tests, and expected_tests.txt is not in this
PR's changed files. Run exactly as CI runs it, on your branch:

stated cluster_tests = 418
ERROR: collected 420 test(s) but expected 418. A run that quietly collects fewer tests
       than it should is a green that means nothing.
no tests ran in 0.31s

The failure mode is worth naming: it is not that a number is stale, it is that nothing
in the cluster half executes. Your own new twin never runs either.

And the number has moved under you. #1133 merged at 19:33, taking main to 421. So:

your branch states        418
main states now           421
merged onto current main  423   <- collected, and what the file must say

Also one conflict, from the same collision:

CONFLICT (content): test/pytest/test_compare_to_bash.py

Both PRs add to COMPLETE — yours encode_post_codec, mine analyze_reltuples,
projection_drop_column, projection_update. A union of the two lists is the whole
resolution.

Everything else checks out, and I verified rather than read it

11 new ledger rows    all majors=15;16;17;18;19
census                re-derived 1407, states 1407, 0 duplicate keys after the merge
new pair              literal 5 | template 3 | missing: 0
encode_post_codec.sh  11 passed + 0 failed, rc=0, on pg18a

The suite's own output is the evidence the change does what the title says:

-- tail: range=1703495993 body-spread=4903 encoded_vectors=0  bytes=440420
-- rep : encoded_vectors=20 bytes=39906  raw=1600000

Encoding rejected entirely on the outlier-range column, kept on the one where it wins by
40x. That is the decision the PR is about, shown in one line.

The four ways I tried to break the C, and why none of them worked

Recording the negatives, because on a change that builds a second copy of a column chunk
the absence of these is the interesting part.

1. Is col->valueStream cumulative? If it were, rawRegion would be quadratic and
wrong. It is not: initStringInfo(&group->columns[c].valueStream) at line 781 is per chunk
group, and entryRawLen at 1326 reads the same length the new appendBinaryStringInfo
copies. The two regions accumulate in lockstep.

2. Can an encoding ENLARGE a vector, making rawRegion->len > encoded->len false
exactly when raw would win? No. PgColumnarEncodeChunk starts bestLen = rawLen and every
candidate must clear len < bestLen, on the FULL chunk and not only the sample — the
sample picks candidates, the full encode is re-checked. So encLen <= rawLen always, and
the precondition is exactly "an encoding was applied". It is tight, not merely safe.

3. Does the descriptor follow the bytes? Yes, and this is the one that would have been
a corruption rather than a size regression. desc = rawDesc swaps in the all-NONE
descriptor built alongside, and the trailing shared-table length is written
unconditionally so columnar_reader.c's exact-length check is satisfied either way.

4. Does the GUC actually gate the cost? Yes. The copy is inside
if (pgcolumnar_enable_post_codec_encoding_choice), so with the choice off no second
region is built and the flush holds what it held before. Given #1075 measured flush memory
on a 200,000-row load, gating the allocation rather than only the decision is the right
call.

Two small things, neither blocking

rawUsedLevel is set by the call and never read. Harmless, and the compiler will not
complain because its address is taken, but it is a declared variable that answers no
question.

The old desc is not freed when desc = rawDesc replaces it. Almost certainly
context-scoped and reclaimed at flush, so I am noting it rather than claiming a leak — I
did not chase which context these makeStringInfo calls land in.

Verdict

Set cluster_tests to 423 after reseating, union the COMPLETE lists, and this is good to
go. I would not have found the count by reading; the half refusing to run is what says it,
and that is the guard doing its job a few minutes before CI would have.