From cf5ba7c30c0428f5ff49197ec1e0f052035300d6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 16 Oct 2017 16:02:51 -0400 Subject: [PATCH] Treat aggregate direct arguments as per-agg data not per-trans data. There is no reason to insist that direct arguments must match before we can merge transition states of two aggregate calls. They're only used during the finalfn call, so we can treat them as like the finalfn itself. This allows, eg, merging of select percentile_cont(0.25) within group (order by a), percentile_disc(0.5) within group (order by a) from ... This didn't matter (and could not have been tested) before we allowed state merging of OSAs otherwise. Discussion: https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com --- src/backend/executor/nodeAgg.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 82ed5b3e1c..2b118359b5 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -259,13 +259,6 @@ typedef struct AggStatePerTransData */ bool aggshared; - /* - * Nominal number of arguments for aggregate function. For plain aggs, - * this excludes any ORDER BY expressions. For ordered-set aggs, this - * counts both the direct and aggregated (ORDER BY) arguments. - */ - int numArguments; - /* * Number of aggregated input columns. This includes ORDER BY expressions * in both the plain-agg and ordered-set cases. Ordered-set direct args @@ -302,9 +295,6 @@ typedef struct AggStatePerTransData /* Oid of state value's datatype */ Oid aggtranstype; - /* ExprStates for any direct-argument expressions */ - List *aggdirectargs; - /* * fmgr lookup data for transition function or combine function. Note in * particular that the fn_strict flag is kept here. @@ -444,6 +434,9 @@ typedef struct AggStatePerAggData */ int numFinalArgs; + /* ExprStates for any direct-argument expressions */ + List *aggdirectargs; + /* * We need the len and byval info for the agg's result data type in order * to know how to copy/delete values. @@ -1544,7 +1537,7 @@ finalize_aggregate(AggState *aggstate, * for the transition state value. */ i = 1; - foreach(lc, pertrans->aggdirectargs) + foreach(lc, peragg->aggdirectargs) { ExprState *expr = (ExprState *) lfirst(lc); @@ -3313,6 +3306,10 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) else peragg->numFinalArgs = numDirectArgs + 1; + /* Initialize any direct-argument expressions */ + peragg->aggdirectargs = ExecInitExprList(aggref->aggdirectargs, + (PlanState *) aggstate); + /* * build expression trees using actual argument & result types for the * finalfn, if it exists and is required. @@ -3657,10 +3654,6 @@ build_pertrans_for_aggref(AggStatePerTrans pertrans, } - /* Initialize any direct-argument expressions */ - pertrans->aggdirectargs = ExecInitExprList(aggref->aggdirectargs, - (PlanState *) aggstate); - /* * If we're doing either DISTINCT or ORDER BY for a plain agg, then we * have a list of SortGroupClause nodes; fish out the data in them and @@ -3847,7 +3840,6 @@ find_compatible_peragg(Aggref *newagg, AggState *aggstate, newagg->aggstar != existingRef->aggstar || newagg->aggvariadic != existingRef->aggvariadic || newagg->aggkind != existingRef->aggkind || - !equal(newagg->aggdirectargs, existingRef->aggdirectargs) || !equal(newagg->args, existingRef->args) || !equal(newagg->aggorder, existingRef->aggorder) || !equal(newagg->aggdistinct, existingRef->aggdistinct) || @@ -3857,7 +3849,8 @@ find_compatible_peragg(Aggref *newagg, AggState *aggstate, /* if it's the same aggregate function then report exact match */ if (newagg->aggfnoid == existingRef->aggfnoid && newagg->aggtype == existingRef->aggtype && - newagg->aggcollid == existingRef->aggcollid) + newagg->aggcollid == existingRef->aggcollid && + equal(newagg->aggdirectargs, existingRef->aggdirectargs)) { list_free(*same_input_transnos); *same_input_transnos = NIL; -- 2.39.5