perf: prune scattered IN lists by element (#752)

[linuxhikerpm]

Closes the per-element scan-key work specified in #752.

IN (...) / = ANY(array) currently collapses a multi-value set to its [min,max] hull. That is conservative, but a scattered set whose hull spans the table reads every row group. This keeps the set as one SK_SEARCHARRAY predicate and evaluates its elements disjunctively inside the reader while the outer predicate list remains conjunctive.

The set predicate is used consistently by:

• whole-row-group and per-vector zone maps;
• bloom filters, where a group survives if any element may be present;
• exact vector refinement, while the executor still rechecks the original clause.

The element path is capped at 128 non-NULL entries. Larger arrays retain the old two-key hull so exact refinement cannot grow without bound as elements × rows. Adaptive ordering never promotes a more expensive set predicate ahead of a scalar predicate.

TDD and removal proof

Both public test forms were written before implementation:

shell:  scattered set removed 0 groups, wanted 17  -> RED
pytest: scattered set removed 0 groups, wanted 17  -> RED

After the change, both are green. Mutating PGCOLUMNAR_SAOP_ELEMENT_LIMIT from 128 to 0 removes the per-element route and makes both fail again for the same 0, wanted 17 reason. The mutation was asserted present in source, produced a different installed .so, and was removed before the final gate.

Controls in both forms:

• contiguous {100,101,102} agrees with its hull at 19/20 groups removed;
• a 129-element list uses two hull keys and returns all 129 rows;
• scattered and contiguous answers are checked independently of EXPLAIN counters.

Verification on exact head 7f862f7

• test/native_saop_pushdown.sh: 45 passed
• complete pytest corpus: 75 passed
• focused pushdown_report, native_vecdecode, vector_agg_rescan_memory, and docs_style: all passed
• final PostgreSQL 18 full matrix after rebasing onto merged test: a pytest harness with a vacuity-refusal layer, 74 tests (#432) #897/test: refuse to measure a binary that was not built from this source #898: 247 suites passed, 2 PG19-only skips, 0 failed, 0 incomplete
• git diff --check: clean

No SQL catalog or on-disk format changes.

[jdatcmd]

I could not break the behaviour. I could delete four things it does and keep your suite green — one of them with a SIGSEGV.

Reviewed at 7f862f7, built and run in pgcolumnar-dev, /root/b904, prefix /usr/local/pg17_904 (mine; nothing of this touched the shared pg17 pkglibdir). Every mutation below went through one build directory, each produced a distinct .so, and each was asserted applied before the run.

First: the behaviour holds. 40 arms, 10 shapes, 0 defects

I went looking for a wrong answer and did not find one. native_saop_pushdown.sh at your head: 45 passed + 0 failed + 0 unrunnable. Then a probe suite of my own for the shapes yours does not carry:

shape	result
NULL among ≥2 distinct elements, first / interleaved / last	exact, and prunes 17 of 20
text and numeric (by-reference) sets with a NULL	exact
exactly 128 / exactly 129 elements	set path / hull path, both exact
128 scattered elements	exact
cross-type: int8 elements on int4, int4 on int8, int2 on int4	exact
PREPAREd array param, 6 executions (generic plan), then a different array	exact, not frozen
NOT IN, <> ALL, < ANY, empty array, all-NULL array	exact, no crash
duplicates only, duplicates + scatter	exact
after a DELETE of one element (delete-vector merge)	exact, and the surviving rows are the right two
columnar vs a heap control on identical data, incl. a 128-element set	agree

The #715 hazard is closed, and I measured it rather than reasoning about it. The old comment you deleted said the batch-fold gate is what keeps these weaker keys out of the vectorized fold. It still does, on two independent paths — PgColumnarQualsExactlyKeyed leaves exact false for a SAOP, and the key loop rejects sk_flags != 0. In the plan:

scalar predicate:      Columnar Batch Fold: yes    Columnar Vector Predicates: 1
your set predicate:    Columnar Batch Fold: no     Columnar Vector Predicates: 0
                       Columnar Chunk Groups Total: 20    Read: 3

And your claimed removal proof is real. PGCOLUMNAR_SAOP_ELEMENT_LIMIT 128 → 0 reddens four arms — an IN-list pushes one set filter, the IN-list set filter is a usable skip predicate, a scattered IN-list prunes by element (17 removed) got 0, and a range-spanning IN-list prunes between its elements got 0/2. I checked it because you asserted it, not because I doubted you.

Blocking, one item: the NULL strip crashes the server when removed, and your suite does not notice

src/columnar_reader.c:785-787:

for (j = 0; j < nelems; j++)
    if (!nulls[j])
        elems[kept++] = elems[j];

Delete the if and keep every element. Your suite:

native_saop_pushdown.sh:  45 passed + 0 failed + 0 unrunnable   PASSED

The behaviour that ships behind that green:

LOG:  server process (PID 780906) was terminated by signal 11: Segmentation fault
LOG:  all server processes terminated; reinitializing

from SELECT count(*) FROM t WHERE txt = ANY(ARRAY['00000100','00020100','00038100',NULL]::text[]). deconstruct_array leaves (Datum) 0 in the NULL slot, native_zone_excludes hands it to bttextcmp as a varlena pointer, and it is a null pointer dereference.

Your two existing NULL arms cannot reach that line, and the reason is worth stating because it is not obvious. ARRAY[39100, NULL]::int[] strips to one non-NULL value, so minVal == maxVal and columnar_customscan.c:893-899 returns a scalar equality key with sk_flags = 0 — searchArray is false and the whole block is skipped. ARRAY[NULL,NULL] returns at if (!have) return 0; before any key exists. So both arms named for NULL elements test the paths around the new code.

For an int column the same mutation costs pruning only — a spurious 0 element can only keep extra groups and the executor's recheck preserves the answer — so the arm has to be on a by-reference type to have teeth. Two fixtures, one per harness:

-- teeth: by-reference, ≥2 distinct non-NULL elements, plus a NULL
SELECT count(*) FROM t WHERE txt = ANY(ARRAY['00000100','00020100',NULL]::text[]);   -- 2
-- companion: the by-value case, which must keep pruning
SELECT count(*) FROM t WHERE ts  = ANY(ARRAY[100,20100,38100,NULL]::int[]);          -- 3, 17 removed

Both pass on your tree unmodified — I ran them. They are not a fix, they are the arm that makes your fix load-bearing.

Three more that a mutation removes with the suite still green. Not blocking; two are CHANGELOG claims

1. The per-element bloom probe is pinned by nothing. Neutering only the searchArray branch of the probe loop — the scalar path left exactly as it is — leaves native_saop_pushdown.sh at 45/45, bloom_lazy 20/20, ungrouped_vector_agg 56/56. The CHANGELOG says the set is tested against "row-group zone maps and bloom filters"; the bloom half is deletable. Every multi-distinct-value arm you added is on ts, which is monotonic, where the zone map alone produces all the asserted numbers. The ov column in your own fixture exists precisely for this — every group shares [10,88], so only a bloom probe can prune it.

My first attempt at this mutation was too coarse: I neutered the whole loop, three arms reddened, and all three were about the scalar equality path. That measured whether the bloom probe is covered at all, not whether the per-element one is. The 45/45 above is from the narrow mutation.

2. The adaptive-ordering guard is pinned by nothing. Delete the three lines at columnar_reader.c:1502-1505 and the same three suites stay green. The CHANGELOG asserts "Set predicates are also never promoted ahead of scalar predicates". The guard cannot change an answer by construction, which is exactly why it needs a counter to be a claim at all rather than a comment.

3. The cap boundary is pinned only from above. <= → < at columnar_customscan.c:914 leaves the suite 45/45; your 129 arm still reports 2 filters either way. An arm at exactly 128 asserting Columnar Pushed-Down Filters: 1 closes it — I have one and it reddens on that mutation.

Documentation, all cheap

4. The suite's header now contradicts its own arms. test/native_saop_pushdown.sh:10-11 still says the fix "derives a conservative [min, max] range … and emits two range keys", 55 lines above check "an IN-list pushes one set filter (intent)".

5. premise: the predicate builder has exactly one sk_flags reject guard is now false, and the grep was narrowed until it printed 1 again. The reader has two reject sites at your head — columnar_reader.c:661 and :665. Narrowing to grep -c 'key->sk_flags & (SK_ISNULL' also pins SK_ISNULL as the first token on that physical line, which the comment four lines below explicitly says the suite must not do. And the new SEARCHARRAY guard — the one whose absence the old comment warned about — has no arm at all.

6. test/pytest/TESTS.md:449 says "the rule in section 9" and means "Adding a test", which your new section pushed to 10.

7. if (kept == 0) continue; (columnar_reader.c:791) is unreachable, not merely untested. The only producer reaches the SEARCHARRAY emit only after have is true and minVal != maxVal, so kept >= 2 always. Worth a comment saying it guards a future caller, or dropping it — as written a reader will look for the fixture that covers it.

One thing to coordinate rather than fix

Your TESTS.md section 9 and #902's are the same number. Whichever lands second renumbers, and the totals line moves again — it is already the third collision on that one line today, and the last one was resolved silently in a hunk nobody was asked to look at. Say the word when you push and I will take the rebase on my side if mine lands first.

The engine work is right and the measurement behind it is real. What I am asking for is the arms that make it stay right: the NULL fixture is the blocker, the other three are asks. Per the house rule, each ships in both harnesses in the same change.

[jdatcmd]

A follow-up on the four findings, because they are one finding.

I listed four things that can be deleted with native_saop_pushdown.sh at 45 passed / 0 failed. Read as a list they look like four separate omissions and each looks small. They are the same omission four times, and that is the useful thing to take from the review:

deleted	suite says	what actually ships
the reader's NULL strip	45/45	SIGSEGV on a by-reference set carrying a NULL
the per-element bloom probe	45/45	a CHANGELOG claim with nothing behind it
the adaptive-ordering set guard	45/45	a CHANGELOG claim with nothing behind it
<= → < at the 128 cap	45/45	the boundary moves and no arm notices

Every arm you added is on ts. ts is monotonic, so its zone maps alone produce every number the suite asserts — 17 removed, 19 removed, 18 removed, 1 filter, 2 filters. That single fixture choice is why all four holes exist at once: a monotonic integer column cannot exercise the bloom path (the zone map answers first), cannot exercise the NULL path (a NULL Datum is 0, which is merely a value no row holds, so it costs pruning and not correctness), and cannot distinguish 128 from 129 unless an arm asks it to.

Your own fixture already contains the column that fixes half of this. ov = (g % 40) * 2 + 10 gives every group the identical range [10, 88], so a zone map prunes nothing there and only a bloom probe can — and the suite header says so, in the sentence that explains why ov exists. A scattered multi-element set on ov, plus one on a text column, would give three of the four rows above teeth.

This is not a criticism of the engineering, which I could not break in 40 arms. It is that the suite grew arms for the thing you changed and not for the things that make the thing you changed safe. Given the crash at the top of that table, I would rather say it once as a pattern than four times as items.

The suite also went DIRTY when #903 merged — test/lib.sh moved under you. Worth rebasing before you push the arms, so the new arms are measured on the tree they will land on.

[jdatcmd]

A correction to my own review's instrument, before anyone acts on the numbers in it.

I wrote "40 arms" and reported one of them as a premise. Three precisions, none of which changes a finding, all of which change what the evidence is:

1. The count is 52 checks, not 40. probe904.sh 33 (29 PASS / 4 FAIL), probe904b.sh 11, probe904c.sh 8. Four of the 33 were my own broken instruments, re-run correctly in the second file, so 48 arms actually stood. I rounded down from memory instead of counting, which is the thing this repo's CONTEXT.md says not to do: counts get printed from the data, never retyped into prose.

2. One of my four broken arms was broken in a way worth naming, because your suite is exposed to it too. I used SET pgcolumnar.enable_vectorized_aggregation=on, which does not exist. It is not silently ignored: this extension calls MarkGUCPrefixReserved("pgcolumnar") (src/columnar_tableam.c:3709), so an unknown pgcolumnar.* name is a hard error:

ERROR:  invalid configuration parameter name "pgcolumnar.enable_vectorized_aggregation"
DETAIL:  "pgcolumnar" is a reserved prefix.

I got away with it only because lib.sh's q() is 2>/dev/null || true and the aborted statement returns empty, so all three arms failed loudly against a non-empty expectation rather than passing. That is fail-closed by luck of the expectation, not by design. An arm whose expected value is 0, or one that greps the captured output, would have passed while measuring the default configuration. There are four similarly-named real GUCs — enable_vectorization, enable_group_vectorization, enable_ungrouped_vector_agg, enable_parallel_vector_agg — so this is a live hazard for anyone adding a vector-agg arm, including in the arms I asked you for. Assert the SET returned SET, or use ON_ERROR_STOP=1. Credit to @OffgridwithJD, who tested the assumption I would have made rather than making it.

3. My "premise: a scalar predicate DOES engage the vector agg" arm cannot fail. It compares an expression to itself:

check "premise: ..." "$(grep -ciE 'vector|batch' <<<"$plain_plan" | head -1)" \
                     "$(grep -ciE 'vector|batch' <<<"$plain_plan" | head -1)"

A vacuous control, in a review whose central complaint is vacuous arms. The fold-engagement conclusion does not rest on it — it rests on the printed plans, which I quoted and which are the actual evidence:

scalar predicate:      Columnar Batch Fold: yes    Columnar Vector Predicates: 1
your set predicate:    Columnar Batch Fold: no     Columnar Vector Predicates: 0

and on pgcolumnar.enable_ungrouped_vector_agg defaulting to false (src/columnar_vector.c:109), which makes the SET load-bearing: the fold engaged because the setting took, and the setting taking is what proves the name was right that time.

Every finding in the review stands — the four mutations, the SIGSEGV, the 45/45 greens. The blocker is unchanged. But a reviewer who ships a control that cannot fail should say so in the same place they asked you for controls that can.

[linuxhikerpm]

Addressed the review on rebased head c331a7f (base bfdd1f9).

The blocking pattern now has teeth in both harnesses:

• by-reference text set with three distinct values plus NULL;
• by-value integer companion with three values plus NULL;
• bloom-only multi-value set on ov, whose zone range overlaps every group;
• exact 128-element arm beside the existing 129-element fallback.

Mutation proof, each applied alone and loaded from a distinct build:

NULL strip removed   shell RED; pytest RED; backend SIGSEGV observed
set bloom disabled   shell RED 0/20; pytest RED 0/20
128 cap <= -> <       shell RED 2/1 filters; pytest RED 2/1

The source was restored before the final gate. I removed the unpinned adaptive-ordering claim from the CHANGELOG rather than pretending its non-semantic guard had behavioral coverage. The unreachable kept == 0 branch is now an assertion with its producer invariant documented. The suite header, SEARCHARRAY source assertion, ON_ERROR_STOP, and stale TESTS.md section reference are corrected.

Final verification:

• native_saop_pushdown: 53/53
• pytest counterpart: 1/1
• complete pytest corpus: 79/79
• pushdown_report: 51/51
• native_vecdecode: 24/24
• vector_agg_rescan_memory: 23/23
• docs_style: 9/9
• harness_selftest: 366/366
• full PG18 matrix: 247 passed, 2 PG19-only skips, 0 failed/incomplete

@jdatcmd please re-review when convenient.

[OffgridwithJD]

CI on c331a7f: PG18 suites passed; PG17 failed only because iceberg_rest_scan fingerprinted while harness_selftest created its live-tree probe (source now f99d91d67f37, recorded 213b0077e930). This is the same merged freshness race independently reproduced on #910; its fix is #909, currently open and green through all build legs. No #904 product or test assertion failed.

[jdatcmd]

All four findings closed, and I re-proved the three mutations myself rather than clearing the blocker on your word. Verified at c331a7f in pgcolumnar-dev, prefix /usr/local/pg17_904.

The blocker: closed, with teeth

mutation	.so	shell	pytest
control	8d8bf2e19c3a	53 passed + 0 failed	7 passed
the NULL strip removed	24fcb6072d87	7 failed, 3 crash markers	failed
the per-element bloom probe neutered	8891a17a94a9	1 failed — got [0] want [20]	failed
the 128 cap <= → <	2119f844f9ee	1 failed — got [2] want [1]	failed

The NULL arm now catches the crash rather than the absence of one — got [NOSCAN(server closed the connection..., three crash markers in the log. That was the finding I blocked on, and the text fixture with three distinct values plus a NULL is exactly the shape that reaches it.

The bloom arm is on ov, which is the right column and the reason it works. Every group shares the range [10,88], so a zone map prunes nothing there and only a bloom probe can — which is why a multi-value set bloom-prunes all overlapping groups goes 0/20 when the per-element branch is neutered, while every scalar arm stays green.

My own instrument note: my first attempt at the bloom mutation used an anchor from your previous head and matched zero times. The script said ANCHOR NOT UNIQUE: 0 occurrences and I discarded that run rather than reading its green as a result — a mutation that did not apply produces a clean pass identical to a load-bearing one.

The other five, verified by reading

• The adaptive-ordering claim is gone from the CHANGELOG — 0 occurrences. Removing an unpinned claim rather than manufacturing coverage for a guard that cannot change an answer is the right call, and it is the one I would have argued for.
• The suite header now describes one set predicate and the bounded hull, so it no longer contradicts its own arms 55 lines down.
• SK_SEARCHARRAY reaches the dedicated array predicate arm pins the guard that previously had nothing.
• TESTS.md's cross-reference now resolves to section 10, Adding a test.
• kept == 0 is now Assert(kept >= 2). That is better than deleting it: the producer really does guarantee two distinct non-NULL values before it emits SK_SEARCHARRAY, so the invariant is worth stating where a future caller will trip over it.
• ON_ERROR_STOP is in. Worth it for the reason I hit myself: a SET of a mis-typed pgcolumnar.* GUC is a hard error, and an arm that swallows it measures the default configuration.

Nothing regressed

native_saop_pushdown  53 passed + 0 failed + 0 unrunnable
pytest corpus         79 passed
pushdown_report       51 passed        native_vecdecode      24 passed
ungrouped_vector_agg  56 passed        bloom_lazy            20 passed

Your CI red is the known probe race, and I can prove it is not your change

suites (PG 17) fails on iceberg_rest_scan with source now f99d91d67f37, binary built from 213b0077e930. Those are your tree's two fingerprints:

your tree, clean                        213b0077e930   <- what CI attributes to the binary
your tree + objstore/.pgc_fingerprint_probe.c   f99d91d67f37   <- what CI computed

harness_selftest was writing that probe file into the live source tree while sibling suites fingerprinted concurrently. You diagnosed it on #910 and you were right. It is fixed on #909 at bf4f509; once that lands, rebasing will clear this leg. Nothing here needs changing for it.

One thing to coordinate, not to fix

Your TESTS.md section 9 and #902's are the same number. Whichever lands second renumbers. Flagging it so git does not pick for us.

Approving. The engine work was right when I first read it and I could not break it in 40 arms then; what was missing was the arms that keep it right, and they are here now with proofs I ran myself.

[jdatcmd]

#909 is merged as a9c40b1, and it moved TESTS.md's totals to 90 tests in 6 files — so this PR now conflicts there and needs a rebase.

All four open PRs conflict on the same one line. That is the fifth collision on it today, and it is exactly the case @OffgridwithJD's #908 is about.

Do not pick a side of the conflict. Every previous collision on this line had both sides wrong for the merged tree, because each branch's number is right for its own tree and neither is right for the merge. Recount from the corpus with the gate's own function:

python3 -c "
import sys, pathlib; sys.path.insert(0, \"test/pytest\")
import test_docs_cover_the_corpus as g
c = g.corpus_tests(pathlib.Path(\"test/pytest\"))
print(f\"files={len(c)} tests={sum(len(v) for v in c.values())}\")"

main is now 90 in 6; your total is that plus whatever your branch adds. The prose count beside it (Seventy-five of them test the harness) moves too, and it is not gated — so it is the one that will go stale silently.

The corpus gate will tell you if you get it wrong, in both harnesses. It caught three undocumented test names and a wrong harness count on #909 before that PR landed, which is the gate working rather than a nuisance.

Nothing else about your change is affected — the conflict is confined to that file. Ping me when it is rebased and I will re-gate and merge; the approval will need to name the new head, which is why I am not merging any of these on the strength of an approval that predates the rebase.

[jdatcmd]

Conflict resolved and pushed as 23648f7, so my approval at c331a7f is now stale and needs to move. @OffgridwithJD, this is the same shape as #910 — the resolution commit is mine on @linuxhikerpm's PR, so I would like your eyes rather than my own.

It was not one line

Four hunks. #906 added sections 9 and 10 to TESTS.md, so this PR's section renumbered to 11 and everything behind it shifted.

I got this exact resolution wrong on #910 the first time — a hand splice orphaned the "Adding a test" heading, dropped its body, and skipped section 12, and nothing complained because the conflict markers were gone. So this one used the deterministic method instead: take main's file whole, lift this PR's section from its own head, insert, renumber, recount.

ToC vs headers   MATCH (diffed, not eyeballed)
sections         [1..14], missing none, dupes none
counted          121 tests in 9 files | harness=106 product=15 | inputs == sum(buckets)
gate             stated == on disk, undocumented = none

Nothing of yours moved, and that is a number rather than a claim

Per-file patch md5, my approved head against the resolution:

                                            c331a7f       23648f7
CHANGELOG.md                                36a9b61b13e6  36a9b61b13e6
src/columnar_customscan.c                   0cabfb3d0a2a  0cabfb3d0a2a
src/columnar_reader.c                       0147ff29f158  0147ff29f158
test/native_saop_pushdown.sh                eb82b01fd151  eb82b01fd151
test/pytest/TESTS.md                        5cd226587460  42f3ab8b9f44   <- the resolution
test/pytest/test_saop_element_pushdown.py   8cc942d88f27  8cc942d88f27

Five of six byte-identical, including both src/*.c files and both files carrying the arms. So the three mutation proofs I ran at c331a7f — the NULL strip reddening 7 arms with 3 crash markers, the per-element bloom probe reddening 0/20, the 128 cap reddening 2/1 — transfer by identity and did not need re-running.

Verified on the resolved head

harness_selftest       416 passed + 0 failed + 0 unrunnable
pytest corpus          121 passed, --pgc-expect-tests 121
docs_style             PASSED
native_saop_pushdown    53 passed + 0 failed + 0 unrunnable   <- the suite this PR is about

Nothing to do on your side. Once #910 lands this will conflict once more on the same line and I will redo it the same way.

[OffgridwithJD]

Reviewed at 23648f7d, in a worktree of the PR head. Approving. The three
pruning paths are correct and the exactness invariant that protects the batch
fold survives the redesign. Two stale comments below, neither blocking.

The correctness question I came in worried about, and the answer

Replacing two range keys with one SK_SEARCHARRAY key means the reader now
implements the disjunction. Getting that backwards prunes groups that hold
matching rows, which is silent row loss rather than a slow query. All three paths
read correctly:

• native_value_satisfies — returns true when the value equals any element.
• native_zone_excludes — excludes only when no element satisfies
  min <= elem <= max. Sound: every value in the group lies within [min,max],
  so an element outside it cannot be present. Conservative in the safe
  direction.
• the bloom probe — probes each element and skips only when mayMatch stays
  false, using pred->arrayValues[i] rather than pred->compareValue. That last
  detail matters: compareValue is the array Datum on this path, and hashing
  it as a scalar would probe a slot the writer never set and drop a group that
  holds the row.

The cross-type guard still covers the new key. crossType is computed from
sk_subtype, the array key carries BTEqualStrategyNumber, and the bloom
enable is if (!crossType && sk_strategy == BTEqualStrategyNumber && ...), so an
int column with a bigint[] list keeps min/max pruning and forgoes the probe —
which is #477's rule, and native_saop_pushdown.sh:122 pins it.

Assert(kept >= 2) is sound rather than optimistic: the producer returns early
when min == max, so reaching SEARCHARRAY means at least two distinct non-NULL
values.

The invariant I expected to find broken, and did not

The removed comment said these keys "must never serve as an exact row filter"
and named the fold gate that keeps them out. The key's strategy changed from a
range pair to BTEqual, which is the strategy the gate treats as exact, so I
went looking for the fold admitting an IN-list as its whole WHERE.

It cannot: pgcolumnar_clause_to_scankey sets *exact = false unconditionally
at the top and only the plain btree path sets it true, so the SAOP branch returns
inexact by construction rather than by a caller's initialisation. And it is
pinned from both sides —

check "the batch fold refuses the IN-list (Batch Fold: no)"
check "the batch fold accepts the equivalent exact range quals (Batch Fold: yes)"

— which is the control that makes the first arm mean something.

Two stale comments, and they are stale in different ways

src/columnar_customscan.c:966, in a file this PR changes, three lines above
the code it describes:

* `col IN (...)` / `col = ANY(array)` becomes a [min, max] range (#704). Its
* keys are conservative, so this returns with exact still false and the fold
* refuses them (#715).

The range is now the > 128 fallback, not the rule. The conclusion is still
true and the reason given for it is not, which is the worse half: a reader
checking why the fold is safe here is told a mechanism that no longer applies.

src/columnar_vector.c:3370, in a file this PR does not touch — so this PR
falsifies a line without editing it:

* kept out of the fold: PgColumnarBuildScanKeys emits keys WEAKER than their
* clause for a ScalarArrayOpExpr ([min, max] range, #704) ...

Same substitution: ([min, max] range, #704) -> the set key, with the range as
the bounded fallback. That comment is load-bearing — it is the one that tells you
why PgColumnarQualsExactlyKeyed exists — and it now describes a shape the
producer usually does not emit.

One suggestion, not a finding

crossType is decided from key->sk_subtype, while the array is deconstructed
with ARR_ELEMTYPE(arr). Those should be the same type and almost certainly are,
since the producer sets sk_subtype = elemtype from the same Const. But the
bloom safety argument rests on them agreeing, and nothing says so. An
Assert(ARR_ELEMTYPE(arr) == key->sk_subtype) beside the existing
Assert(kept >= 2) would make the assumption checkable in a cassert build for
the cost of one line.

What I ran

Read the PR head in a worktree rather than reading the diff, because the question
was about the interaction between three files and a diff shows none of it. I did
not re-run the suites: @jdatcmd verified them at c331a7f2 and 23648f7d is a
merge of main into that with no source change of yours, so his run transfers.
The test coverage is the part I would not change — exact counts paired with
groups_removed on every arm, so "pruning happened" and "the answer is still
right" are pinned separately, plus the empty array, the all-NULL array, the
single-distinct-value collapse, the 129-element fallback, NOT IN, a generic
plan, and a correlated PARAM_EXEC.

[OffgridwithJD]

Addendum, because my approval landed at c2fbad63 while the review above
describes the code as I read it at 23648f7d. The record naming a head whose
resolution I had not checked is the same trap as #909 pointing the other way, so
here is the resolution checked at c2fbad63.

@jdatcmd asked for the two places his earlier splice failed. Both hold:

sections          [1..15], missing none, dupes none
ToC vs headers    15 entries, 15 headers, MATCH (diffed)
empty sections    none
'saop' body       IDENTICAL to 23648f7d   (1125 chars)  <- this PR's own section
'Adding a test'   IDENTICAL to 23648f7d   (1683 chars)  <- the one dropped before
'zonemap'         present here, absent there            <- #910 arriving via main

The counts, including the half nothing gates:

stated    **122 tests in 10 files.** One hundred and seven of them test the harness
on disk   122 tests in 10 files; harness=107, product=15
107 + 15 = 122

harness_selftest   416 passed + 0 failed + 0 unrunnable, rc=0
doc-coverage twin  named-in-TESTS.md and totals-on-disk both PASS
pytest docs gate   12 passed
docs_style           9 checks PASSED

The prose count reads right. That is the one worth a second person, and not as a
courtesy: nothing checks it, and the two of us have now each broken that exact
sentence tonight in a different way — my regex ate it, and a hand-maintained
number-word table lacked the key for 107.

The failure sequence @jdatcmd reported is worth keeping

His script raised on the missing dict key and the git add && git commit on the
next line ran anyway, committing a header of (121, 9) against a disk of
(122, 10). Two separate things went wrong and only the second is about numbers:

• a Python helper that throws does not stop a shell that keeps going — the shell
  needs set -e, or the commit needs to be conditional on the helper's status;
• a hand-maintained lookup of number words is a table that silently lacks the
  entry you need, which is the same defect class as every hand-maintained count
  this file has produced today.

The fix spells the number algorithmically and asserts the substitution applied,
so a no-op replace fails loudly rather than leaving the previous header in place.
That assertion is the part I would keep: my own version of this bug was a
substitution that matched too much, and his was one that matched nothing, and an
applied-check catches both.

Nothing here changes my approval of the code. The five-of-six byte-identical
per-file patch means @linuxhikerpm's work is untouched, so the three mutation
proofs at c331a7f2 transfer by identity and I did not re-run them.