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

Commitd48bf6a

Browse files
committed
Fix AggGetAggref() so it won't lie to aggregate final functions.
If we merge the transition calculations for two different aggregates,it's reasonable to assume that the transition function should not carewhich of those Aggref structs it gets from AggGetAggref(). It is notreasonable to make the same assumption about an aggregate final function,however. Commit804163b broke this, as it will pass whichever Aggrefwas first associated with the transition state in both cases.This doesn't create an observable bug so far as the core system isconcerned, because the only existing uses of AggGetAggref() are inordered-set aggregates that happen to not pay attention to anythingbut the input properties of the Aggref; and besides that, we disabledsharing of transition calculations for OSAs yesterday. Nonetheless,if some third-party code were using AggGetAggref() in a normal aggregate,they would be entitled to call this a bug. Hence, back-patch the fixto 9.6 where the problem was introduced.In passing, improve some of the comments about transition state sharing.Discussion:https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com
1 parent5c926e6 commitd48bf6a

File tree

2 files changed

+44
-24
lines changed

2 files changed

+44
-24
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1524,8 +1524,8 @@ finalize_aggregate(AggState *aggstate,
15241524
{
15251525
intnumFinalArgs=peragg->numFinalArgs;
15261526

1527-
/* set up aggstate->curpertrans for AggGetAggref() */
1528-
aggstate->curpertrans=pertrans;
1527+
/* set up aggstate->curperagg for AggGetAggref() */
1528+
aggstate->curperagg=peragg;
15291529

15301530
InitFunctionCallInfoData(fcinfo,&peragg->finalfn,
15311531
numFinalArgs,
@@ -1558,7 +1558,7 @@ finalize_aggregate(AggState *aggstate,
15581558
*resultVal=FunctionCallInvoke(&fcinfo);
15591559
*resultIsNull=fcinfo.isnull;
15601560
}
1561-
aggstate->curpertrans=NULL;
1561+
aggstate->curperagg=NULL;
15621562
}
15631563
else
15641564
{
@@ -2708,6 +2708,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
27082708
aggstate->current_set=0;
27092709
aggstate->peragg=NULL;
27102710
aggstate->pertrans=NULL;
2711+
aggstate->curperagg=NULL;
27112712
aggstate->curpertrans=NULL;
27122713
aggstate->input_done= false;
27132714
aggstate->agg_done= false;
@@ -3056,27 +3057,29 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
30563057
*
30573058
* Scenarios:
30583059
*
3059-
* 1.An aggregate functionappears more than once in query:
3060+
* 1.Identical aggregate functioncalls appear in the query:
30603061
*
30613062
* SELECT SUM(x) FROM ... HAVING SUM(x) > 0
30623063
*
3063-
* Since the aggregates are the identical, we only need to calculate
3064-
* the calculate it once. Both aggregates will share the same 'aggno'
3065-
* value.
3064+
* Since these aggregates are identical, we only need to calculate
3065+
* the value once. Both aggregates will share the same 'aggno' value.
30663066
*
30673067
* 2. Two different aggregate functions appear in the query, but the
3068-
* aggregates have the same transitionfunction and initial value, but
3069-
* different finalfunction:
3068+
* aggregates have the samearguments,transitionfunctions and
3069+
*initial values (and, presumably,different finalfunctions):
30703070
*
3071-
* SELECTSUM(x),AVG(x) FROM ...
3071+
* SELECTAVG(x),STDDEV(x) FROM ...
30723072
*
30733073
* In this case we must create a new peragg for the varying aggregate,
3074-
* and need to call the final functions separately, but can share the
3075-
* same transition state.
3074+
* and we need to call the final functions separately, but we need
3075+
* only run the transition function once. (This requires that the
3076+
* final functions be nondestructive of the transition state, but
3077+
* that's required anyway for other reasons.)
30763078
*
3077-
* For either of these optimizations to be valid, the aggregate's
3078-
* arguments must be the same, including any modifiers such as ORDER BY,
3079-
* DISTINCT and FILTER, and they mustn't contain any volatile functions.
3079+
* For either of these optimizations to be valid, all aggregate properties
3080+
* used in the transition phase must be the same, including any modifiers
3081+
* such as ORDER BY, DISTINCT and FILTER, and the arguments mustn't
3082+
* contain any volatile functions.
30803083
* -----------------
30813084
*/
30823085
aggno=-1;
@@ -3719,7 +3722,7 @@ GetAggInitVal(Datum textInitVal, Oid transtype)
37193722
*
37203723
* As a side-effect, this also collects a list of existing per-Trans structs
37213724
* with matching inputs. If no identical Aggref is found, the list is passed
3722-
* later tofind_compatible_perstate, to see if we can at least reuse the
3725+
* later tofind_compatible_pertrans, to see if we can at least reuse the
37233726
* state value of another aggregate.
37243727
*/
37253728
staticint
@@ -3739,11 +3742,12 @@ find_compatible_peragg(Aggref *newagg, AggState *aggstate,
37393742

37403743
/*
37413744
* Search through the list of already seen aggregates. If we find an
3742-
* existing aggregate with the same aggregate function and input
3743-
* parameters as an existing one, then we can re-use that one. While
3745+
* existing identical aggregate call, then we can re-use that one. While
37443746
* searching, we'll also collect a list of Aggrefs with the same input
37453747
* parameters. If no matching Aggref is found, the caller can potentially
3746-
* still re-use the transition state of one of them.
3748+
* still re-use the transition state of one of them. (At this stage we
3749+
* just compare the parsetrees; whether different aggregates share the
3750+
* same transition function will be checked later.)
37473751
*/
37483752
for (aggno=0;aggno <=lastaggno;aggno++)
37493753
{
@@ -3792,7 +3796,7 @@ find_compatible_peragg(Aggref *newagg, AggState *aggstate,
37923796
* struct
37933797
*
37943798
* Searches the list of transnos for a per-Trans struct with the same
3795-
* transitionstate and initial condition. (The inputs have already been
3799+
* transitionfunction and initial condition. (The inputs have already been
37963800
* verified to match.)
37973801
*/
37983802
staticint
@@ -3838,16 +3842,16 @@ find_compatible_pertrans(AggState *aggstate, Aggref *newagg,
38383842
aggdeserialfn!=pertrans->deserialfn_oid)
38393843
continue;
38403844

3841-
/* Check that the initial condition matches, too. */
3845+
/*
3846+
* Check that the initial condition matches, too.
3847+
*/
38423848
if (initValueIsNull&&pertrans->initValueIsNull)
38433849
returntransno;
38443850

38453851
if (!initValueIsNull&& !pertrans->initValueIsNull&&
38463852
datumIsEqual(initValue,pertrans->initValue,
38473853
pertrans->transtypeByVal,pertrans->transtypeLen))
3848-
{
38493854
returntransno;
3850-
}
38513855
}
38523856
return-1;
38533857
}
@@ -4066,6 +4070,13 @@ AggCheckCallContext(FunctionCallInfo fcinfo, MemoryContext *aggcontext)
40664070
* If the function is being called as an aggregate support function,
40674071
* return the Aggref node for the aggregate call. Otherwise, return NULL.
40684072
*
4073+
* Aggregates sharing the same inputs and transition functions can get
4074+
* merged into a single transition calculation. If the transition function
4075+
* calls AggGetAggref, it will get some one of the Aggrefs for which it is
4076+
* executing. It must therefore not pay attention to the Aggref fields that
4077+
* relate to the final function, as those are indeterminate. But if a final
4078+
* function calls AggGetAggref, it will get a precise result.
4079+
*
40694080
* Note that if an aggregate is being used as a window function, this will
40704081
* return NULL. We could provide a similar function to return the relevant
40714082
* WindowFunc node in such cases, but it's not needed yet.
@@ -4075,8 +4086,16 @@ AggGetAggref(FunctionCallInfo fcinfo)
40754086
{
40764087
if (fcinfo->context&&IsA(fcinfo->context,AggState))
40774088
{
4089+
AggStatePerAggcurperagg;
40784090
AggStatePerTranscurpertrans;
40794091

4092+
/* check curperagg (valid when in a final function) */
4093+
curperagg= ((AggState*)fcinfo->context)->curperagg;
4094+
4095+
if (curperagg)
4096+
returncurperagg->aggref;
4097+
4098+
/* check curpertrans (valid when in a transition function) */
40804099
curpertrans= ((AggState*)fcinfo->context)->curpertrans;
40814100

40824101
if (curpertrans)

‎src/include/nodes/execnodes.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1795,7 +1795,7 @@ typedef struct AggState
17951795
ExprContext**aggcontexts;/* econtexts for long-lived data (per GS) */
17961796
ExprContext*tmpcontext;/* econtext for input expressions */
17971797
ExprContext*curaggcontext;/* currently active aggcontext */
1798-
AggStatePerTranscurpertrans;/* currently active trans state */
1798+
AggStatePerTranscurpertrans;/* currently active trans state, if any */
17991799
boolinput_done;/* indicates end of input */
18001800
boolagg_done;/* indicates completion of Agg scan */
18011801
intprojected_set;/* The last projected grouping set */
@@ -1820,6 +1820,7 @@ typedef struct AggState
18201820
TupleTableSlot*evalslot;/* slot for agg inputs */
18211821
ProjectionInfo*evalproj;/* projection machinery */
18221822
TupleDescevaldesc;/* descriptor of input tuples */
1823+
AggStatePerAggcurperagg;/* currently active aggregate, if any */
18231824
}AggState;
18241825

18251826
/* ----------------

0 commit comments

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