refactor: one place prints the chunk-group statistics (#495)

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);
 

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);
 

src/columnar_vector.c

@@ -3534,32 +3534,26 @@ 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);
 
 	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);
 	}
 }
 

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