database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-28 11:44:57 +03:00
Code Activity

Fix improper repetition of previous results from a hashed aggregate.

Browse Source 
ExecReScanAgg's check for whether it could re-use a previously calculated
hashtable neglected the possibility that the Agg node might reference
PARAM_EXEC Params that are not referenced by its input plan node.  That's
okay if the Params are in upper tlist or qual expressions; but if one
appears in aggregate input expressions, then the hashtable contents need
to be recomputed when the Param's value changes.

To avoid unnecessary performance degradation in the case of a Param that
isn't within an aggregate input, add logic to the planner to determine
which Params are within aggregate inputs.  This requires a new field in
struct Agg, but fortunately we never write plans to disk, so this isn't
an initdb-forcing change.

Per report from Jeevan Chalke.  This has been broken since forever,
so back-patch to all supported branches.

Andrew Gierth, with minor adjustments by me

Report: <CAM2+6=VY8ykfLT5Q8vb9B6EbeBk-NGuLbT6seaQ+Fq4zXvrDcA@mail.gmail.com>
This commit is contained in:
Tom Lane
2016-08-24 14:37:51 -04:00
parent 07d47231fb
commit 08a823e53b
8 changed files with 158 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/executor/nodeAgg.c
View File

@@ -2049,13 +2049,14 @@ void
ExecReScanAgg(AggState *node)
{
ExprContext *econtext = node->ss.ps.ps_ExprContext;
Agg *aggnode = (Agg *) node->ss.ps.plan;
int aggno;
node->agg_done = false;
node->ss.ps.ps_TupFromTlist = false;
if (((Agg *) node->ss.ps.plan)->aggstrategy == AGG_HASHED)
if (aggnode->aggstrategy == AGG_HASHED)
{
/*
* In the hashed case, if we haven't yet built the hash table then we
@@ -2067,11 +2068,13 @@ ExecReScanAgg(AggState *node)
return;
/*
* 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.
* If we do have the hash table, and the subplan does not have any
* parameter changes, and none of our own parameter changes affect
* input expressions of the aggregated functions, then we can just
* rescan the existing hash table; no need to build it again.
*/
if (node->ss.ps.lefttree->chgParam == NULL)
if (node->ss.ps.lefttree->chgParam == NULL &&
!bms_overlap(node->ss.ps.chgParam, aggnode->aggParams))
{
ResetTupleHashIterator(node->hashtable, &node->hashiter);
return;
@@ -2110,7 +2113,7 @@ ExecReScanAgg(AggState *node)
*/
MemoryContextResetAndDeleteChildren(node->aggcontext);
if (((Agg *) node->ss.ps.plan)->aggstrategy == AGG_HASHED)
if (aggnode->aggstrategy == AGG_HASHED)
{
/* Rebuild an empty hash table */
build_hash_table(node);