Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitaaad96e

Browse files
committed
Fix improper repetition of previous results from a hashed aggregate.
ExecReScanAgg's check for whether it could re-use a previously calculatedhashtable neglected the possibility that the Agg node might referencePARAM_EXEC Params that are not referenced by its input plan node. That'sokay if the Params are in upper tlist or qual expressions; but if oneappears in aggregate input expressions, then the hashtable contents needto be recomputed when the Param's value changes.To avoid unnecessary performance degradation in the case of a Param thatisn't within an aggregate input, add logic to the planner to determinewhich Params are within aggregate inputs. This requires a new field instruct Agg, but fortunately we never write plans to disk, so this isn'tan 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 meReport: <CAM2+6=VY8ykfLT5Q8vb9B6EbeBk-NGuLbT6seaQ+Fq4zXvrDcA@mail.gmail.com>
1 parente8aed97 commitaaad96e

File tree

8 files changed

+155
-7
lines changed

8 files changed

+155
-7
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1914,13 +1914,14 @@ void
19141914
ExecReScanAgg(AggState*node)
19151915
{
19161916
ExprContext*econtext=node->ss.ps.ps_ExprContext;
1917+
Agg*aggnode= (Agg*)node->ss.ps.plan;
19171918
intaggno;
19181919

19191920
node->agg_done= false;
19201921

19211922
node->ss.ps.ps_TupFromTlist= false;
19221923

1923-
if (((Agg*)node->ss.ps.plan)->aggstrategy==AGG_HASHED)
1924+
if (aggnode->aggstrategy==AGG_HASHED)
19241925
{
19251926
/*
19261927
* In the hashed case, if we haven't yet built the hash table then we
@@ -1932,11 +1933,13 @@ ExecReScanAgg(AggState *node)
19321933
return;
19331934

19341935
/*
1935-
* If we do have the hash table and the subplan does not have any
1936-
* parameter changes, then we can just rescan the existing hash table;
1937-
* no need to build it again.
1936+
* If we do have the hash table, and the subplan does not have any
1937+
* parameter changes, and none of our own parameter changes affect
1938+
* input expressions of the aggregated functions, then we can just
1939+
* rescan the existing hash table; no need to build it again.
19381940
*/
1939-
if (node->ss.ps.lefttree->chgParam==NULL)
1941+
if (node->ss.ps.lefttree->chgParam==NULL&&
1942+
!bms_overlap(node->ss.ps.chgParam,aggnode->aggParams))
19401943
{
19411944
ResetTupleHashIterator(node->hashtable,&node->hashiter);
19421945
return;
@@ -1973,7 +1976,7 @@ ExecReScanAgg(AggState *node)
19731976
*/
19741977
MemoryContextResetAndDeleteChildren(node->aggcontext);
19751978

1976-
if (((Agg*)node->ss.ps.plan)->aggstrategy==AGG_HASHED)
1979+
if (aggnode->aggstrategy==AGG_HASHED)
19771980
{
19781981
/* Rebuild an empty hash table */
19791982
build_hash_table(node);

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ _copyAgg(const Agg *from)
781781
COPY_POINTER_FIELD(grpOperators,from->numCols*sizeof(Oid));
782782
}
783783
COPY_SCALAR_FIELD(numGroups);
784+
COPY_BITMAPSET_FIELD(aggParams);
784785

785786
returnnewnode;
786787
}

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,7 @@ _outAgg(StringInfo str, const Agg *node)
645645
appendStringInfo(str," %u",node->grpOperators[i]);
646646

647647
WRITE_LONG_FIELD(numGroups);
648+
WRITE_BITMAPSET_FIELD(aggParams);
648649
}
649650

650651
staticvoid

‎src/backend/optimizer/plan/createplan.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4311,6 +4311,7 @@ make_agg(PlannerInfo *root, List *tlist, List *qual,
43114311
node->grpColIdx=grpColIdx;
43124312
node->grpOperators=grpOperators;
43134313
node->numGroups=numGroups;
4314+
node->aggParams=NULL;/* SS_finalize_plan() will fill this */
43144315

43154316
copy_plan_costsize(plan,lefttree);/* only care about copying size */
43164317
cost_agg(&agg_path,root,

‎src/backend/optimizer/plan/subselect.c

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ static Bitmapset *finalize_plan(PlannerInfo *root,
8080
Bitmapset*valid_params,
8181
Bitmapset*scan_params);
8282
staticboolfinalize_primnode(Node*node,finalize_primnode_context*context);
83+
staticboolfinalize_agg_primnode(Node*node,finalize_primnode_context*context);
8384

8485

8586
/*
@@ -2356,6 +2357,29 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
23562357
locally_added_param);
23572358
break;
23582359

2360+
caseT_Agg:
2361+
{
2362+
Agg*agg= (Agg*)plan;
2363+
2364+
/*
2365+
* AGG_HASHED plans need to know which Params are referenced
2366+
* in aggregate calls. Do a separate scan to identify them.
2367+
*/
2368+
if (agg->aggstrategy==AGG_HASHED)
2369+
{
2370+
finalize_primnode_contextaggcontext;
2371+
2372+
aggcontext.root=root;
2373+
aggcontext.paramids=NULL;
2374+
finalize_agg_primnode((Node*)agg->plan.targetlist,
2375+
&aggcontext);
2376+
finalize_agg_primnode((Node*)agg->plan.qual,
2377+
&aggcontext);
2378+
agg->aggParams=aggcontext.paramids;
2379+
}
2380+
}
2381+
break;
2382+
23592383
caseT_WindowAgg:
23602384
finalize_primnode(((WindowAgg*)plan)->startOffset,
23612385
&context);
@@ -2364,7 +2388,6 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
23642388
break;
23652389

23662390
caseT_Hash:
2367-
caseT_Agg:
23682391
caseT_Material:
23692392
caseT_Sort:
23702393
caseT_Unique:
@@ -2508,6 +2531,28 @@ finalize_primnode(Node *node, finalize_primnode_context *context)
25082531
(void*)context);
25092532
}
25102533

2534+
/*
2535+
* finalize_agg_primnode: find all Aggref nodes in the given expression tree,
2536+
* and add IDs of all PARAM_EXEC params appearing within their aggregated
2537+
* arguments to the result set.
2538+
*/
2539+
staticbool
2540+
finalize_agg_primnode(Node*node,finalize_primnode_context*context)
2541+
{
2542+
if (node==NULL)
2543+
return false;
2544+
if (IsA(node,Aggref))
2545+
{
2546+
Aggref*agg= (Aggref*)node;
2547+
2548+
/* we should not consider the direct arguments, if any */
2549+
finalize_primnode((Node*)agg->args,context);
2550+
return false;/* there can't be any Aggrefs below here */
2551+
}
2552+
returnexpression_tree_walker(node,finalize_agg_primnode,
2553+
(void*)context);
2554+
}
2555+
25112556
/*
25122557
* SS_make_initplan_from_plan - given a plan tree, make it an InitPlan
25132558
*

‎src/include/nodes/plannodes.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,8 @@ typedef struct Agg
633633
AttrNumber*grpColIdx;/* their indexes in the target list */
634634
Oid*grpOperators;/* equality operators to compare with */
635635
longnumGroups;/* estimated number of groups in input */
636+
Bitmapset*aggParams;/* IDs of Params used in Aggref inputs */
637+
/* Note: planner provides numGroups & aggParams only in AGG_HASHED case */
636638
}Agg;
637639

638640
/* ----------------

‎src/test/regress/expected/aggregates.out

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,79 @@ from tenk1 o;
305305
9999
306306
(1 row)
307307

308+
-- Test handling of Params within aggregate arguments in hashed aggregation.
309+
-- Per bug report from Jeevan Chalke.
310+
explain (verbose, costs off)
311+
select s1, s2, sm
312+
from generate_series(1, 3) s1,
313+
lateral (select s2, sum(s1 + s2) sm
314+
from generate_series(1, 3) s2 group by s2) ss
315+
order by 1, 2;
316+
QUERY PLAN
317+
------------------------------------------------------------------
318+
Sort
319+
Output: s1.s1, s2.s2, (sum((s1.s1 + s2.s2)))
320+
Sort Key: s1.s1, s2.s2
321+
-> Nested Loop
322+
Output: s1.s1, s2.s2, (sum((s1.s1 + s2.s2)))
323+
-> Function Scan on pg_catalog.generate_series s1
324+
Output: s1.s1
325+
Function Call: generate_series(1, 3)
326+
-> HashAggregate
327+
Output: s2.s2, sum((s1.s1 + s2.s2))
328+
-> Function Scan on pg_catalog.generate_series s2
329+
Output: s2.s2
330+
Function Call: generate_series(1, 3)
331+
(13 rows)
332+
333+
select s1, s2, sm
334+
from generate_series(1, 3) s1,
335+
lateral (select s2, sum(s1 + s2) sm
336+
from generate_series(1, 3) s2 group by s2) ss
337+
order by 1, 2;
338+
s1 | s2 | sm
339+
----+----+----
340+
1 | 1 | 2
341+
1 | 2 | 3
342+
1 | 3 | 4
343+
2 | 1 | 3
344+
2 | 2 | 4
345+
2 | 3 | 5
346+
3 | 1 | 4
347+
3 | 2 | 5
348+
3 | 3 | 6
349+
(9 rows)
350+
351+
explain (verbose, costs off)
352+
select array(select sum(x+y) s
353+
from generate_series(1,3) y group by y order by s)
354+
from generate_series(1,3) x;
355+
QUERY PLAN
356+
-------------------------------------------------------------------
357+
Function Scan on pg_catalog.generate_series x
358+
Output: (SubPlan 1)
359+
Function Call: generate_series(1, 3)
360+
SubPlan 1
361+
-> Sort
362+
Output: (sum((x.x + y.y))), y.y
363+
Sort Key: (sum((x.x + y.y)))
364+
-> HashAggregate
365+
Output: sum((x.x + y.y)), y.y
366+
-> Function Scan on pg_catalog.generate_series y
367+
Output: y.y
368+
Function Call: generate_series(1, 3)
369+
(12 rows)
370+
371+
select array(select sum(x+y) s
372+
from generate_series(1,3) y group by y order by s)
373+
from generate_series(1,3) x;
374+
array
375+
---------
376+
{2,3,4}
377+
{3,4,5}
378+
{4,5,6}
379+
(3 rows)
380+
308381
--
309382
-- test for bitwise integer aggregates
310383
--

‎src/test/regress/sql/aggregates.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,28 @@ select
8686
(selectmax((selecti.unique2from tenk1 iwherei.unique1=o.unique1)))
8787
from tenk1 o;
8888

89+
-- Test handling of Params within aggregate arguments in hashed aggregation.
90+
-- Per bug report from Jeevan Chalke.
91+
explain (verbose, costs off)
92+
select s1, s2, sm
93+
from generate_series(1,3) s1,
94+
lateral (select s2,sum(s1+ s2) sm
95+
from generate_series(1,3) s2group by s2) ss
96+
order by1,2;
97+
select s1, s2, sm
98+
from generate_series(1,3) s1,
99+
lateral (select s2,sum(s1+ s2) sm
100+
from generate_series(1,3) s2group by s2) ss
101+
order by1,2;
102+
103+
explain (verbose, costs off)
104+
select array(selectsum(x+y) s
105+
from generate_series(1,3) ygroup by yorder by s)
106+
from generate_series(1,3) x;
107+
select array(selectsum(x+y) s
108+
from generate_series(1,3) ygroup by yorder by s)
109+
from generate_series(1,3) x;
110+
89111
--
90112
-- test for bitwise integer aggregates
91113
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp