aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Rowley <drowley@postgresql.org>2021-07-26 14:55:31 +1200
committerDavid Rowley <drowley@postgresql.org>2021-07-26 14:55:31 +1200
commit2b58f894e56a1944e824e19c92337d6bf24d9c41 (patch)
treefc33bdaf763779085565f9551feae73ce1133c1b
parent4ef64c425dbcda151c9f163aadff982343808e09 (diff)
downloadpostgresql-2b58f894e56a1944e824e19c92337d6bf24d9c41.tar.gz
postgresql-2b58f894e56a1944e824e19c92337d6bf24d9c41.zip
Fix incorrect comment for get_agg_clause_costs
Adjust the header comment in get_agg_clause_costs so that it matches what the function currently does. No recursive searching has been done ever since 0a2bc5d61. It also does not determine the aggtranstype like the comment claimed. That's all done in preprocess_aggref(). preprocess_aggref also now determines the numOrderedAggs, so remove the mention that get_agg_clause_costs also calculates "counts". Normally, since this is just an adjustment of a comment it might not be worth back-patching, but since this code is new to PG14 and that version is still in beta, then it seems worth having the comments match. Discussion: https://postgr.es/m/CAApHDvrrGrTJFPELrjx0CnDtz9B7Jy2XYW3Z2BKifAWLSaJYwQ@mail.gmail.com Backpatch-though: 14
-rw-r--r--src/backend/optimizer/prep/prepagg.c28
1 files changed, 12 insertions, 16 deletions
diff --git a/src/backend/optimizer/prep/prepagg.c b/src/backend/optimizer/prep/prepagg.c
index 89046f9afbb..e1c525760cd 100644
--- a/src/backend/optimizer/prep/prepagg.c
+++ b/src/backend/optimizer/prep/prepagg.c
@@ -515,28 +515,24 @@ GetAggInitVal(Datum textInitVal, Oid transtype)
/*
* get_agg_clause_costs
- * Recursively find the Aggref nodes in an expression tree, and
- * accumulate cost information about them.
+ * Process the PlannerInfo's 'aggtransinfos' and 'agginfos' lists
+ * accumulating the cost information about them.
*
* 'aggsplit' tells us the expected partial-aggregation mode, which affects
* the cost estimates.
*
- * NOTE that the counts/costs are ADDED to those already in *costs ... so
- * the caller is responsible for zeroing the struct initially.
+ * NOTE that the costs are ADDED to those already in *costs ... so the caller
+ * is responsible for zeroing the struct initially.
*
- * We count the nodes, estimate their execution costs, and estimate the total
- * space needed for their transition state values if all are evaluated in
- * parallel (as would be done in a HashAgg plan). Also, we check whether
- * partial aggregation is feasible. See AggClauseCosts for the exact set
- * of statistics collected.
+ * For each AggTransInfo, we add the cost of an aggregate transition using
+ * either the transfn or combinefn depending on the 'aggsplit' value. We also
+ * account for the costs of any aggfilters and any serializations and
+ * deserializations of the transition state and also estimate the total space
+ * needed for the transition states as if each aggregate's state was stored in
+ * memory concurrently (as would be done in a HashAgg plan).
*
- * In addition, we mark Aggref nodes with the correct aggtranstype, so
- * that that doesn't need to be done repeatedly. (That makes this function's
- * name a bit of a misnomer.)
- *
- * This does not descend into subqueries, and so should be used only after
- * reduction of sublinks to subplans, or in contexts where it's known there
- * are no subqueries. There mustn't be outer-aggregate references either.
+ * For each AggInfo in the 'agginfos' list we add the cost of running the
+ * final function and the direct args, if any.
*/
void
get_agg_clause_costs(PlannerInfo *root, AggSplit aggsplit, AggClauseCosts *costs)