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

Commitba1dfbe

Browse files
committed
Fix edge case leading to agg transitions skipping ExecAggTransReparent() calls.
The code checking whether an aggregate transition value needs to bereparented into the current context has always only compared thetransition return value with the previous transition value by datum,i.e. without regard for NULLness. This normally works, because whenthe transition function returns NULL (via fcinfo->isnull), it'llreturn a value that won't be the same as its input value.But there's no hard requirement that that's the case. And it turnsout, it's possible to hit this case (see discussion or reproducers),leading to a non-null transition value not being reparented, followedby a crash caused by that.Instead of adding another comparison of NULLness, instead haveExecAggTransReparent() ensure that pergroup->transValue ends up as 0when the new transition value is NULL. That avoids having to add anadditional branch to the much more common cases of the transitionfunction returning the old transition value (which is a pointer inthis case), and when the new value is different, but not NULL.In branches since69c3936, also deduplicate the reparenting codebetween the expression evaluation based transitions, and the path forordered aggregates.Reported-By: Teodor Sigaev, Nikita GlukhovAuthor: Andres FreundDiscussion:https://postgr.es/m/bd34e930-cfec-ea9b-3827-a8bc50891393@sigaev.ruBackpatch: 9.4-, this issue has existed since at least 7.4
1 parentdbe405b commitba1dfbe

File tree

1 file changed

+19
-0
lines changed

1 file changed

+19
-0
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,15 @@ advance_transition_function(AggState *aggstate,
497497
* If pass-by-ref datatype, must copy the new value into aggcontext and
498498
* pfree the prior transValue. But if transfn returned a pointer to its
499499
* first input, we don't need to do anything.
500+
*
501+
* It's safe to compare newVal with pergroupstate->transValue without
502+
* regard for either being NULL, because we below take care to set
503+
* transValue to 0 when NULL. Otherwise we could end up accidentally not
504+
* reparenting, when the transValue has the same numerical value as
505+
* newVal, despite being NULL. This is a somewhat hot path, making it
506+
* undesirable to instead solve this with another branch for the common
507+
* case of the transition function returning its (modified) input
508+
* argument.
500509
*/
501510
if (!peraggstate->transtypeByVal&&
502511
DatumGetPointer(newVal)!=DatumGetPointer(pergroupstate->transValue))
@@ -508,6 +517,16 @@ advance_transition_function(AggState *aggstate,
508517
peraggstate->transtypeByVal,
509518
peraggstate->transtypeLen);
510519
}
520+
else
521+
{
522+
/*
523+
* Ensure that pergroupstate->transValue ends up being 0, so we
524+
* can safely compare newVal/transValue without having to check
525+
* their respective nullness.
526+
*/
527+
newVal= (Datum)0;
528+
}
529+
511530
if (!pergroupstate->transValueIsNull)
512531
pfree(DatumGetPointer(pergroupstate->transValue));
513532
}

0 commit comments

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