Fix use of incorrect TupleTableSlot in DISTINCT aggregates

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

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index b1f92e013e21d23f6c65cdc1c5247ddc40bbf575..3c17cc6b1e17d603f309b106b99ee0cf38689d43 100644 (file)
--- 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 d8271da4d1fb098023ec6a27fa1502aea9d44f92..f635c5a1afb710d4769757a5390a350432620c29 100644 (file)
--- 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 75c78be640b2914ab624a9eff4bafabd529d21d1..cc8f0efad55f2db77243f15df3c199234456ff63 100644 (file)
--- 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