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

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 82ed5b3e1cbfeaeb941c0c764f728c21c6858429..2b118359b53654eed879e1a5b6acd78e2ff39c76 100644 (file)
--- 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;