From 77c300f5ade0b4223320babadadbf2b55e8b213b Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Sat, 8 Aug 2026 18:30:48 -0600 Subject: [PATCH] fix: every columnar node reports Columnar Vectors Skipped, not just the 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: #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 #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 #495's construction and the reason is #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 #512, #495, #493. --- src/columnar.h | 1 + src/columnar_customscan.c | 12 ++++++++++-- src/columnar_vector.c | 7 +++++++ test/native_vecskip.sh | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/columnar.h b/src/columnar.h index 8e43c67a..da2624c5 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -932,6 +932,7 @@ typedef struct PgColumnarGroupStats uint64 groupsTotal; uint64 groupsRead; uint64 groupsRemoved; + uint64 vectorsSkipped; } PgColumnarGroupStats; extern void PgColumnarExplainPushedDown(int64 nfilters, ExplainState *es); diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index 7a495145..67dc5862 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -2170,6 +2170,15 @@ PgColumnarExplainGroupStats(const PgColumnarGroupStats *stats, ExplainState *es) (int64) stats->groupsRead, es); ExplainPropertyInteger("Columnar Chunk Groups Removed by Filter", NULL, (int64) stats->groupsRemoved, es); + /* + * Vectors skipped belongs with the group counters rather than beside them: + * it was printed by the scalar node alone, so a plan could not say whether + * per-vector skipping happened on the vectorized aggregate path -- the path + * where it matters most, since #512 is about the fold and the skip vector + * disagreeing and this is the number that would show it. + */ + ExplainPropertyInteger("Columnar Vectors Skipped", NULL, + (int64) stats->vectorsSkipped, es); } /* ------------------------------------------------------------------------- @@ -2298,9 +2307,8 @@ PgColumnarExplainCustomScan(CustomScanState *node, List *ancestors, gs.groupsTotal = groupsTotal; gs.groupsRead = groupsRead; gs.groupsRemoved = groupsSkipped; + gs.vectorsSkipped = PgColumnarVectorsSkipped(cstate->readState); PgColumnarExplainGroupStats(&gs, es); - ExplainPropertyInteger("Columnar Vectors Skipped", NULL, - (int64) PgColumnarVectorsSkipped(cstate->readState), es); /* * Rows the qual rejected before their remaining projected columns were diff --git a/src/columnar_vector.c b/src/columnar_vector.c index de9980fe..27e99614 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -595,6 +595,7 @@ typedef struct PgColumnarAggScanState bool haveStats; uint64 groupsRead; uint64 groupsSkipped; + uint64 vectorsSkipped; uint64 groupsTotal; /* @@ -696,6 +697,7 @@ typedef struct PgColumnarGroupAggScanState bool haveStats; uint64 groupsRead; uint64 groupsSkipped; + uint64 vectorsSkipped; uint64 groupsTotal; int usablePreds; /* of npreds, how many can exclude (#479) */ } PgColumnarGroupAggScanState; @@ -3268,6 +3270,7 @@ pgcolumnar_native_batch_fold(PgColumnarAggScanState *state, Relation rel, PgColumnarReadStats(rs, &state->groupsRead, &state->groupsSkipped, &state->groupsTotal); state->usablePreds = PgColumnarReadUsablePredicates(rs); + state->vectorsSkipped = PgColumnarVectorsSkipped(rs); state->haveStats = true; state->batchFolded = true; PgColumnarEndRead(rs); @@ -3402,6 +3405,7 @@ pgcolumnar_native_scan_agg(PgColumnarAggScanState *state, PgColumnarReadStats(rs, &state->groupsRead, &state->groupsSkipped, &state->groupsTotal); state->usablePreds = PgColumnarReadUsablePredicates(rs); + state->vectorsSkipped = PgColumnarVectorsSkipped(rs); state->haveStats = true; } @@ -3558,6 +3562,7 @@ PgColumnarExplainAggScan(CustomScanState *node, List *ancestors, ExplainState *e gs.groupsTotal = state->groupsTotal; gs.groupsRead = state->groupsRead; gs.groupsRemoved = state->groupsSkipped; + gs.vectorsSkipped = state->vectorsSkipped; PgColumnarExplainGroupStats(&gs, es); } } @@ -4119,6 +4124,7 @@ pgcolumnar_groupagg_build(PgColumnarGroupAggScanState *state) PgColumnarReadStats(rs, &state->groupsRead, &state->groupsSkipped, &state->groupsTotal); state->usablePreds = PgColumnarReadUsablePredicates(rs); + state->vectorsSkipped = PgColumnarVectorsSkipped(rs); state->haveStats = true; PgColumnarEndRead(rs); @@ -4238,6 +4244,7 @@ PgColumnarExplainGroupAggScan(CustomScanState *node, List *ancestors, gs.groupsTotal = state->groupsTotal; gs.groupsRead = state->groupsRead; gs.groupsRemoved = state->groupsSkipped; + gs.vectorsSkipped = state->vectorsSkipped; PgColumnarExplainGroupStats(&gs, es); } } diff --git a/test/native_vecskip.sh b/test/native_vecskip.sh index 985e1348..48af9bb2 100644 --- a/test/native_vecskip.sh +++ b/test/native_vecskip.sh @@ -72,6 +72,42 @@ check "no whole group removed" "$(counter 'Columnar Chunk Groups Removed by Filt check "non-selective removes no vectors" \ "$(counter 'Columnar Vectors Skipped' 'SELECT id FROM n WHERE id > 0')" "0" +# ---- the vectorized aggregate must report the counter too -------------------- +# +# "Columnar Vectors Skipped" is emitted by the scalar node only. The two +# vectorized aggregate nodes print Chunk Groups Total/Read/Removed, Usable Skip +# Predicates and Vector Predicates -- but not this one -- so a plan cannot say +# whether per-vector skipping happened on the aggregate path. That is the path +# where it matters most: #512 is precisely about the fold and the skip vector +# disagreeing, and the number that would show it is the one not printed. +# +# Same table, same predicate as the scalar checks above, so the two arms differ +# only in the node. +# pgcolumnar.enable_ungrouped_vector_agg defaults to OFF, so the aggregate node +# has to be asked for. Without the SET this falls back to the scalar scan -- which +# is what the first version of this check did, and the premise below caught it +# passing for that reason. +AGGQ="SELECT count(*), sum(v) FROM n WHERE id BETWEEN 100 AND 200" +explain_agg() { + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -At \ + -c "SET pgcolumnar.enable_ungrouped_vector_agg = on; + EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF) $1" 2>/dev/null +} + +# Premise, and it has teeth: if the planner declines the vectorized aggregate, +# this query falls back to the scalar scan -- which DOES print the line, so the +# check below would pass while testing nothing at all. +check "premise: the aggregate arm really is a vectorized aggregate" \ + "$(explain_agg "$AGGQ" | grep -q 'Columnar Vectorized Aggregates' && echo yes || echo no)" \ + "yes" +check "premise: and it is not the scalar scan" \ + "$(explain_agg "$AGGQ" | grep -q 'Columnar Projected Columns' && echo yes || echo no)" "no" + +check "the vectorized aggregate reports Columnar Vectors Skipped" \ + "$(explain_agg "$AGGQ" | grep -q 'Columnar Vectors Skipped' && echo yes || echo no)" \ + "yes" + # Boundary and cross-vector ranges still return exactly the heap rows. check "cross-vector range parity" \ "$(pgc_set_hash 'SELECT id, v FROM n WHERE id BETWEEN 2000 AND 5000')" \