Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 7da5159

Browse files
committed
Fix incorrect presorted DISTINCT aggregate if condition
Here we fix a faulty "if" condition which failed to correctly handle two or more consecutive NULL transition values when checking if the new value is DISTINCT from the old value for presorted aggregates. Given a suitably non-strict aggregate transition function, a byref aggregate could cause a crash due to calling the type's equality function and passing along a (Datum) 0 value to test for equality, the equality function would then try to dereference that 0 Datum and segfault. For byval types, there'd have been no crash and the equality function would have seen that the two 0 Datums matched, which (only by chance) meant the calling code would have worked correctly. Here we ensure that we only call the equality function when neither of the input values are NULL. This code is all new as of 1349d27, so no backpatch needed. Reported-by: Fujii Masao Discussion: https://postgr.es/m/860c6d6f-a3c5-3ae9-9da2-827177bede06@oss.nttdata.com
1 parent 836c31b commit 7da5159

File tree

3 files changed

+15
-3
lines changed

3 files changed

+15
-3
lines changed

src/backend/executor/execExprInterp.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4250,9 +4250,9 @@ ExecEvalPreOrderedDistinctSingle(AggState *aggstate, AggStatePerTrans pertrans)
42504250

42514251
if (!pertrans->haslast ||
42524252
pertrans->lastisnull != isnull ||
4253-
!DatumGetBool(FunctionCall2Coll(&pertrans->equalfnOne,
4254-
pertrans->aggCollation,
4255-
pertrans->lastdatum, value)))
4253+
(!isnull && !DatumGetBool(FunctionCall2Coll(&pertrans->equalfnOne,
4254+
pertrans->aggCollation,
4255+
pertrans->lastdatum, value))))
42564256
{
42574257
if (pertrans->haslast && !pertrans->inputtypeByVal)
42584258
pfree(DatumGetPointer(pertrans->lastdatum));

src/test/regress/expected/aggregates.out

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,14 @@ group by ten;
15061506
-> Seq Scan on tenk1
15071507
(5 rows)
15081508

1509+
-- Ensure consecutive NULLs are properly treated as distinct from each other
1510+
select array_agg(distinct val)
1511+
from (select null as val from generate_series(1, 2));
1512+
array_agg
1513+
-----------
1514+
{NULL}
1515+
(1 row)
1516+
15091517
-- Ensure no ordering is requested when enable_presorted_aggregate is off
15101518
set enable_presorted_aggregate to off;
15111519
explain (costs off)

src/test/regress/sql/aggregates.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,10 @@ select
567567
from tenk1
568568
group by ten;
569569

570+
-- Ensure consecutive NULLs are properly treated as distinct from each other
571+
select array_agg(distinct val)
572+
from (select null as val from generate_series(1, 2));
573+
570574
-- Ensure no ordering is requested when enable_presorted_aggregate is off
571575
set enable_presorted_aggregate to off;
572576
explain (costs off)

0 commit comments

Comments
 (0)