From ae69c4fcf1337af399a788ab8d96af540bd1cd8e Mon Sep 17 00:00:00 2001 From: David Rowley <drowley@postgresql.org> Date: Thu, 4 Jan 2024 20:38:25 +1300 Subject: [PATCH] Fix use of incorrect TupleTableSlot in DISTINCT aggregates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1349d2790 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 1349d2790 was added --- src/backend/executor/execExprInterp.c | 20 +++++++++++++++++--- src/test/regress/expected/aggregates.out | 13 +++++++++++++ src/test/regress/sql/aggregates.sql | 9 +++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index b1f92e013e2..3c17cc6b1e1 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4579,12 +4579,16 @@ ExecEvalPreOrderedDistinctSingle(AggState *aggstate, AggStatePerTrans pertrans) /* * ExecEvalPreOrderedDistinctMulti * Returns true when the aggregate input is distinct from the previous - * input and returns false when the input matches the previous input. + * input and returns false when the input matches the previous input, or + * when there was no previous input. */ bool ExecEvalPreOrderedDistinctMulti(AggState *aggstate, AggStatePerTrans pertrans) { ExprContext *tmpcontext = aggstate->tmpcontext; + bool isdistinct = false; /* for now */ + TupleTableSlot *save_outer; + TupleTableSlot *save_inner; for (int i = 0; i < pertrans->numTransInputs; i++) { @@ -4596,6 +4600,10 @@ ExecEvalPreOrderedDistinctMulti(AggState *aggstate, AggStatePerTrans pertrans) pertrans->sortslot->tts_nvalid = pertrans->numInputs; ExecStoreVirtualTuple(pertrans->sortslot); + /* save the previous slots before we overwrite them */ + save_outer = tmpcontext->ecxt_outertuple; + save_inner = tmpcontext->ecxt_innertuple; + tmpcontext->ecxt_outertuple = pertrans->sortslot; tmpcontext->ecxt_innertuple = pertrans->uniqslot; @@ -4607,9 +4615,15 @@ ExecEvalPreOrderedDistinctMulti(AggState *aggstate, AggStatePerTrans pertrans) pertrans->haslast = true; ExecCopySlot(pertrans->uniqslot, pertrans->sortslot); - return true; + + isdistinct = true; } - return false; + + /* restore the original slots */ + tmpcontext->ecxt_outertuple = save_outer; + tmpcontext->ecxt_innertuple = save_inner; + + return isdistinct; } /* diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index d8271da4d1f..f635c5a1afb 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -1694,6 +1694,19 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) {"(0,,)","(1,3,foo)","(2,2,bar)","(3,1,baz)"} (1 row) +-- test a more complex permutation that has previous caused issues +select + string_agg(distinct 'a', ','), + sum(( + select sum(1) + from (values(1)) b(id) + where a.id = b.id +)) from unnest(array[1]) a(id); + string_agg | sum +------------+----- + a | 1 +(1 row) + -- check node I/O via view creation and usage, also deparsing logic create view agg_view1 as select aggfns(a,b,c) diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 75c78be640b..cc8f0efad55 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -643,6 +643,15 @@ select aggfns(distinct a,b,c order by a,c using ~<~,b) from (values (1,3,'foo'),(0,null,null),(2,2,'bar'),(3,1,'baz')) v(a,b,c), generate_series(1,2) i; +-- test a more complex permutation that has previous caused issues +select + string_agg(distinct 'a', ','), + sum(( + select sum(1) + from (values(1)) b(id) + where a.id = b.id +)) from unnest(array[1]) a(id); + -- check node I/O via view creation and usage, also deparsing logic create view agg_view1 as