>>>>> "Andrew" == Andrew Gierth <and...@tao11.riddles.org.uk> writes:
>>>>> "Tom" == Tom Lane <t...@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 */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
