]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix planner error with SRFs and grouping sets
authorRichard Guo <rguo@postgresql.org>
Thu, 25 Dec 2025 03:12:52 +0000 (12:12 +0900)
committerRichard Guo <rguo@postgresql.org>
Thu, 25 Dec 2025 03:15:09 +0000 (12:15 +0900)
If there are any SRFs in a PathTarget, we must separate it into
SRF-computing and SRF-free targets.  This is because the executor can
only handle SRFs that appear at the top level of the targetlist of a
ProjectSet plan node.

If we find a subexpression that matches an expression already computed
in the previous plan level, we should treat it like a Var and should
not split it again.  setrefs.c will later replace the expression with
a Var referencing the subplan output.

However, when processing the grouping target for grouping sets, the
planner can fail to recognize that an expression is already computed
in the scan/join phase.  The root cause is a mismatch in the
nullingrels bits.  Expressions in the grouping target carry the
grouping nulling bit in their nullingrels to indicate that they can be
nulled by the grouping step.  However, the corresponding expressions
in the scan/join target do not have these bits.

As a result, the exact match check in list_member() fails, leading the
planner to incorrectly believe that the expression needs to be
re-evaluated from its arguments, which are often not available in the
subplan.  This can lead to planner errors such as "variable not found
in subplan target list".

To fix, ignore the grouping nulling bit when checking whether an
expression from the grouping target is available in the pre-grouping
input target.  This aligns with the matching logic in setrefs.c.

Backpatch to v18, where this issue was introduced.

Bug: #19353
Reported-by: Marian MULLER REBEYROL <marian.muller@serli.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/19353-aaa179bba986a19b@postgresql.org
Backpatch-through: 18

src/backend/optimizer/plan/planner.c
src/backend/optimizer/util/tlist.c
src/include/optimizer/tlist.h
src/test/regress/expected/groupingsets.out
src/test/regress/sql/groupingsets.sql

index b8d622d88ca67e9ce2e83785f69e9a7035b14a92..ce2c03daefde18a3a45690a25a726abc94b1c567 100644 (file)
@@ -1744,9 +1744,10 @@ grouping_planner(PlannerInfo *root, double tuple_fraction,
                        sort_input_target = linitial_node(PathTarget, sort_input_targets);
                        Assert(!linitial_int(sort_input_targets_contain_srfs));
                        /* likewise for grouping_target vs. scanjoin_target */
-                       split_pathtarget_at_srfs(root, grouping_target, scanjoin_target,
-                                                                        &grouping_targets,
-                                                                        &grouping_targets_contain_srfs);
+                       split_pathtarget_at_srfs_grouping(root,
+                                                                                         grouping_target, scanjoin_target,
+                                                                                         &grouping_targets,
+                                                                                         &grouping_targets_contain_srfs);
                        grouping_target = linitial_node(PathTarget, grouping_targets);
                        Assert(!linitial_int(grouping_targets_contain_srfs));
                        /* scanjoin_target will not have any SRFs precomputed for it */
index d2b4ecc5e5131f5507112b141fd7c08e3de75d12..8ee565a9b132542e9c8ad8de98388d1a76ea021b 100644 (file)
@@ -19,6 +19,7 @@
 #include "optimizer/cost.h"
 #include "optimizer/optimizer.h"
 #include "optimizer/tlist.h"
+#include "rewrite/rewriteManip.h"
 
 
 /*
@@ -45,6 +46,8 @@ typedef struct
 
 typedef struct
 {
+       PlannerInfo *root;
+       bool            is_grouping_target; /* true if processing grouping target */
        /* This is a List of bare expressions: */
        List       *input_target_exprs; /* exprs available from input */
        /* These are Lists of Lists of split_pathtarget_items: */
@@ -59,6 +62,12 @@ typedef struct
        Index           current_sgref;  /* current subexpr's sortgroupref, or 0 */
 } split_pathtarget_context;
 
+static void split_pathtarget_at_srfs_extended(PlannerInfo *root,
+                                                                                         PathTarget *target,
+                                                                                         PathTarget *input_target,
+                                                                                         List **targets,
+                                                                                         List **targets_contain_srfs,
+                                                                                         bool is_grouping_target);
 static bool split_pathtarget_walker(Node *node,
                                                                        split_pathtarget_context *context);
 static void add_sp_item_to_pathtarget(PathTarget *target,
@@ -822,6 +831,51 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
 
 /*
  * split_pathtarget_at_srfs
+ *             Split given PathTarget into multiple levels to position SRFs safely,
+ *             performing exact matching against input_target.
+ *
+ * This is a wrapper for split_pathtarget_at_srfs_extended() that is used when
+ * both targets are on the same side of the grouping boundary (i.e., both are
+ * pre-grouping or both are post-grouping).  In this case, no special handling
+ * for the grouping nulling bit is required.
+ *
+ * See split_pathtarget_at_srfs_extended() for more details.
+ */
+void
+split_pathtarget_at_srfs(PlannerInfo *root,
+                                                PathTarget *target, PathTarget *input_target,
+                                                List **targets, List **targets_contain_srfs)
+{
+       split_pathtarget_at_srfs_extended(root, target, input_target,
+                                                                         targets, targets_contain_srfs,
+                                                                         false);
+}
+
+/*
+ * split_pathtarget_at_srfs_grouping
+ *             Split given PathTarget into multiple levels to position SRFs safely,
+ *             ignoring the grouping nulling bit when matching against input_target.
+ *
+ * This variant is used when the targets cross the grouping boundary (i.e.,
+ * target is post-grouping while input_target is pre-grouping).  In this case,
+ * we need to ignore the grouping nulling bit when checking for expression
+ * availability to avoid incorrectly re-evaluating SRFs that have already been
+ * computed in input_target.
+ *
+ * See split_pathtarget_at_srfs_extended() for more details.
+ */
+void
+split_pathtarget_at_srfs_grouping(PlannerInfo *root,
+                                                                 PathTarget *target, PathTarget *input_target,
+                                                                 List **targets, List **targets_contain_srfs)
+{
+       split_pathtarget_at_srfs_extended(root, target, input_target,
+                                                                         targets, targets_contain_srfs,
+                                                                         true);
+}
+
+/*
+ * split_pathtarget_at_srfs_extended
  *             Split given PathTarget into multiple levels to position SRFs safely
  *
  * The executor can only handle set-returning functions that appear at the
@@ -860,6 +914,13 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
  * already meant as a reference to a lower subexpression).  So, don't expand
  * any tlist expressions that appear in input_target, if that's not NULL.
  *
+ * This check requires extra care when processing the grouping target
+ * (indicated by the is_grouping_target flag).  In this case input_target is
+ * pre-grouping while target is post-grouping, so the latter may carry
+ * nullingrels bits from the grouping step that are absent in the former.  We
+ * must ignore those bits to correctly recognize that the tlist expressions are
+ * available in input_target.
+ *
  * It's also important that we preserve any sortgroupref annotation appearing
  * in the given target, especially on expressions matching input_target items.
  *
@@ -877,10 +938,11 @@ apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target)
  * are only a few possible patterns for which levels contain SRFs.
  * But this representation decouples callers from that knowledge.
  */
-void
-split_pathtarget_at_srfs(PlannerInfo *root,
-                                                PathTarget *target, PathTarget *input_target,
-                                                List **targets, List **targets_contain_srfs)
+static void
+split_pathtarget_at_srfs_extended(PlannerInfo *root,
+                                                                 PathTarget *target, PathTarget *input_target,
+                                                                 List **targets, List **targets_contain_srfs,
+                                                                 bool is_grouping_target)
 {
        split_pathtarget_context context;
        int                     max_depth;
@@ -905,7 +967,12 @@ split_pathtarget_at_srfs(PlannerInfo *root,
                return;
        }
 
-       /* Pass any input_target exprs down to split_pathtarget_walker() */
+       /*
+        * Pass 'root', the is_grouping_target flag, and any input_target exprs
+        * down to split_pathtarget_walker().
+        */
+       context.root = root;
+       context.is_grouping_target = is_grouping_target;
        context.input_target_exprs = input_target ? input_target->exprs : NIL;
 
        /*
@@ -1076,9 +1143,27 @@ split_pathtarget_at_srfs(PlannerInfo *root,
 static bool
 split_pathtarget_walker(Node *node, split_pathtarget_context *context)
 {
+       Node       *sanitized_node = node;
+
        if (node == NULL)
                return false;
 
+       /*
+        * If we are crossing the grouping boundary (post-grouping target vs
+        * pre-grouping input_target), we must ignore the grouping nulling bit to
+        * correctly check if the subexpression is available in input_target. This
+        * aligns with the matching logic in set_upper_references().
+        */
+       if (context->is_grouping_target &&
+               context->root->parse->hasGroupRTE &&
+               context->root->parse->groupingSets != NIL)
+       {
+               sanitized_node =
+                       remove_nulling_relids(node,
+                                                                 bms_make_singleton(context->root->group_rtindex),
+                                                                 NULL);
+       }
+
        /*
         * A subexpression that matches an expression already computed in
         * input_target can be treated like a Var (which indeed it will be after
@@ -1087,7 +1172,7 @@ split_pathtarget_walker(Node *node, split_pathtarget_context *context)
         * substructure.  (Note in particular that this preserves the identity of
         * any expressions that appear as sortgrouprefs in input_target.)
         */
-       if (list_member(context->input_target_exprs, node))
+       if (list_member(context->input_target_exprs, sanitized_node))
        {
                split_pathtarget_item *item = palloc(sizeof(split_pathtarget_item));
 
index 9c355bf11794cfe4f236d8383409a5297f699923..8d2c1ec08ba3ffd6bfff41787bb8e3bba4e79f28 100644 (file)
@@ -48,6 +48,11 @@ extern void apply_pathtarget_labeling_to_tlist(List *tlist, PathTarget *target);
 extern void split_pathtarget_at_srfs(PlannerInfo *root,
                                                                         PathTarget *target, PathTarget *input_target,
                                                                         List **targets, List **targets_contain_srfs);
+extern void split_pathtarget_at_srfs_grouping(PlannerInfo *root,
+                                                                                         PathTarget *target,
+                                                                                         PathTarget *input_target,
+                                                                                         List **targets,
+                                                                                         List **targets_contain_srfs);
 
 /* Convenience macro to get a PathTarget with valid cost/width fields */
 #define create_pathtarget(root, tlist) \
index 4c9b439f1c78d3c849e1d2023f1a4ea8ab01546c..3b908fb3bf6c0cefa55bbc61fc37f6e9b25c6d55 100644 (file)
@@ -2558,4 +2558,71 @@ group by grouping sets((a, b), (a));
  2 | 2 |          4
 (4 rows)
 
+-- test handling of SRFs with grouping sets
+explain (verbose, costs off)
+select generate_series(1, a) as g
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(g)
+order by 1;
+                          QUERY PLAN                          
+--------------------------------------------------------------
+ Sort
+   Output: (generate_series(1, "*VALUES*".column1))
+   Sort Key: (generate_series(1, "*VALUES*".column1))
+   ->  MixedAggregate
+         Output: (generate_series(1, "*VALUES*".column1))
+         Hash Key: generate_series(1, "*VALUES*".column1)
+         Group Key: ()
+         ->  ProjectSet
+               Output: generate_series(1, "*VALUES*".column1)
+               ->  Values Scan on "*VALUES*"
+                     Output: "*VALUES*".column1
+(11 rows)
+
+select generate_series(1, a) as g
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(g)
+order by 1;
+ g 
+---
+ 1
+ 2
+  
+(3 rows)
+
+explain (verbose, costs off)
+select generate_series(1, a) as g, a+b as ab
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(a, ab)
+order by 1, 2;
+                                                       QUERY PLAN                                                        
+-------------------------------------------------------------------------------------------------------------------------
+ Sort
+   Output: (generate_series(1, "*VALUES*".column1)), (("*VALUES*".column1 + "*VALUES*".column2)), "*VALUES*".column1
+   Sort Key: (generate_series(1, "*VALUES*".column1)), (("*VALUES*".column1 + "*VALUES*".column2))
+   ->  ProjectSet
+         Output: generate_series(1, "*VALUES*".column1), (("*VALUES*".column1 + "*VALUES*".column2)), "*VALUES*".column1
+         ->  MixedAggregate
+               Output: "*VALUES*".column1, (("*VALUES*".column1 + "*VALUES*".column2))
+               Hash Key: "*VALUES*".column1, ("*VALUES*".column1 + "*VALUES*".column2)
+               Hash Key: "*VALUES*".column1
+               Group Key: ()
+               ->  Values Scan on "*VALUES*"
+                     Output: ("*VALUES*".column1 + "*VALUES*".column2), "*VALUES*".column1
+(12 rows)
+
+select generate_series(1, a) as g, a+b as ab
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(a, ab)
+order by 1, 2;
+ g | ab 
+---+----
+ 1 |  2
+ 1 |  4
+ 1 |   
+ 1 |   
+ 2 |  4
+ 2 |   
+(6 rows)
+
 -- end
index 6d875475fae1eb2a4e7ca8834737860d5bf98951..3e010961fab1e03c661fde3dc742890a03fcf5a7 100644 (file)
@@ -721,4 +721,27 @@ select a, b, row_number() over (order by a, b nulls first)
 from (values (1, 1), (2, 2)) as t (a, b) where a = b
 group by grouping sets((a, b), (a));
 
+-- test handling of SRFs with grouping sets
+explain (verbose, costs off)
+select generate_series(1, a) as g
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(g)
+order by 1;
+
+select generate_series(1, a) as g
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(g)
+order by 1;
+
+explain (verbose, costs off)
+select generate_series(1, a) as g, a+b as ab
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(a, ab)
+order by 1, 2;
+
+select generate_series(1, a) as g, a+b as ab
+from (values (1, 1), (2, 2)) as t (a, b)
+group by rollup(a, ab)
+order by 1, 2;
+
 -- end