Re: [BUGS] Combination of ordered-set aggregate function terminates JDBC connection on PostgreSQL 9.6.5 - Mailing list pgsql-bugs

I wrote:
> BTW ... I was quite surprised to notice that the aggdirectargs
> are treated as a property that has to be matched in order to
> combine transition states.  They aren't used during the transition
> phase, so this seems like a pointless constraint.  We could move
> the aggdirectargs ExprState list to AggStatePerAggData and treat
> the aggdirectargs as part of the finalfn-related data, instead.

Here's a pretty-lightly-tested patch for that.

            regards, tom lane

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 807822b..8f5f7f1 100644
*** a/src/backend/executor/nodeAgg.c
--- b/src/backend/executor/nodeAgg.c
*************** typedef struct AggStatePerTransData
*** 260,272 ****
      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
       * are not counted, though.
--- 260,265 ----
*************** typedef struct AggStatePerTransData
*** 300,308 ****
      /* Oid of state value's datatype */
      Oid            aggtranstype;

!     /* ExprStates of the FILTER and argument expressions. */
!     ExprState  *aggfilter;        /* state of FILTER expression, if any */
!     List       *aggdirectargs;    /* states of direct-argument expressions */

      /*
       * fmgr lookup data for transition function or combine function.  Note in
--- 293,300 ----
      /* Oid of state value's datatype */
      Oid            aggtranstype;

!     /* ExprState for the FILTER expression, if any. */
!     ExprState  *aggfilter;

      /*
       * fmgr lookup data for transition function or combine function.  Note in
*************** typedef struct AggStatePerAggData
*** 442,447 ****
--- 434,442 ----
       */
      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.
*************** finalize_aggregate(AggState *aggstate,
*** 1526,1532 ****
       * for the transition state value.
       */
      i = 1;
!     foreach(lc, pertrans->aggdirectargs)
      {
          ExprState  *expr = (ExprState *) lfirst(lc);

--- 1521,1527 ----
       * for the transition state value.
       */
      i = 1;
!     foreach(lc, peragg->aggdirectargs)
      {
          ExprState  *expr = (ExprState *) lfirst(lc);

*************** ExecInitAgg(Agg *node, EState *estate, i
*** 3293,3298 ****
--- 3288,3297 ----
          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.
*************** build_pertrans_for_aggref(AggStatePerTra
*** 3605,3615 ****

      }

!     /* Initialize the input and FILTER expressions */
      pertrans->aggfilter = ExecInitExpr(aggref->aggfilter,
                                         (PlanState *) aggstate);
-     pertrans->aggdirectargs = ExecInitExprList(aggref->aggdirectargs,
-                                                (PlanState *) aggstate);

      /*
       * If we're doing either DISTINCT or ORDER BY for a plain agg, then we
--- 3604,3612 ----

      }

!     /* Initialize the FILTER expression */
      pertrans->aggfilter = ExecInitExpr(aggref->aggfilter,
                                         (PlanState *) aggstate);

      /*
       * If we're doing either DISTINCT or ORDER BY for a plain agg, then we
*************** find_compatible_peragg(Aggref *newagg, A
*** 3793,3799 ****
              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) ||
--- 3790,3795 ----
*************** find_compatible_peragg(Aggref *newagg, A
*** 3803,3809 ****
          /* if it's the same aggregate function then report exact match */
          if (newagg->aggfnoid == existingRef->aggfnoid &&
              newagg->aggtype == existingRef->aggtype &&
!             newagg->aggcollid == existingRef->aggcollid)
          {
              list_free(*same_input_transnos);
              *same_input_transnos = NIL;
--- 3799,3806 ----
          /* if it's the same aggregate function then report exact match */
          if (newagg->aggfnoid == existingRef->aggfnoid &&
              newagg->aggtype == existingRef->aggtype &&
!             newagg->aggcollid == existingRef->aggcollid &&
!             equal(newagg->aggdirectargs, existingRef->aggdirectargs))
          {
              list_free(*same_input_transnos);
              *same_input_transnos = NIL;

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs