Re: Strange result with LATERAL query - Mailing list pgsql-hackers

>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

 Tom> I'm not sure if it's worth trying to distinguish whether the Param
 Tom> is inside any aggregate calls or not.

How about:

--
Andrew (irc:RhodiumToad)

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 1ec2515..4a9fefb 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3426,10 +3426,11 @@ ExecReScanAgg(AggState *node)

         /*
          * If we do have the hash table and the subplan does not have any
-         * parameter changes, then we can just rescan the existing hash table;
-         * no need to build it again.
+         * parameter changes, we might still need to rebuild the hash if any of
+         * the parameter changes affect the input to aggregate functions.
          */
-        if (outerPlan->chgParam == NULL)
+        if (outerPlan->chgParam == NULL
+            && !bms_overlap(node->ss.ps.chgParam, aggnode->aggParam))
         {
             ResetTupleHashIterator(node->hashtable, &node->hashiter);
             return;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index c7a0644..7b5eb86 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -871,6 +871,7 @@ _copyAgg(const Agg *from)
     COPY_SCALAR_FIELD(aggstrategy);
     COPY_SCALAR_FIELD(aggsplit);
     COPY_SCALAR_FIELD(numCols);
+    COPY_BITMAPSET_FIELD(aggParam);
     if (from->numCols > 0)
     {
         COPY_POINTER_FIELD(grpColIdx, from->numCols * sizeof(AttrNumber));
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 1fab807..5e48edd 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -706,6 +706,7 @@ _outAgg(StringInfo str, const Agg *node)
     WRITE_ENUM_FIELD(aggstrategy, AggStrategy);
     WRITE_ENUM_FIELD(aggsplit, AggSplit);
     WRITE_INT_FIELD(numCols);
+    WRITE_BITMAPSET_FIELD(aggParam);

     appendStringInfoString(str, " :grpColIdx");
     for (i = 0; i < node->numCols; i++)
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index c83063e..67dcf17 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -2004,6 +2004,7 @@ _readAgg(void)
     READ_ENUM_FIELD(aggstrategy, AggStrategy);
     READ_ENUM_FIELD(aggsplit, AggSplit);
     READ_INT_FIELD(numCols);
+    READ_BITMAPSET_FIELD(aggParam);
     READ_ATTRNUMBER_ARRAY(grpColIdx, local_node->numCols);
     READ_OID_ARRAY(grpOperators, local_node->numCols);
     READ_LONG_FIELD(numGroups);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index a46cc10..2e56484 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -82,6 +82,7 @@ static Bitmapset *finalize_plan(PlannerInfo *root,
               Bitmapset *valid_params,
               Bitmapset *scan_params);
 static bool finalize_primnode(Node *node, finalize_primnode_context *context);
+static bool finalize_agg_primnode(Node *node, finalize_primnode_context *context);


 /*
@@ -2659,8 +2660,30 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
                               &context);
             break;

-        case T_Hash:
         case T_Agg:
+            {
+                Agg           *agg = (Agg *) plan;
+                finalize_primnode_context aggcontext;
+
+                /*
+                 * We need to know which params are referenced in inputs to
+                 * aggregate calls, so that we know whether we need to rebuild
+                 * the hashtable in the AGG_HASHED case. Rescan the tlist and
+                 * qual for this purpose.
+                 */
+
+                aggcontext = context;
+                aggcontext.paramids = NULL;
+
+                finalize_agg_primnode((Node *) agg->plan.targetlist, &aggcontext);
+                finalize_agg_primnode((Node *) agg->plan.qual, &aggcontext);
+
+                /* remember results for execution */
+                agg->aggParam = aggcontext.paramids;
+            }
+            break;
+
+        case T_Hash:
         case T_Material:
         case T_Sort:
         case T_Unique:
@@ -2812,6 +2835,24 @@ finalize_primnode(Node *node, finalize_primnode_context *context)
 }

 /*
+ * finalize_agg_primnode: add IDs of all PARAM_EXEC params appearing inside
+ * Aggref nodes in the given expression tree to the result set.
+ */
+static bool
+finalize_agg_primnode(Node *node, finalize_primnode_context *context)
+{
+    if (node == NULL)
+        return false;
+    if (IsA(node, Aggref))
+    {
+        finalize_primnode(node, context);
+        return false;            /* no more to do here */
+    }
+    return expression_tree_walker(node, finalize_agg_primnode,
+                                  (void *) context);
+}
+
+/*
  * SS_make_initplan_output_param - make a Param for an initPlan's output
  *
  * The plan is expected to return a scalar value of the given type/collation.
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 369179f..3d5e4df 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -712,6 +712,7 @@ typedef struct Agg
     AggStrategy aggstrategy;    /* basic strategy, see nodes.h */
     AggSplit    aggsplit;        /* agg-splitting mode, see nodes.h */
     int            numCols;        /* number of grouping columns */
+    Bitmapset  *aggParam;        /* params used in Aggref inputs */
     AttrNumber *grpColIdx;        /* their indexes in the target list */
     Oid           *grpOperators;    /* equality operators to compare with */
     long        numGroups;        /* estimated number of groups in input */