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

Commit 6298673

Browse files
committed
Fix use of incorrect TupleTableSlot in DISTINCT aggregates
1349d27 added code to allow DISTINCT and ORDER BY aggregates to work more efficiently by using presorted input. That commit added some code that made use of the AggState's tmpcontext and adjusted the ecxt_outertuple and ecxt_innertuple slots before checking if the current row is distinct from the previously seen row. That code forgot to set the TupleTableSlots back to what they were originally, which could result in errors such as: ERROR: attribute 1 of type record has wrong type This only affects aggregate functions which have multiple arguments when DISTINCT is used. For example: string_agg(DISTINCT col, ', ') Thanks to Tom Lane for identifying the breaking commit. Bug: #18264 Reported-by: Vojtěch Beneš Discussion: https://postgr.es/m/18264-e363593d7e9feb7d@postgresql.org Backpatch-through: 16, where 1349d27 was added
1 parent 79a2af3 commit 6298673

File tree

3 files changed

+39
-3
lines changed

3 files changed

+39
-3
lines changed

src/backend/executor/execExprInterp.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4538,12 +4538,16 @@ ExecEvalPreOrderedDistinctSingle(AggState *aggstate, AggStatePerTrans pertrans)
45384538
/*
45394539
* ExecEvalPreOrderedDistinctMulti
45404540
* Returns true when the aggregate input is distinct from the previous
4541-
* input and returns false when the input matches the previous input.
4541+
* input and returns false when the input matches the previous input, or
4542+
* when there was no previous input.
45424543
*/
45434544
bool
45444545
ExecEvalPreOrderedDistinctMulti(AggState *aggstate, AggStatePerTrans pertrans)
45454546
{
45464547
ExprContext *tmpcontext = aggstate->tmpcontext;
4548+
bool isdistinct = false; /* for now */
4549+
TupleTableSlot *save_outer;
4550+
TupleTableSlot *save_inner;
45474551

45484552
for (int i = 0; i < pertrans->numTransInputs; i++)
45494553
{
@@ -4555,6 +4559,10 @@ ExecEvalPreOrderedDistinctMulti(AggState *aggstate, AggStatePerTrans pertrans)
45554559
pertrans->sortslot->tts_nvalid = pertrans->numInputs;
45564560
ExecStoreVirtualTuple(pertrans->sortslot);
45574561

4562+
/* save the previous slots before we overwrite them */
4563+
save_outer = tmpcontext->ecxt_outertuple;
4564+
save_inner = tmpcontext->ecxt_innertuple;
4565+
45584566
tmpcontext->ecxt_outertuple = pertrans->sortslot;
45594567
tmpcontext->ecxt_innertuple = pertrans->uniqslot;
45604568

@@ -4566,9 +4574,15 @@ ExecEvalPreOrderedDistinctMulti(AggState *aggstate, AggStatePerTrans pertrans)
45664574

45674575
pertrans->haslast = true;
45684576
ExecCopySlot(pertrans->uniqslot, pertrans->sortslot);
4569-
return true;
4577+
4578+
isdistinct = true;
45704579
}
4571-
return false;
4580+
4581+
/* restore the original slots */
4582+
tmpcontext->ecxt_outertuple = save_outer;
4583+
tmpcontext->ecxt_innertuple = save_inner;
4584+
4585+
return isdistinct;
45724586
}
45734587

45744588
/*

src/test/regress/expected/aggregates.out

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,6 +1694,19 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b)
16941694
{"(0,,)","(1,3,foo)","(2,2,bar)","(3,1,baz)"}
16951695
(1 row)
16961696

1697+
-- test a more complex permutation that has previous caused issues
1698+
select
1699+
string_agg(distinct 'a', ','),
1700+
sum((
1701+
select sum(1)
1702+
from (values(1)) b(id)
1703+
where a.id = b.id
1704+
)) from unnest(array[1]) a(id);
1705+
string_agg | sum
1706+
------------+-----
1707+
a | 1
1708+
(1 row)
1709+
16971710
-- check node I/O via view creation and usage, also deparsing logic
16981711
create view agg_view1 as
16991712
select aggfns(a,b,c)

src/test/regress/sql/aggregates.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,15 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b)
643643
from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c),
644644
generate_series(1,2) i;
645645

646+
-- test a more complex permutation that has previous caused issues
647+
select
648+
string_agg(distinct 'a', ','),
649+
sum((
650+
select sum(1)
651+
from (values(1)) b(id)
652+
where a.id = b.id
653+
)) from unnest(array[1]) a(id);
654+
646655
-- check node I/O via view creation and usage, also deparsing logic
647656

648657
create view agg_view1 as

0 commit comments

Comments
 (0)