Finish the plan-shape sweep #203 started

[ChronicallyJD]

#203 asserted the node in the three files you named and I flagged the rest as a sweep left undone. This is the rest of it.

What was exposed

I surveyed all 28 suites that read EXPLAIN, narrowed to the 13 that pull a numeric counter out of it and assert on the number, and found six with no node assertion at all:

native_skip, native_bloom, native_cluster, native_recluster, sorted_projection, write_minmax_fastpath

Columnar Chunk Groups Removed by Filter and Columnar Vectors Skipped are reported by the columnar custom scan and by nothing else. When the planner stops choosing it, the read returns nothing and the check fails describing skipping, or bloom, or clustering — anything except the plan change that actually happened.

The helper

In test/lib.sh as pgc_is_columnar_scan, once rather than copied six times.

It uses EXPLAIN without ANALYZE: the marker line comes from the plan rather than the run, which I confirmed rather than assumed, so the assertion costs a plan and does not execute the query. And it greps positively for the node's own marker rather than testing for absence — a plan that fell back to a sequential scan has no Columnar lines to be absent, so an absence test would pass for exactly the case worth catching.

Two suites deliberately left alone

• analyze_stats reads Rows Removed by Filter, a generic executor counter, and compares the same query with and without an index. Its own comment says the comparison is independent of plan shape, and it is. An assertion there would contradict what the check is for.
• audit greps for Pushed-Down Filters: 1 and requires the value, so it cannot pass without the node. Only the failure message would improve, and not enough to justify touching that file.

Verified by taking the node away

With enable_custom_scan=off, every one of the six fails its node assertion ahead of the counter checks that assertion guards, while the parity checks around them keep passing — which is the point, since parity cannot see a plan change:

===== native_cluster =====
PASS  row count
PASS  box parity (pre-cluster)
FAIL  the plan under test is a columnar custom scan
PASS  row count after cluster
PASS  box parity (post-cluster)
PASS  full-table parity after cluster
FAIL  clustering increases 2D skipping
FAIL  clustering skips most groups

Gate — read this part

• Build preflight: 15.18, 16.14, 17.6, 18.4, 19beta2 — zero warnings.
• Full suite: PG19 all pass. PG18 FAILED, on native_fetch_position and nothing else.
• All six swept suites: PASS on both majors.

I am not going to describe that as green. The failing check is doubling the row group does not double the cost of the same fetches, at small=97ms big=205ms — a ratio of 2.11 against a 2.0 bound. Nothing of mine was running alongside it; I had stopped everything for the gate.

It is not this change — this is test-only and does not touch that suite — and here is that claim measured rather than asserted. The same check, run on its own, four times on each side:

alone, main:    1.11  1.32  1.13  1.12
alone, branch:  1.13  1.08  1.15  1.17
inside the matrix:  2.11

The root cause, which is worth more than this PR

run_all_versions.sh runs six suites concurrently:

maxjobs="${PGC_JOBS:-6}"
while [ "$(jobs -rp | wc -l)" -ge "$maxjobs" ]; do wait -n; done

So every wall-clock ratio in the matrix is measured against five other suites and their clusters. "Passes on an idle box" is not a property the gate can ever observe — the gate is never idle, by design.

That reframes the three flaky timing suites of this session. They are not three thin fixtures that met three unlucky boxes; they are three wall-clock ratios measured under six-way concurrency, and the ~1.8x inflation above is the harness measuring itself. #204 fixes one of them properly by giving the fixture forty points of headroom instead of arguing about the box. The other two want the same treatment, and I would rather that be a deliberate follow-up than something I bolt onto a test-only sweep.

Happy to take native_fetch_position and native_cancel next on the same principle if you want them.

[ChronicallyJD]

Status update: the red this PR disclosed is gone, and it was the contention flake, exactly as the PR said.

When I opened this, its gate was red on native_fetch_position under matrix contention — the pre-existing flake, not this PR's checks. #209 has since merged and moved those wall-clock suites into a serial phase, so the cause is now removed in main.

Re-gated the state the merge button produces — main (bde54c6, which carries #209's is_timing_suite) with this branch merged in (d9905af, clean, no conflicts):

• preflight 15.18 / 16.14 / 17.6 / 18.4 / 19beta2, 0 warnings each
• matrix ALL VERSIONS PASSED on PG18 and PG19, 0 failures
• the six suites this PR adds a node assertion to are all green on both majors: native_skip, native_bloom, native_cluster, native_recluster, sorted_projection, write_minmax_fastpath
• native_fetch_position=PASS on both — the suite that was red now runs serially

So the branch is green on current main with nothing in it changed; the earlier red was its stale pre-#209 base measuring #209's absence. Ready when you are.

[jdatcmd]

Approving.

The helper is right and I checked the two things that could make it quietly wrong:

• Positive grep, not absence. Columnar Projected Columns is the node's own marker; a plan that fell back to a seqscan has no Columnar line, so an absence test would pass for exactly the case worth catching. You got this the correct way round.
• The aggregate queries still carry the marker. sorted_projection and write_minmax_fastpath assert on aggregate/groups-removed queries, and the concern would be that a vectorized aggregate plan reports Columnar Vectorized Aggregates instead of Columnar Projected Columns, which would make the new check fail on a normal run. It doesn't — those two suites pass on both majors in your gate, and a passing normal run is itself the proof the marker appears for those queries. The underlying scan still projects under the aggregate.

EXPLAIN without ANALYZE is the right call and you confirmed the line is planner-sourced rather than assuming it. Proof by removal in both directions (fallback fails the node check ahead of the counters it guards; normal passes) is exactly the standard.

On the red

Not this change, and now demonstrably not a fixture-of-the-week either. native_fetch_position is in is_timing_suite as of #209 (merged), so on current main it runs serially, after the parallel batch. Your PR does not touch it. Its 2.11-vs-2.0 was the six-way contention you diagnosed, measured against your own alone: 1.08–1.17 — the harness measuring itself. Re-gating this branch on main now carries the serialization and should clear it.

Your root-cause section is worth more than the PR, and it is the right frame: "passes on an idle box" is not a property this gate can observe, because it is never idle. #204 and #209 took two of the three; native_fetch_position and native_cancel want the same headroom-not-argument treatment. I'd take you up on that offer.

Merging — test-only, correct, and the only red is a now-serialized suite it does not touch.