diff --git a/src/columnar.h b/src/columnar.h index 0f07032..32f5178 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -910,6 +910,34 @@ extern Node *PgColumnarCreateGroupAggScanState(CustomScan *cscan); * Shared by the base custom scan and the vectorized aggregate (spec 9). Clauses * that are not simple "column op const" comparisons are ignored. */ +/* + * The chunk-group statistics every scan node reports, and the one place they are + * printed (#495). + * + * Three nodes emit these lines -- the scalar custom scan and both vectorized + * aggregates -- and each used to print its own copy. #484 had to add + * "Usable Skip Predicates" in three places for exactly that reason, and said why + * it could not do fewer: fixing one would leave a line of plan text meaning two + * different things depending on which node ran. That invariant was held by + * discipline; this holds it by structure, and pushdown_report.sh fails if a + * fourth emitter appears. + * + * Two functions rather than one because the nodes interleave: the aggregate node + * prints "Columnar Batch Fold" between the filter count and the group counters, + * so combining them would reorder its plan output. + */ +typedef struct PgColumnarGroupStats +{ + int64 usableSkipPredicates; + uint64 groupsTotal; + uint64 groupsRead; + uint64 groupsRemoved; +} PgColumnarGroupStats; + +extern void PgColumnarExplainPushedDown(int64 nfilters, ExplainState *es); +extern void PgColumnarExplainGroupStats(const PgColumnarGroupStats *stats, + ExplainState *es); + extern ScanKey PgColumnarBuildScanKeys(List *qual, Index scanrelid, TupleDesc tupdesc, int *nkeys); diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index c10edca..c71deab 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -2075,6 +2075,50 @@ PgColumnarReScanCustomScan(CustomScanState *node) ExecScanReScan(&node->ss); } +/* + * PgColumnarExplainPushedDown + * How many quals the scan was GIVEN as scan keys. + * + * Describes the PLAN, not the run, which is why it prints whether or not the + * scan read anything: someone who sets pgcolumnar.enable_qual_pushdown to test a + * theory and checks EXPLAIN to confirm it took effect was once told it had not + * (#191). Every other "Columnar ..." line describes the run. + */ +void +PgColumnarExplainPushedDown(int64 nfilters, ExplainState *es) +{ + ExplainPropertyInteger("Columnar Pushed-Down Filters", NULL, nfilters, es); +} + +/* + * PgColumnarExplainGroupStats + * What the scan actually did to chunk groups. + * + * "Usable Skip Predicates" is separate from the filter count above and both are + * needed (#479). That one says whether pushdown took effect; this one says + * whether the predicates it pushed can prune, because + * pgcolumnar_make_predicates drops any it cannot evaluate against the stored + * min/max and a dropped key skips nothing. A single number cannot say both, and + * saying only the first is how #477 stayed invisible for a year -- + * "Pushed-Down Filters: 1" beside "Chunk Groups Removed by Filter: 0" reads as an + * unselective predicate and meant an unusable one. + * + * No enable_qual_pushdown ternary: with the setting off the reader builds no + * predicates, so this is already zero. It describes the run. + */ +void +PgColumnarExplainGroupStats(const PgColumnarGroupStats *stats, ExplainState *es) +{ + ExplainPropertyInteger("Columnar Usable Skip Predicates", NULL, + stats->usableSkipPredicates, es); + ExplainPropertyInteger("Columnar Chunk Groups Total", NULL, + (int64) stats->groupsTotal, es); + ExplainPropertyInteger("Columnar Chunk Groups Read", NULL, + (int64) stats->groupsRead, es); + ExplainPropertyInteger("Columnar Chunk Groups Removed by Filter", NULL, + (int64) stats->groupsRemoved, es); +} + /* ------------------------------------------------------------------------- * parallel scan (gap 23): a shared atomic hands out stripe indices so several * workers scanning the same relation each claim distinct stripes. The custom @@ -2167,15 +2211,15 @@ PgColumnarExplainCustomScan(CustomScanState *node, List *ancestors, * plan -- Projected Columns, Chunk Groups Total and the counters below it -- * so this one line meant something different from all of its neighbours. */ - ExplainPropertyInteger("Columnar Pushed-Down Filters", NULL, - pgcolumnar_enable_qual_pushdown ? cstate->nScanKeys : 0, - es); + PgColumnarExplainPushedDown(pgcolumnar_enable_qual_pushdown + ? cstate->nScanKeys : 0, es); if (cstate->readState != NULL) { uint64 groupsRead = 0; uint64 groupsSkipped = 0; uint64 groupsTotal = 0; + PgColumnarGroupStats gs; PgColumnarReadStats(cstate->readState, &groupsRead, &groupsSkipped, &groupsTotal); @@ -2197,16 +2241,11 @@ PgColumnarExplainCustomScan(CustomScanState *node, List *ancestors, * No enable_qual_pushdown ternary here: with the setting off the reader * builds no predicates, so this is already 0. It describes the run. */ - ExplainPropertyInteger("Columnar Usable Skip Predicates", NULL, - PgColumnarReadUsablePredicates(cstate->readState), - es); - - ExplainPropertyInteger("Columnar Chunk Groups Total", NULL, - (int64) groupsTotal, es); - ExplainPropertyInteger("Columnar Chunk Groups Read", NULL, - (int64) groupsRead, es); - ExplainPropertyInteger("Columnar Chunk Groups Removed by Filter", NULL, - (int64) groupsSkipped, es); + gs.usableSkipPredicates = PgColumnarReadUsablePredicates(cstate->readState); + gs.groupsTotal = groupsTotal; + gs.groupsRead = groupsRead; + gs.groupsRemoved = groupsSkipped; + PgColumnarExplainGroupStats(&gs, es); ExplainPropertyInteger("Columnar Vectors Skipped", NULL, (int64) PgColumnarVectorsSkipped(cstate->readState), es); diff --git a/src/columnar_vector.c b/src/columnar_vector.c index c54ee97..bde6bdd 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -3534,8 +3534,7 @@ PgColumnarExplainAggScan(CustomScanState *node, List *ancestors, ExplainState *e ExplainPropertyInteger("Columnar Vectorized Aggregates", NULL, state->naggs, es); - ExplainPropertyInteger("Columnar Pushed-Down Filters", NULL, - state->npreds, es); + PgColumnarExplainPushedDown(state->npreds, es); if (state->scanFold) ExplainPropertyText("Columnar Batch Fold", state->batchEligible ? "yes" : "no", es); @@ -3543,23 +3542,18 @@ PgColumnarExplainAggScan(CustomScanState *node, List *ancestors, ExplainState *e if (state->haveStats) { /* - * See PgColumnarExplainCustomScan: npreds above counts the quals that - * became scan keys, this counts the ones the reader can exclude a chunk - * group with, and only the pair distinguishes an unselective predicate - * from an unusable one (#479). This node fills npreds from - * PgColumnarCountConvertibleQuals, which is the same built-key count the - * scalar node reports, so it has the same gap and needs the same second - * number -- otherwise one line of plan text would mean two different - * things depending on which node ran. + * npreds is this node's built-key count from + * PgColumnarCountConvertibleQuals, the same quantity the scalar node + * reports, so it has the same gap and needs the same second number. + * PgColumnarExplainGroupStats is why that is now automatic. */ - ExplainPropertyInteger("Columnar Usable Skip Predicates", NULL, - state->usablePreds, es); - ExplainPropertyInteger("Columnar Chunk Groups Total", NULL, - (int64) state->groupsTotal, es); - ExplainPropertyInteger("Columnar Chunk Groups Read", NULL, - (int64) state->groupsRead, es); - ExplainPropertyInteger("Columnar Chunk Groups Removed by Filter", NULL, - (int64) state->groupsSkipped, es); + PgColumnarGroupStats gs; + + gs.usableSkipPredicates = state->usablePreds; + gs.groupsTotal = state->groupsTotal; + gs.groupsRead = state->groupsRead; + gs.groupsRemoved = state->groupsSkipped; + PgColumnarExplainGroupStats(&gs, es); } } @@ -4226,20 +4220,17 @@ PgColumnarExplainGroupAggScan(CustomScanState *node, List *ancestors, state->nkeys, es); ExplainPropertyInteger("Columnar Vectorized Aggregates", NULL, state->naggs, es); - ExplainPropertyInteger("Columnar Pushed-Down Filters", NULL, - state->npreds, es); + PgColumnarExplainPushedDown(state->npreds, es); if (state->haveStats) { - /* of those, the ones that can exclude a chunk group (#479) */ - ExplainPropertyInteger("Columnar Usable Skip Predicates", NULL, - state->usablePreds, es); - ExplainPropertyInteger("Columnar Chunk Groups Total", NULL, - (int64) state->groupsTotal, es); - ExplainPropertyInteger("Columnar Chunk Groups Read", NULL, - (int64) state->groupsRead, es); - ExplainPropertyInteger("Columnar Chunk Groups Removed by Filter", NULL, - (int64) state->groupsSkipped, es); + PgColumnarGroupStats gs; + + gs.usableSkipPredicates = state->usablePreds; + gs.groupsTotal = state->groupsTotal; + gs.groupsRead = state->groupsRead; + gs.groupsRemoved = state->groupsSkipped; + PgColumnarExplainGroupStats(&gs, es); } } diff --git a/test/pushdown_report.sh b/test/pushdown_report.sh index 1b5be4a..e4170a4 100755 --- a/test/pushdown_report.sh +++ b/test/pushdown_report.sh @@ -336,4 +336,32 @@ check "grouped: the usable arm reports one of each, agreeing" \ check "grouped: and the unusable arm builds no skip predicate" \ "$(field "$g_unusable" 'Columnar Usable Skip Predicates')" "0" +# ---- one emitter per shared statistic (#495) -------------------------------- +# +# Five of these lines are printed by every scan node -- the scalar custom scan and +# the two vectorized aggregates -- and each used to print its own copy. #484 had +# to add "Usable Skip Predicates" in three places for that reason, and its own +# rationale says why it could not do fewer: "fixing one would leave a line of plan +# text meaning two different things depending on which node ran." +# +# Unifying three call sites into one is only durable if a FOURTH is detectable. +# Otherwise the next node grows its own copy, the suite stays green, and the three +# reappear exactly as they arose. So this asserts the count at the source, which +# is the only place a new caller is visible before it has drifted. +# +# A grep over source rather than a behavioural check on purpose: the failure being +# guarded is "someone wrote a second emitter", which is a property of the code and +# not of any plan. The behavioural half -- that the nodes agree -- is the checks +# above. +PGC_SRC_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/src" +for _line in "Columnar Pushed-Down Filters" \ + "Columnar Usable Skip Predicates" \ + "Columnar Chunk Groups Total" \ + "Columnar Chunk Groups Read" \ + "Columnar Chunk Groups Removed by Filter"; do + check "exactly one emitter for \"$_line\" (#495)" \ + "$(grep -rho "ExplainPropertyInteger(\"$_line\"" "$PGC_SRC_DIR" | wc -l | tr -d ' ')" \ + "1" +done + pgc_summary