Skip to content

Commit 0a2bc5d

Browse files
committed
Move per-agg and per-trans duplicate finding to the planner.
This has the advantage that the cost estimates for aggregates can count the number of calls to transition and final functions correctly. Bump catalog version, because views can contain Aggrefs. Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/b2e3536b-1dbc-8303-c97e-89cb0b4a9a48%40iki.fi
1 parent e522024 commit 0a2bc5d

File tree

29 files changed

+954
-746
lines changed

29 files changed

+954
-746
lines changed

contrib/postgres_fdw/postgres_fdw.c

+2-10
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include "optimizer/pathnode.h"
3333
#include "optimizer/paths.h"
3434
#include "optimizer/planmain.h"
35+
#include "optimizer/prep.h"
3536
#include "optimizer/restrictinfo.h"
3637
#include "optimizer/tlist.h"
3738
#include "parser/parsetree.h"
@@ -2944,16 +2945,7 @@ estimate_path_cost_size(PlannerInfo *root,
29442945
MemSet(&aggcosts, 0, sizeof(AggClauseCosts));
29452946
if (root->parse->hasAggs)
29462947
{
2947-
get_agg_clause_costs(root, (Node *) fpinfo->grouped_tlist,
2948-
AGGSPLIT_SIMPLE, &aggcosts);
2949-
2950-
/*
2951-
* The cost of aggregates in the HAVING qual will be the same
2952-
* for each child as it is for the parent, so there's no need
2953-
* to use a translated version of havingQual.
2954-
*/
2955-
get_agg_clause_costs(root, (Node *) root->parse->havingQual,
2956-
AGGSPLIT_SIMPLE, &aggcosts);
2948+
get_agg_clause_costs(root, AGGSPLIT_SIMPLE, &aggcosts);
29572949
}
29582950

29592951
/* Get number of grouping columns and possible number of groups */

src/backend/executor/execExpr.c

+3-7
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ static void ExecBuildAggTransCall(ExprState *state, AggState *aggstate,
9999
* the same as the per-query context of the associated ExprContext.
100100
*
101101
* Any Aggref, WindowFunc, or SubPlan nodes found in the tree are added to
102-
* the lists of such nodes held by the parent PlanState (or more accurately,
103-
* the AggrefExprState etc. nodes created for them are added).
102+
* the lists of such nodes held by the parent PlanState.
104103
*
105104
* Note: there is no ExecEndExpr function; we assume that any resource
106105
* cleanup needed will be handled by just releasing the memory context
@@ -779,18 +778,15 @@ ExecInitExprRec(Expr *node, ExprState *state,
779778
case T_Aggref:
780779
{
781780
Aggref *aggref = (Aggref *) node;
782-
AggrefExprState *astate = makeNode(AggrefExprState);
783781

784782
scratch.opcode = EEOP_AGGREF;
785-
scratch.d.aggref.astate = astate;
786-
astate->aggref = aggref;
783+
scratch.d.aggref.aggno = aggref->aggno;
787784

788785
if (state->parent && IsA(state->parent, AggState))
789786
{
790787
AggState *aggstate = (AggState *) state->parent;
791788

792-
aggstate->aggs = lappend(aggstate->aggs, astate);
793-
aggstate->numaggs++;
789+
aggstate->aggs = lappend(aggstate->aggs, aggref);
794790
}
795791
else
796792
{

src/backend/executor/execExprInterp.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -1494,12 +1494,12 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
14941494
* Returns a Datum whose value is the precomputed aggregate value
14951495
* found in the given expression context.
14961496
*/
1497-
AggrefExprState *aggref = op->d.aggref.astate;
1497+
int aggno = op->d.aggref.aggno;
14981498

14991499
Assert(econtext->ecxt_aggvalues != NULL);
15001500

1501-
*op->resvalue = econtext->ecxt_aggvalues[aggref->aggno];
1502-
*op->resnull = econtext->ecxt_aggnulls[aggref->aggno];
1501+
*op->resvalue = econtext->ecxt_aggvalues[aggno];
1502+
*op->resnull = econtext->ecxt_aggnulls[aggno];
15031503

15041504
EEO_NEXT();
15051505
}

0 commit comments

Comments
 (0)