From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> |
Cc: | Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Strange result with LATERAL query |
Date: | 2016-08-24 15:08:28 |
Message-ID: | 18374.1472051308@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> Something is wrong with the way chgParam is being handled in Agg nodes.
> The code in ExecReScanAgg seems to assume that if the lefttree doesn't
> have any parameter changes then it suffices to re-project the data from
> the existing hashtable; but of course this is nonsense if the parameter
> is in an input to an aggregate function.
It looks like it's sufficient to do this:
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 1ec2515..f468fad 100644
*** a/src/backend/executor/nodeAgg.c
--- b/src/backend/executor/nodeAgg.c
*************** ExecReScanAgg(AggState *node)
*** 3425,3435 ****
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 (outerPlan->chgParam == NULL)
{
ResetTupleHashIterator(node->hashtable, &node->hashiter);
return;
--- 3425,3436 ----
return;
/*
! * If we do have the hash table and there are no relevant parameter
! * changes, then we can just rescan the existing hash table; no need
! * to build it again.
*/
! if (node->ss.ps.chgParam == NULL &&
! outerPlan->chgParam == NULL)
{
ResetTupleHashIterator(node->hashtable, &node->hashiter);
return;
I'm not sure if it's worth trying to distinguish whether the Param is
inside any aggregate calls or not. The existing code gets the right
answer for
select array(select x+sum(y) from generate_series(1,3) y group by y)
from generate_series(1,3) x;
and we'd be losing some efficiency for cases like that if we fix
it as above. But is it worth the trouble?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2016-08-24 15:26:04 | Re: Strange result with LATERAL query |
Previous Message | Emre Hasegeli | 2016-08-24 14:47:19 | Re: [PATCH] Alter or rename enum value |