pgsql: Fix edge case leading to agg transitions skipping ExecAggTransRe

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Fix edge case leading to agg transitions skipping ExecAggTransRe
Date: 2020-01-21 07:34:29
Message-ID: E1ito3R-0006jb-8n@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Fix edge case leading to agg transitions skipping ExecAggTransReparent() calls.

The code checking whether an aggregate transition value needs to be
reparented into the current context has always only compared the
transition return value with the previous transition value by datum,
i.e. without regard for NULLness. This normally works, because when
the transition function returns NULL (via fcinfo->isnull), it'll
return 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 turns
out, it's possible to hit this case (see discussion or reproducers),
leading to a non-null transition value not being reparented, followed
by a crash caused by that.

Instead of adding another comparison of NULLness, instead have
ExecAggTransReparent() ensure that pergroup->transValue ends up as 0
when the new transition value is NULL. That avoids having to add an
additional branch to the much more common cases of the transition
function returning the old transition value (which is a pointer in
this case), and when the new value is different, but not NULL.

In branches since 69c3936a149, also deduplicate the reparenting code
between the expression evaluation based transitions, and the path for
ordered aggregates.

Reported-By: Teodor Sigaev, Nikita Glukhov
Author: Andres Freund
Discussion: https://postgr.es/m/bd34e930-cfec-ea9b-3827-a8bc50891393@sigaev.ru
Backpatch: 9.4-, this issue has existed since at least 7.4

Branch
------
REL9_4_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/ba1dfbe22d3002ff56933ee6ebd26b9bc9be3d86

Modified Files
--------------
src/backend/executor/nodeAgg.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2020-01-21 21:17:29 pgsql: Clarify behavior of adding and altering a column in same ALTER c
Previous Message Andres Freund 2020-01-21 07:34:26 pgsql: Fix edge case leading to agg transitions skipping ExecAggTransRe