Skip to content

fix: push scan keys in the batch fold to prune row groups (#349 item 2) - #354

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
ChronicallyJD:fix/349-fold-scan-keys
Aug 3, 2026
Merged

fix: push scan keys in the batch fold to prune row groups (#349 item 2)#354
jdatcmd merged 1 commit into
commandprompt:mainfrom
ChronicallyJD:fix/349-fold-scan-keys

Conversation

@ChronicallyJD

Copy link
Copy Markdown
Collaborator

What

Fixes #349 item 2: the ungrouped batch fold opened the reader with no scan keys, so it read and decoded every row group even when the query's zone maps could rule most out. On clustered / range-partitioned data — the workload the fold targets — a selective indexed-column filter that the row path prunes to a couple of groups made the fold decode all of them.

Measured

Clustered fixture, selective filter:

chunk groups read removed by filter
before (fold) 200 / 200 0
after (fold) 2 / 200 198

Same as the row path, identical answer.

How

The fold already builds the scan keys (for its inline per-value recheck); it just passed 0, NULL to ColumnarBeginRead. Pass them, and columnar_native_load_group prunes whole groups its zone maps rule out. The fold still walks every surviving group in full and rechecks the WHERE inline, so the result is unchanged. Per-vector skipping within a surviving group is deliberately left out — it would need to step the present index past a skipped vector, and the packed present-value stream is built whole regardless of the skip vector, so walking all rows stays correct.

Tests / gate

test/ungrouped_vector_agg.sh gains a clustered fixture asserting the fold removes groups by filter, removes the same groups the row path does, and returns the row path's value. Gate: preflight all 5 majors, full assert matrix PG18+PG19, pg18_san (ungrouped/parallel/native_agg/native_index) — all green (the only red is the pre-existing analyze_stats wall-clock flake, which fails identically on clean main).

🤖 Generated with Claude Code

…mpt#349 item 2)

The ungrouped batch fold opened the reader with no scan keys, so it read and
decoded every row group even when the query's zone maps could rule most out. On
clustered or range-partitioned data -- the workload the fold targets -- a
selective indexed-column filter that the row path prunes to a couple of groups
made the fold decode all of them: measured 200/200 groups read vs the row path's
2/200, ~100x the work.

The fold already builds the scan keys (for its inline per-value recheck); it just
did not hand them to ColumnarBeginRead. Pass them, and the reader prunes whole
groups its zone maps rule out, exactly as the row path does. The fold still walks
every surviving group in full and rechecks the WHERE inline, so the answer is
unchanged. Per-vector skipping WITHIN a surviving group is left out (it would need
to step the present index past a skipped vector); columnar_native_load_group
builds the packed present-value stream whole regardless of the skip vector, so
walking all rows is correct.

test/ungrouped_vector_agg.sh gains a clustered fixture asserting the fold now
removes groups by filter, removes the same groups the row path does, and returns
the row path's value. 32/32 on PG18 assert; ungrouped/parallel/native_agg suites
unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UX1jrWiQsJJA1t4pkmkb4T
@jdatcmd

jdatcmd commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Verified independently, on my own clustered fixture rather than the one in the PR.

400,000 rows, 200 groups at stripe_row_limit => 2000, filter k BETWEEN 1000 AND 3000:

arm batch fold groups read removed by filter value
GUC off (row path) no 2 198 2001|145170
GUC on yes 2 198 2001|145170
heap oracle 2001|145170

The fold now prunes exactly the groups the row path prunes, and returns the row path's answer, which is also heap's answer.

Suite: 32/32 on all five majors (PG15, 16, 17, 18, 19), run locally on a branch already containing merged main.

On the correctness argument in the body, which is the part worth checking rather than taking on trust: leaving per-vector skipping out is sound because columnar_native_load_group decodes the packed present-value stream whole and builds the skip vector afterwards, so a fold that walks every row of a surviving group and advances the present index over each stays correct regardless of what the skip vector says. Group-level pruning is separately safe because zone maps only exclude groups no row can match. Both hold in the code as written.

Good catch on the framing too: the fold already built the scan keys for its inline recheck and simply passed 0, NULL. That is the cheapest possible version of this fix.

No objections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants