fix: every columnar node reports Columnar Vectors Skipped, not just the scalar one - #522
Merged
jdatcmd merged 1 commit intoAug 9, 2026
Conversation
…he scalar one "Columnar Vectors Skipped" was emitted at one site, in the scalar custom scan's EXPLAIN. The two vectorized aggregate nodes print Usable Skip Predicates, Chunk Groups Total/Read/Removed, Vector Predicates and Batch Fold -- but not this one. So a plan could not say whether per-vector skipping happened on the aggregate path, which is the path where it matters most: commandprompt#512 is about the batch fold and the skip vector disagreeing, and this is the number that would show it. ## What it shows, now that it is printed Same table, same predicate, the native_vecskip fixture (8192 rows, one row group, 8 vectors of 1024, monotonic id so each vector's min/max is tight): | arm | Columnar Vectors Skipped | | --- | ---: | | scalar custom scan | 7 | | vectorized aggregate (Batch Fold: yes) | 0 | The scalar node skips 7 of 8 vectors. The fold skips none, and reads all eight. That is commandprompt#512 stated as a number rather than as a code reading, and until now the plan could not say it. This is a reporting change only: no counter changes, nothing new is skipped. ## Where it goes Into PgColumnarGroupStats and its one emitter, rather than a second call beside it. That is commandprompt#495's construction and the reason is commandprompt#493's: a counter emitted from one node's callback is a counter the other nodes will not have. The scalar node's output is byte-identical -- the line already sat immediately after the group counters, which is exactly where the shared emitter puts it. Three capture sites needed the field (two ungrouped paths, one grouped), because the aggregates report from a snapshot taken while the read state is still open rather than from the read state itself. Changing one and not the others is how a duplicated cost model goes wrong in this tree. ## Tests In native_vecskip.sh, whose fixture already proves both outcomes are reachable: a selective range skips vectors, a non-selective one skips none. Two premises, and they earned their place immediately. The first version of the aggregate check PASSED while testing nothing, because pgcolumnar.enable_ungrouped_vector_agg defaults to off, the query fell back to the scalar scan, and the scalar scan already printed the line: FAIL premise: the aggregate arm really is a vectorized aggregate: got [no] want [yes] FAIL premise: and it is not the scalar scan: got [yes] want [no] PASS the vectorized aggregate reports Columnar Vectors Skipped <-- vacuous With the GUC set so the node is actually reached, the target check goes red for the right reason, and green with the fix. Refs commandprompt#512, commandprompt#495, commandprompt#493.
jdatcmd
approved these changes
Aug 9, 2026
jdatcmd
left a comment
jdatcmd
left a comment
Collaborator
There was a problem hiding this comment.
Approved. native_vecskip=PASS in CI and not among the six skips.
This turned a reporting gap into evidence for #512, which is more than it was asked to do. Same table, same predicate, only the node differing:
scalar custom scan Columnar Vectors Skipped: 7
vectorized aggregate (Batch Fold: yes) Columnar Vectors Skipped: 0
Before this line existed, no plan could say that. "The fold fetches skipVec and never indexes it" is a claim about source; 7-against-0 is the same fact a reader can check from EXPLAIN. It is cited in #523's body.
Putting it in PgColumnarGroupStats and its single emitter rather than a second call beside it is #495's construction applied for #493's reason — a counter emitted from one node's callback is a counter the other nodes will not have, which is precisely how this gap arose. Three capture sites for two ungrouped paths and one grouped, because the aggregates report from a snapshot rather than the live read state, is the detail that would have been easy to get wrong by adding one call and declaring it done.
One caveat I have already attached downstream
7-against-0 is a hazard number, not a cost one, and I have written it that way into #523 so it cannot be misread. The fold traverses those vectors, but the work it does on them is a scan-key re-check rather than materialisation: teaching it to honour skipVec measures 76.8 ms → 78.4 ms, slower, on a 2M-row single-group fixture with 1,952 vectors skipped and identical answers, because the gather must still run to keep each column's present-index aligned.
So the correct reading is "the fold walks vectors the zone maps ruled out, which is why holes would be read" — not "the fold is paying 8x". Both numbers belong together or someone optimises a 2% regression into place.
The vacuous first version is the transferable part
FAIL premise: the aggregate arm really is a vectorized aggregate: got [no]
FAIL premise: and it is not the scalar scan: got [yes]
PASS the vectorized aggregate reports Columnar Vectors Skipped <-- vacuous
enable_ungrouped_vector_agg defaults off, so the query fell back to a scalar scan that already printed the line, and the check went green having exercised none of the new code. That is the same root as my own failure on #512 four hours earlier — a default silently routing you to the path you are not testing — and in both cases the only thing that caught it was a premise asserting which node actually ran.
Your first probe reporting 0 skipped on both arms while a passing check in the same suite said otherwise is the detail worth keeping: a probe contradicting a green check in its own suite is information, and the probe is the newer claim.
Merging.
ChronicallyJD
added a commit
to ChronicallyJD/pgcolumnar
that referenced
this pull request
Aug 9, 2026
… produced (commandprompt#542) vectorsSkipped was incremented in pgcolumnar_native_skip_current_vector, which only the row path enters. The batch fold takes a group whole through PgColumnarReadFoldNextGroup and never produces rows one at a time, so it never reached the counter: a fold decoding 1 of 59 vectors reported Columnar Vectors Skipped: 0 That is worse than printing nothing. It answers the question wrongly, and it is the number commandprompt#522 added so a plan could say whether per-vector skipping happened on the fold path -- the path where it matters, since commandprompt#512 is about the fold and the skip vector disagreeing. It is now counted from the final nativeSkipVec, after both decode passes, which both paths share. Counted from the MASK and deliberately not as (maxVecCount - groupVecDecoded). That subtraction looks equivalent and is not, because commandprompt#452 phase 1b-ii made the loop two-pass: pass 0 decodes the qual columns against the mask the ZONE MAPS built, refine_skipvec sharpens it to "no row in this vector matches", and pass 1 decodes the payload columns against the sharpened mask. groupVecDecoded is the max across columns over BOTH passes, so on a zero-matching range it is the qual column's full 32 while the payload columns decoded nothing -- the subtraction reports 0 skipped for a group in which every vector was ruled out. That is not hypothetical: it made native_exact_selection's "zone maps rule out nothing" premise read -32, a count that cannot exist. The mask is what says what was skipped, so ask it. The mask-based count agrees with the row path on every fixture measured -- 58 on native_fold_skipguard's, 32 on native_exact_selection's, 30 on a 1b-ii shape -- and adds the fold arm, where the number was 0. arm before after Decoded scalar 58 58 1 fold 0 58 1 so decoded + skipped == the group's 59 vectors on BOTH arms, where it previously held on the scalar arm alone. It also repairs an invariant commandprompt#541 depends on. vectorsRuledOutByValue is declared "subset of vectorsSkipped" and is incremented during mask refinement, which both arms run; on the fold arm it read 1 beside a vectorsSkipped of 0, a subset larger than its superset. The suite half matters as much. native_fold_skipguard read its premise off the SCALAR arm -- "same table, same predicate, same reader" -- and that proxy is what hid this: the premise for the check under test was measured on a plan that was not the one under test. It now reads the fold plan and asserts exact values, making it a WORK-DONE assertion rather than another correctness one. A fold that silently stopped skipping and decoded all 59 vectors passes every correctness check we own, because decoding everything reads real values rather than holes. Also corrects two statements the suite had outlived: a header describing a world where "the fold does NOT honour the skip vector" (commandprompt#452 phase 1b-i ended that), and a check named "the fold answers correctly while decode skips nothing", which contradicted the premise directly above it even when written. Gated on pg18a and pg19a: native_exact_selection 16/16, native_fold_skipguard 6/6, native_vecdecode 24/24, ungrouped_vector_agg 45/45, 0 warnings, built from make clean with no stale objects (commandprompt#536). Removal proof: putting the counter back on the row path fails exactly the two new checks, got [0] want [58] and got [1] want [59].
ChronicallyJD
added a commit
to ChronicallyJD/pgcolumnar
that referenced
this pull request
Aug 9, 2026
… produced (commandprompt#542) vectorsSkipped was incremented in pgcolumnar_native_skip_current_vector, which only the row path enters. The batch fold takes a group whole through PgColumnarReadFoldNextGroup and never produces rows one at a time, so it never reached the counter: a fold decoding 1 of 59 vectors reported Columnar Vectors Skipped: 0 That is worse than printing nothing. It answers the question wrongly, and it is the number commandprompt#522 added so a plan could say whether per-vector skipping happened on the fold path -- the path where it matters, since commandprompt#512 is about the fold and the skip vector disagreeing. It is now counted from the final nativeSkipVec, after both decode passes, which both paths share. Counted from the MASK and deliberately not as (maxVecCount - groupVecDecoded). That subtraction looks equivalent and is not, because commandprompt#452 phase 1b-ii made the loop two-pass: pass 0 decodes the qual columns against the mask the ZONE MAPS built, refine_skipvec sharpens it to "no row in this vector matches", and pass 1 decodes the payload columns against the sharpened mask. groupVecDecoded is the max across columns over BOTH passes, so on a zero-matching range it is the qual column's full 32 while the payload columns decoded nothing -- the subtraction reports 0 skipped for a group in which every vector was ruled out. That is not hypothetical: it made native_exact_selection's "zone maps rule out nothing" premise read -32, a count that cannot exist. The mask is what says what was skipped, so ask it. The mask-based count agrees with the row path on every fixture measured -- 58 on native_fold_skipguard's, 32 on native_exact_selection's, 30 on a 1b-ii shape -- and adds the fold arm, where the number was 0. arm before after Decoded scalar 58 58 1 fold 0 58 1 so decoded + skipped == the group's 59 vectors on BOTH arms, where it previously held on the scalar arm alone. It also repairs an invariant commandprompt#541 depends on. vectorsRuledOutByValue is declared "subset of vectorsSkipped" and is incremented during mask refinement, which both arms run; on the fold arm it read 1 beside a vectorsSkipped of 0, a subset larger than its superset. The suite half matters as much. native_fold_skipguard read its premise off the SCALAR arm -- "same table, same predicate, same reader" -- and that proxy is what hid this: the premise for the check under test was measured on a plan that was not the one under test. It now reads the fold plan and asserts exact values, making it a WORK-DONE assertion rather than another correctness one. A fold that silently stopped skipping and decoded all 59 vectors passes every correctness check we own, because decoding everything reads real values rather than holes. Also corrects two statements the suite had outlived: a header describing a world where "the fold does NOT honour the skip vector" (commandprompt#452 phase 1b-i ended that), and a check named "the fold answers correctly while decode skips nothing", which contradicted the premise directly above it even when written. Gated on pg18a and pg19a: native_exact_selection 16/16, native_fold_skipguard 6/6, native_vecdecode 24/24, ungrouped_vector_agg 45/45, 0 warnings, built from make clean with no stale objects (commandprompt#536). Removal proof: putting the counter back on the row path fails exactly the two new checks, got [0] want [58] and got [1] want [59].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Columnar Vectors Skippedwas emitted at exactly one site: the scalar custom scan's EXPLAIN callback. The two vectorized aggregate nodes print Usable Skip Predicates, Chunk Groups Total/Read/Removed, Vector Predicates and Batch Fold — but not this one.So a plan could not say whether per-vector skipping happened on the aggregate path, which is the path where it matters most. #512 is about the batch fold and the skip vector disagreeing, and this is the number that would show it.
What it shows
Same table, same predicate,
native_vecskip's fixture — 8192 rows, one row group, 8 vectors of 1024, monotonicidso each vector's min/max is tight. Only the node differs:Columnar Vectors SkippedBatch Fold: yes)The scalar node steps past 7 of 8 vectors. The fold traverses all eight. That is #512 stated as a number rather than as a reading of the source, and until this line is printed no plan can say it.
What this number is NOT
It is not a claim that the fold is paying 8x, and it should not be read as a performance defect. The work the fold does on those rows is a scan-key re-check, not materialization.
@jdatcmd measured teaching the fold to honour
skipVec, on a 2M-row single-group fixture with 1,952 vectors skipped: 76.8 ms → 78.4 ms, slower, answers identical. The gather must still run to keep each column's present-index aligned, so skipping saves only the key test and the aggregate apply — less than the per-row vector tracking costs.So on present evidence the visible cost of this traversal is close to zero and possibly negative to remove. The line is worth printing because it makes the hazard legible, not because it exposes a cost: it shows the fold genuinely traverses vectors the zone maps ruled out, which is exactly why holes in a decoded buffer would be read if decode ever started skipping. That is the argument for #512's guard, in data rather than in a code path.
This PR changes reporting only. No counter changes; nothing new is skipped.
Where it goes
Into
PgColumnarGroupStatsand its single emitter, rather than a second call beside it. That is #495's construction applied for #493's reason: a counter emitted from one node's callback is a counter the other nodes will not have — which is precisely how this gap arose.The scalar node's output is byte-identical: the line already sat immediately after the group counters, which is where the shared emitter puts it.
Three capture sites needed the new field (two ungrouped paths, one grouped), because the aggregates report from a snapshot taken while the read state is still open rather than from the read state itself. Changing one and not the others is how a duplicated quantity goes wrong in this tree.
Tests
In
native_vecskip.sh, whose fixture already proves both outcomes are reachable — a selective range skips vectors, a non-selective one skips none.Two premises, and they earned their place immediately. The first version of the aggregate check passed while exercising none of the new code:
pgcolumnar.enable_ungrouped_vector_aggdefaults to off, the query fell back to the scalar scan, and the scalar scan already printed the line.With the GUC set so the node is actually reached, the target check goes red for the right reason and green with the fix.
Gate
native_vecskip13 checks / 0 fail andpushdown_report45 checks / 0 fail, 0 warnings each.pushdown_reportcarries the one-emitter-per-label checks, which is what a new EXPLAIN line most risks breaking.Refs #512, #495, #493.