Fix volatile function evaluation in eager aggregation

Pushing aggregates containing volatile functions below a join can
violate volatility semantics by changing the number of times the
function is executed.

Here we check the Aggref nodes in the targetlist and havingQual for
volatile functions and disable eager aggregation when such functions
are present.

Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Discussion: https://postgr.es/m/CAMbWs48A53PY1Y4zoj7YhxPww9fO1hfnbdntKfA855zpXfVFRA@mail.gmail.com

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index b207b8d913b29099bede70404a5acf3336e807de..96ee312ebdf523e9627237869915f68716663833 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -810,6 +810,17 @@ create_agg_clause_infos(PlannerInfo *root)
                Assert(aggref->aggorder == NIL);
                Assert(aggref->aggdistinct == NIL);
 
+               /*
+                * We cannot push down aggregates that contain volatile functions.
+                * Doing so would change the number of times the function is
+                * evaluated.
+                */
+               if (contain_volatile_functions((Node *) aggref))
+               {
+                       eager_agg_applicable = false;
+                       break;
+               }
+
                /*
                 * If there are any securityQuals, do not try to apply eager
                 * aggregation if any non-leakproof aggregate functions are present.

diff --git a/src/test/regress/expected/eager_aggregate.out b/src/test/regress/expected/eager_aggregate.out
index 5ac966186f7c895b6f54c9eed974ec126a7f820c..456d32eb13d8ab02bc4e02aa14595dd1d189bfee 100644 (file)
--- a/src/test/regress/expected/eager_aggregate.out
+++ b/src/test/regress/expected/eager_aggregate.out
@@ -428,6 +428,44 @@ GROUP BY t1.a ORDER BY t1.a;
 
 RESET geqo;
 RESET geqo_threshold;
+-- Ensure eager aggregation is not applied because random() is a volatile
+-- function
+EXPLAIN (COSTS OFF)
+SELECT t1.a, avg(t2.c + random())
+  FROM eager_agg_t1 t1
+  JOIN eager_agg_t2 t2 ON t1.b = t2.b
+GROUP BY t1.a ORDER BY t1.a;
+                     QUERY PLAN                      
+-----------------------------------------------------
+ GroupAggregate
+   Group Key: t1.a
+   ->  Sort
+         Sort Key: t1.a
+         ->  Hash Join
+               Hash Cond: (t2.b = t1.b)
+               ->  Seq Scan on eager_agg_t2 t2
+               ->  Hash
+                     ->  Seq Scan on eager_agg_t1 t1
+(9 rows)
+
+EXPLAIN (COSTS OFF)
+SELECT t1.a, avg(t2.c) FILTER (WHERE random() > 0.5)
+  FROM eager_agg_t1 t1
+  JOIN eager_agg_t2 t2 ON t1.b = t2.b
+GROUP BY t1.a ORDER BY t1.a;
+                     QUERY PLAN                      
+-----------------------------------------------------
+ GroupAggregate
+   Group Key: t1.a
+   ->  Sort
+         Sort Key: t1.a
+         ->  Hash Join
+               Hash Cond: (t2.b = t1.b)
+               ->  Seq Scan on eager_agg_t2 t2
+               ->  Hash
+                     ->  Seq Scan on eager_agg_t1 t1
+(9 rows)
+
 DROP TABLE eager_agg_t1;
 DROP TABLE eager_agg_t2;
 DROP TABLE eager_agg_t3;

diff --git a/src/test/regress/sql/eager_aggregate.sql b/src/test/regress/sql/eager_aggregate.sql
index abe6d6ae09f1dadb050c9eee3e8120a4bf7cf7cb..53d9b377a64d7043e9c6284d4b45653ec7a78e4a 100644 (file)
--- a/src/test/regress/sql/eager_aggregate.sql
+++ b/src/test/regress/sql/eager_aggregate.sql
@@ -163,6 +163,20 @@ GROUP BY t1.a ORDER BY t1.a;
 RESET geqo;
 RESET geqo_threshold;
 
+-- Ensure eager aggregation is not applied because random() is a volatile
+-- function
+EXPLAIN (COSTS OFF)
+SELECT t1.a, avg(t2.c + random())
+  FROM eager_agg_t1 t1
+  JOIN eager_agg_t2 t2 ON t1.b = t2.b
+GROUP BY t1.a ORDER BY t1.a;
+
+EXPLAIN (COSTS OFF)
+SELECT t1.a, avg(t2.c) FILTER (WHERE random() > 0.5)
+  FROM eager_agg_t1 t1
+  JOIN eager_agg_t2 t2 ON t1.b = t2.b
+GROUP BY t1.a ORDER BY t1.a;
+
 DROP TABLE eager_agg_t1;
 DROP TABLE eager_agg_t2;
 DROP TABLE eager_agg_t3;