]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Strip PlaceHolderVars from index operands
authorRichard Guo <rguo@postgresql.org>
Mon, 29 Dec 2025 02:38:49 +0000 (11:38 +0900)
committerRichard Guo <rguo@postgresql.org>
Mon, 29 Dec 2025 02:43:06 +0000 (11:43 +0900)
When pulling up a subquery, we may need to wrap its targetlist items
in PlaceHolderVars to enforce separate identity or as a result of
outer joins.  However, this causes any upper-level WHERE clauses
referencing these outputs to contain PlaceHolderVars, which prevents
indxpath.c from recognizing that they could be matched to index
columns or index expressions, potentially affecting the planner's
ability to use indexes.

To fix, explicitly strip PlaceHolderVars from index operands.  A
PlaceHolderVar appearing in a relation-scan-level expression is
effectively a no-op.  Nevertheless, to play it safe, we strip only
PlaceHolderVars that are not marked nullable.

The stripping is performed recursively to handle cases where
PlaceHolderVars are nested or interleaved with other node types.  To
minimize performance impact, we first use a lightweight walker to
check for the presence of strippable PlaceHolderVars.  The expensive
mutator is invoked only if a candidate is found, avoiding unnecessary
memory allocation and tree copying in the common case where no
PlaceHolderVars are present.

Back-patch to v18.  Although this issue exists before that, changes in
this version made it common enough to notice.  Given the lack of field
reports for older versions, I am not back-patching further.

Reported-by: Haowu Ge <gehaowu@bitmoe.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/62af586c-c270-44f3-9c5e-02c81d537e3d.gehaowu@bitmoe.com
Backpatch-through: 18

src/backend/optimizer/path/indxpath.c
src/backend/optimizer/plan/createplan.c
src/include/optimizer/paths.h
src/test/regress/expected/groupingsets.out
src/test/regress/sql/groupingsets.sql

index fe027461fcdca182af651dec29480909dbcfa3d1..4fe8e5164c7623c6ee3cf724d66f9617a059ddbb 100644 (file)
@@ -196,6 +196,8 @@ static Expr *match_clause_to_ordering_op(IndexOptInfo *index,
 static bool ec_member_matches_indexcol(PlannerInfo *root, RelOptInfo *rel,
                                                                           EquivalenceClass *ec, EquivalenceMember *em,
                                                                           void *arg);
+static bool contain_strippable_phv_walker(Node *node, void *context);
+static Node *strip_phvs_in_index_operand_mutator(Node *node, void *context);
 
 
 /*
@@ -4415,12 +4417,23 @@ match_index_to_operand(Node *operand,
        int                     indkey;
 
        /*
-        * Ignore any RelabelType node above the operand.   This is needed to be
-        * able to apply indexscanning in binary-compatible-operator cases. Note:
-        * we can assume there is at most one RelabelType node;
-        * eval_const_expressions() will have simplified if more than one.
+        * Ignore any PlaceHolderVar node contained in the operand.  This is
+        * needed to be able to apply indexscanning in cases where the operand (or
+        * a subtree) has been wrapped in PlaceHolderVars to enforce separate
+        * identity or as a result of outer joins.
         */
-       if (operand && IsA(operand, RelabelType))
+       operand = strip_phvs_in_index_operand(operand);
+
+       /*
+        * Ignore any RelabelType node above the operand.  This is needed to be
+        * able to apply indexscanning in binary-compatible-operator cases.
+        *
+        * Note: we must handle nested RelabelType nodes here.  While
+        * eval_const_expressions() will have simplified them to at most one
+        * layer, our prior stripping of PlaceHolderVars may have brought separate
+        * RelabelTypes into adjacency.
+        */
+       while (operand && IsA(operand, RelabelType))
                operand = (Node *) ((RelabelType *) operand)->arg;
 
        indkey = index->indexkeys[indexcol];
@@ -4473,6 +4486,92 @@ match_index_to_operand(Node *operand,
        return false;
 }
 
+/*
+ * strip_phvs_in_index_operand
+ *       Strip PlaceHolderVar nodes from the given operand expression to
+ *       facilitate matching against an index's key.
+ *
+ * A PlaceHolderVar appearing in a relation-scan-level expression is
+ * effectively a no-op.  Nevertheless, to play it safe, we strip only
+ * PlaceHolderVars that are not marked nullable.
+ *
+ * The removal is performed recursively because PlaceHolderVars can be nested
+ * or interleaved with other node types.  We must peel back all layers to
+ * expose the base operand.
+ *
+ * As a performance optimization, we first use a lightweight walker to check
+ * for the presence of strippable PlaceHolderVars.  The expensive mutator is
+ * invoked only if a candidate is found, avoiding unnecessary memory allocation
+ * and tree copying in the common case where no PlaceHolderVars are present.
+ */
+Node *
+strip_phvs_in_index_operand(Node *operand)
+{
+       /* Don't mutate/copy if no target PHVs exist */
+       if (!contain_strippable_phv_walker(operand, NULL))
+               return operand;
+
+       return strip_phvs_in_index_operand_mutator(operand, NULL);
+}
+
+/*
+ * contain_strippable_phv_walker
+ *       Detect if there are any PlaceHolderVars in the tree that are candidates
+ *       for stripping.
+ *
+ * We identify a PlaceHolderVar as strippable only if its phnullingrels is
+ * empty.
+ */
+static bool
+contain_strippable_phv_walker(Node *node, void *context)
+{
+       if (node == NULL)
+               return false;
+
+       if (IsA(node, PlaceHolderVar))
+       {
+               PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+               if (bms_is_empty(phv->phnullingrels))
+                       return true;
+       }
+
+       return expression_tree_walker(node, contain_strippable_phv_walker,
+                                                                 context);
+}
+
+/*
+ * strip_phvs_in_index_operand_mutator
+ *       Recursively remove PlaceHolderVars in the tree that match the criteria.
+ *
+ * We strip a PlaceHolderVar only if its phnullingrels is empty, replacing it
+ * with its contained expression.
+ */
+static Node *
+strip_phvs_in_index_operand_mutator(Node *node, void *context)
+{
+       if (node == NULL)
+               return NULL;
+
+       if (IsA(node, PlaceHolderVar))
+       {
+               PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+               /* If matches the criteria, strip it */
+               if (bms_is_empty(phv->phnullingrels))
+               {
+                       /* Recurse on its contained expression */
+                       return strip_phvs_in_index_operand_mutator((Node *) phv->phexpr,
+                                                                                                          context);
+               }
+
+               /* Otherwise, keep this PHV but check its contained expression */
+       }
+
+       return expression_tree_mutator(node, strip_phvs_in_index_operand_mutator,
+                                                                  context);
+}
+
 /*
  * is_pseudo_constant_for_index()
  *       Test whether the given expression can be used as an indexscan
index d721471e2af00c6c53fe926864f0cc093d8a82af..06128a9f243e6390f1286831093fe3834e4293bd 100644 (file)
@@ -5256,7 +5256,8 @@ fix_indexqual_clause(PlannerInfo *root, IndexOptInfo *index, int indexcol,
  * equal to the index's attribute number (index column position).
  *
  * Most of the code here is just for sanity cross-checking that the given
- * expression actually matches the index column it's claimed to.
+ * expression actually matches the index column it's claimed to.  It should
+ * match the logic in match_index_to_operand().
  */
 static Node *
 fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol)
@@ -5265,14 +5266,19 @@ fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol)
        int                     pos;
        ListCell   *indexpr_item;
 
+       Assert(indexcol >= 0 && indexcol < index->ncolumns);
+
+       /*
+        * Remove any PlaceHolderVar wrapping of the indexkey
+        */
+       node = strip_phvs_in_index_operand(node);
+
        /*
         * Remove any binary-compatible relabeling of the indexkey
         */
-       if (IsA(node, RelabelType))
+       while (IsA(node, RelabelType))
                node = (Node *) ((RelabelType *) node)->arg;
 
-       Assert(indexcol >= 0 && indexcol < index->ncolumns);
-
        if (index->indexkeys[indexcol] != 0)
        {
                /* It's a simple index column */
index 8410531f2d640d4c3c236f18536d23c72d96e338..70380276c844a5342f72c5daf3ebb4f79cf0b08e 100644 (file)
@@ -80,6 +80,7 @@ extern bool indexcol_is_bool_constant_for_query(PlannerInfo *root,
                                                                                                int indexcol);
 extern bool match_index_to_operand(Node *operand, int indexcol,
                                                                   IndexOptInfo *index);
+extern Node *strip_phvs_in_index_operand(Node *operand);
 extern void check_index_predicates(PlannerInfo *root, RelOptInfo *rel);
 
 /*
index 3b908fb3bf6c0cefa55bbc61fc37f6e9b25c6d55..ca6f2207ab1c42f7c46064c8f0708dfbb9bdf125 100644 (file)
@@ -463,6 +463,101 @@ select x, y || 'y'
    | 3y
 (8 rows)
 
+-- check that operands wrapped in PlaceHolderVars are capable of index matching
+begin;
+set local enable_bitmapscan = off;
+explain (costs off)
+select x, y
+  from (select unique1 as x, unique2 as y from tenk1) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+                     QUERY PLAN                      
+-----------------------------------------------------
+ Sort
+   Sort Key: tenk1.unique1, tenk1.unique2
+   ->  GroupAggregate
+         Group Key: tenk1.unique1
+         Sort Key: tenk1.unique2
+           Group Key: tenk1.unique2
+         ->  Index Scan using tenk1_unique1 on tenk1
+               Index Cond: (unique1 = 1)
+(8 rows)
+
+select x, y
+  from (select unique1 as x, unique2 as y from tenk1) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+ x |  y   
+---+------
+ 1 |     
+   | 2838
+(2 rows)
+
+explain (costs off)
+select x, y
+  from (select unique1::oid as x, unique2 as y from tenk1) as t
+  where x::integer = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+                           QUERY PLAN                            
+-----------------------------------------------------------------
+ Sort
+   Sort Key: ((tenk1.unique1)::oid), tenk1.unique2
+   ->  GroupAggregate
+         Group Key: ((tenk1.unique1)::oid)
+         Sort Key: tenk1.unique2
+           Group Key: tenk1.unique2
+         ->  Sort
+               Sort Key: ((tenk1.unique1)::oid)
+               ->  Index Scan using tenk1_unique1 on tenk1
+                     Index Cond: (((unique1)::oid)::integer = 1)
+(10 rows)
+
+select x, y
+  from (select unique1::oid as x, unique2 as y from tenk1) as t
+  where x::integer = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+ x |  y   
+---+------
+ 1 |     
+   | 2838
+(2 rows)
+
+explain (costs off)
+select x, y
+  from (select t1.unique1 as x, t1.unique2 as y from tenk1 t1, tenk1 t2) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+                            QUERY PLAN                             
+-------------------------------------------------------------------
+ Sort
+   Sort Key: t1.unique1, t1.unique2
+   ->  GroupAggregate
+         Group Key: t1.unique1
+         Sort Key: t1.unique2
+           Group Key: t1.unique2
+         ->  Nested Loop
+               ->  Index Scan using tenk1_unique1 on tenk1 t1
+                     Index Cond: (unique1 = 1)
+               ->  Index Only Scan using tenk1_hundred on tenk1 t2
+(10 rows)
+
+select x, y
+  from (select t1.unique1 as x, t1.unique2 as y from tenk1 t1, tenk1 t2) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+ x |  y   
+---+------
+ 1 |     
+   | 2838
+(2 rows)
+
+rollback;
 -- check qual push-down rules for a subquery with grouping sets
 explain (verbose, costs off)
 select * from (
index 3e010961fab1e03c661fde3dc742890a03fcf5a7..826ac5f5dbfb4e34193d154ec7e9275cac491554 100644 (file)
@@ -183,6 +183,52 @@ select x, y || 'y'
   group by grouping sets (x, y)
   order by 1, 2;
 
+-- check that operands wrapped in PlaceHolderVars are capable of index matching
+begin;
+
+set local enable_bitmapscan = off;
+
+explain (costs off)
+select x, y
+  from (select unique1 as x, unique2 as y from tenk1) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+
+select x, y
+  from (select unique1 as x, unique2 as y from tenk1) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+
+explain (costs off)
+select x, y
+  from (select unique1::oid as x, unique2 as y from tenk1) as t
+  where x::integer = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+
+select x, y
+  from (select unique1::oid as x, unique2 as y from tenk1) as t
+  where x::integer = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+
+explain (costs off)
+select x, y
+  from (select t1.unique1 as x, t1.unique2 as y from tenk1 t1, tenk1 t2) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+
+select x, y
+  from (select t1.unique1 as x, t1.unique2 as y from tenk1 t1, tenk1 t2) as t
+  where x = 1
+  group by grouping sets (x, y)
+  order by 1, 2;
+
+rollback;
+
 -- check qual push-down rules for a subquery with grouping sets
 explain (verbose, costs off)
 select * from (