]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Strip PlaceHolderVars from partition pruning operands
authorRichard Guo <rguo@postgresql.org>
Thu, 9 Apr 2026 07:41:31 +0000 (16:41 +0900)
committerRichard Guo <rguo@postgresql.org>
Thu, 9 Apr 2026 07:41:31 +0000 (16:41 +0900)
When pulling up a subquery, its targetlist items may be wrapped in
PlaceHolderVars to enforce separate identity or as a result of outer
joins.  This causes any upper-level WHERE clauses referencing these
outputs to contain PlaceHolderVars, which prevents partprune.c from
recognizing that they match partition key columns, defeating partition
pruning.

To fix, strip PlaceHolderVars from operands before comparing them to
partition keys.  A PlaceHolderVar with empty phnullingrels appearing
in a relation-scan-level expression is effectively a no-op, so
stripping it is safe.  This parallels the existing treatment in
indxpath.c for index matching.

In passing, rename strip_phvs_in_index_operand() to strip_noop_phvs()
and move it from indxpath.c to placeholder.c, since it is now a
general-purpose utility used by both index matching and partition
pruning code.

Back-patch to v18.  Although this issue exists before that, changes in
that version made it common enough to notice.  Given the lack of field
reports for older versions, I am not back-patching further.  In the
v18 back-patch, strip_phvs_in_index_operand() is retained as a thin
wrapper around the new strip_noop_phvs() to avoid breaking third-party
extensions that may reference it.

Reported-by: Cándido Antonio Martínez Descalzo <candido@ninehq.com>
Diagnosed-by: David Rowley <dgrowleyml@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/CAH5YaUwVUWETTyVECTnhs7C=CVwi+uMSQH=cOkwAUqMdvXdwWA@mail.gmail.com
Backpatch-through: 18

src/backend/optimizer/path/indxpath.c
src/backend/optimizer/plan/createplan.c
src/backend/optimizer/util/placeholder.c
src/backend/partitioning/partprune.c
src/include/optimizer/paths.h
src/include/optimizer/placeholder.h
src/test/regress/expected/partition_prune.out
src/test/regress/sql/partition_prune.sql

index 67d9dc35f44ceb20a4b010da85bc9d3733eb78e6..430e06dcaaac25090823c4445af0f10a1aedf825 100644 (file)
@@ -30,6 +30,7 @@
 #include "optimizer/optimizer.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
+#include "optimizer/placeholder.h"
 #include "optimizer/prep.h"
 #include "optimizer/restrictinfo.h"
 #include "utils/lsyscache.h"
@@ -195,8 +196,6 @@ 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);
 
 
 /*
@@ -4363,7 +4362,7 @@ match_index_to_operand(Node *operand,
         * a subtree) has been wrapped in PlaceHolderVars to enforce separate
         * identity or as a result of outer joins.
         */
-       operand = strip_phvs_in_index_operand(operand);
+       operand = strip_noop_phvs(operand);
 
        /*
         * Ignore any RelabelType node above the operand.  This is needed to be
@@ -4427,92 +4426,6 @@ 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 c7bc41c30d7b03da982683b755d70327705041b9..de6a183da7996307cdd038e5c22c1f675b4b3ff3 100644 (file)
@@ -5117,7 +5117,7 @@ fix_indexqual_operand(Node *node, IndexOptInfo *index, int indexcol)
        /*
         * Remove any PlaceHolderVar wrapping of the indexkey
         */
-       node = strip_phvs_in_index_operand(node);
+       node = strip_noop_phvs(node);
 
        /*
         * Remove any binary-compatible relabeling of the indexkey
index e1706363c88fa17e2fc1c17d038fef0c3768df3e..dd9b11885af19f4a55d45c67ed5effdf316d4ea6 100644 (file)
@@ -35,6 +35,8 @@ static void find_placeholders_recurse(PlannerInfo *root, Node *jtnode);
 static void find_placeholders_in_expr(PlannerInfo *root, Node *expr);
 static bool contain_placeholder_references_walker(Node *node,
                                                                                                  contain_placeholder_references_context *context);
+static bool contain_noop_phv_walker(Node *node, void *context);
+static Node *strip_noop_phvs_mutator(Node *node, void *context);
 
 
 /*
@@ -585,3 +587,92 @@ get_placeholder_nulling_relids(PlannerInfo *root, PlaceHolderInfo *phinfo)
        result = bms_del_members(result, phinfo->ph_eval_at);
        return result;
 }
+
+/*
+ * strip_noop_phvs
+ *       Strip no-op PlaceHolderVar nodes from the given expression tree.
+ *
+ * A PlaceHolderVar that is not marked as nullable (i.e., its phnullingrels
+ * is empty) is effectively a no-op when it appears in a relation-scan-level
+ * expression.  This function strips such PlaceHolderVars, which is useful
+ * for matching expressions to index keys or partition keys in cases where
+ * the expression has been wrapped in PlaceHolderVars during subquery pullup.
+ *
+ * IMPORTANT: the caller must ensure that the expression is a scan-level
+ * expression, so that non-nullable PlaceHolderVars in it are indeed no-ops.
+ *
+ * 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 expression.
+ *
+ * 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_noop_phvs(Node *node)
+{
+       /* Don't mutate/copy if no target PHVs exist */
+       if (!contain_noop_phv_walker(node, NULL))
+               return node;
+
+       return strip_noop_phvs_mutator(node, NULL);
+}
+
+/*
+ * contain_noop_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_noop_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_noop_phv_walker,
+                                                                 context);
+}
+
+/*
+ * strip_noop_phvs_mutator
+ *       Recursively remove PlaceHolderVars that are not marked nullable.
+ *
+ * We strip a PlaceHolderVar only if its phnullingrels is empty, replacing it
+ * with its contained expression.
+ */
+static Node *
+strip_noop_phvs_mutator(Node *node, void *context)
+{
+       if (node == NULL)
+               return NULL;
+
+       if (IsA(node, PlaceHolderVar))
+       {
+               PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+               if (bms_is_empty(phv->phnullingrels))
+               {
+                       /* Recurse on its contained expression */
+                       return strip_noop_phvs_mutator((Node *) phv->phexpr,
+                                                                                  context);
+               }
+
+               /* Otherwise, keep this PHV but check its contained expression */
+       }
+
+       return expression_tree_mutator(node, strip_noop_phvs_mutator,
+                                                                  context);
+}
index 2901cd348a915cc274aead799c022cfb36efca61..e7c318bbcacc90ccd0f9f1b47586c477613db379 100644 (file)
@@ -49,6 +49,7 @@
 #include "optimizer/cost.h"
 #include "optimizer/optimizer.h"
 #include "optimizer/pathnode.h"
+#include "optimizer/placeholder.h"
 #include "parser/parsetree.h"
 #include "partitioning/partbounds.h"
 #include "partitioning/partprune.h"
@@ -1813,6 +1814,15 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
  *   and couldn't possibly match any other one either, due to its form or
  *   properties (such as containing a volatile function).
  *   Output arguments: none set.
+ *
+ * Note that when pulling up a subquery, the clause operands may get wrapped
+ * in PlaceHolderVars to enforce separate identity or as a result of outer
+ * joins.  We must strip such no-op PlaceHolderVars before comparing operands
+ * to the partition key, otherwise the equal() checks will fail to recognize
+ * valid matches.  This is safe because the clauses here are always
+ * relation-scan-level expressions, where a PlaceHolderVar with empty
+ * phnullingrels is effectively a no-op.  Stripping may also bring separate
+ * RelabelType nodes into adjacency, so we must loop when peeling those.
  */
 static PartClauseMatchStatus
 match_clause_to_partition_key(GeneratePruningStepsContext *context,
@@ -1928,10 +1938,12 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
                PartClauseInfo *partclause;
 
                leftop = (Expr *) get_leftop(clause);
-               if (IsA(leftop, RelabelType))
+               leftop = (Expr *) strip_noop_phvs((Node *) leftop);
+               while (IsA(leftop, RelabelType))
                        leftop = ((RelabelType *) leftop)->arg;
                rightop = (Expr *) get_rightop(clause);
-               if (IsA(rightop, RelabelType))
+               rightop = (Expr *) strip_noop_phvs((Node *) rightop);
+               while (IsA(rightop, RelabelType))
                        rightop = ((RelabelType *) rightop)->arg;
                opno = opclause->opno;
 
@@ -2179,7 +2191,8 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
                                   *elem_clauses;
                ListCell   *lc1;
 
-               if (IsA(leftop, RelabelType))
+               leftop = (Expr *) strip_noop_phvs((Node *) leftop);
+               while (IsA(leftop, RelabelType))
                        leftop = ((RelabelType *) leftop)->arg;
 
                /* check if the LHS matches this partition key */
@@ -2405,7 +2418,8 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
                NullTest   *nulltest = (NullTest *) clause;
                Expr       *arg = nulltest->arg;
 
-               if (IsA(arg, RelabelType))
+               arg = (Expr *) strip_noop_phvs((Node *) arg);
+               while (IsA(arg, RelabelType))
                        arg = ((RelabelType *) arg)->arg;
 
                /* Does arg match with this partition key column? */
@@ -3717,7 +3731,8 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, const Expr *partk
                BooleanTest *btest = (BooleanTest *) clause;
 
                leftop = btest->arg;
-               if (IsA(leftop, RelabelType))
+               leftop = (Expr *) strip_noop_phvs((Node *) leftop);
+               while (IsA(leftop, RelabelType))
                        leftop = ((RelabelType *) leftop)->arg;
 
                if (equal(leftop, partkey))
@@ -3754,7 +3769,8 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, const Expr *partk
 
                leftop = is_not_clause ? get_notclausearg(clause) : clause;
 
-               if (IsA(leftop, RelabelType))
+               leftop = (Expr *) strip_noop_phvs((Node *) leftop);
+               while (IsA(leftop, RelabelType))
                        leftop = ((RelabelType *) leftop)->arg;
 
                /* Compare to the partition key, and make up a clause ... */
index 8751ad7381cf0f73f4eac4221bfa33831f5858b6..17f2099ec3be4e745f06bee0d0199f46b36aaba1 100644 (file)
@@ -88,7 +88,6 @@ 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 28e4da645fb03b3ecc515e4b7a51ff8e5f26b78d..60798281090a6bc01a80b7252cdb63f255041c16 100644 (file)
@@ -32,5 +32,6 @@ extern bool contain_placeholder_references_to(PlannerInfo *root, Node *clause,
                                                                                          int relid);
 extern Relids get_placeholder_nulling_relids(PlannerInfo *root,
                                                                                         PlaceHolderInfo *phinfo);
+extern Node *strip_noop_phvs(Node *node);
 
 #endif                                                 /* PLACEHOLDER_H */
index deacdd7580702a53f407f3dcf3845dac331d0376..849049f9c51a4d5124967052253cf32cc2e15bf2 100644 (file)
@@ -4824,3 +4824,135 @@ select min(a) over (partition by a order by a) from part_abc where a >= stable_o
 
 drop view part_abc_view;
 drop table part_abc;
+--
+-- Check that operands wrapped in PlaceHolderVars are matched to partition
+-- keys, allowing partition pruning to occur.  PlaceHolderVars can be
+-- introduced when a subquery's output is used with grouping sets.
+--
+create table phv_part (a int, b text) partition by list (a);
+create table phv_part_1 partition of phv_part for values in (1);
+create table phv_part_2 partition of phv_part for values in (2);
+create table phv_part_null partition of phv_part for values in (null);
+insert into phv_part values (1, 'one'), (2, 'two'), (null, 'null');
+-- OpExpr: PHV-wrapped operand matched via equal()
+explain (costs off)
+select * from (select a, b from phv_part) t
+  where a = 1
+  group by grouping sets (a, b);
+              QUERY PLAN               
+---------------------------------------
+ MixedAggregate
+   Hash Key: phv_part.b
+   Group Key: phv_part.a
+   ->  Seq Scan on phv_part_1 phv_part
+         Filter: (a = 1)
+(5 rows)
+
+select * from (select a, b from phv_part) t
+  where a = 1
+  group by grouping sets (a, b);
+ a |  b  
+---+-----
+ 1 | 
+   | one
+(2 rows)
+
+-- OpExpr with RelabelType: PHV wrapped around a casted column
+explain (costs off)
+select * from (select a::oid as x, b from phv_part) t
+  where x::int = 1
+  group by grouping sets (x, b);
+                QUERY PLAN                 
+-------------------------------------------
+ HashAggregate
+   Hash Key: (phv_part.a)::oid
+   Hash Key: phv_part.b
+   ->  Seq Scan on phv_part_1 phv_part
+         Filter: (((a)::oid)::integer = 1)
+(5 rows)
+
+select * from (select a::oid as x, b from phv_part) t
+  where x::int = 1
+  group by grouping sets (x, b);
+ x |  b  
+---+-----
+ 1 | 
+   | one
+(2 rows)
+
+-- ScalarArrayOpExpr: IN clause with PHV-wrapped operand
+explain (costs off)
+select * from (select a, b from phv_part) t
+  where a in (1, null)
+  group by grouping sets (a, b);
+                    QUERY PLAN                     
+---------------------------------------------------
+ HashAggregate
+   Hash Key: phv_part.a
+   Hash Key: phv_part.b
+   ->  Seq Scan on phv_part_1 phv_part
+         Filter: (a = ANY ('{1,NULL}'::integer[]))
+(5 rows)
+
+select * from (select a, b from phv_part) t
+  where a in (1, null)
+  group by grouping sets (a, b);
+ a |  b  
+---+-----
+ 1 | 
+   | one
+(2 rows)
+
+-- NullTest: IS NULL with PHV-wrapped operand
+explain (costs off)
+select * from (select a, b from phv_part) t
+  where a is null
+  group by grouping sets (a, b);
+                QUERY PLAN                
+------------------------------------------
+ HashAggregate
+   Hash Key: phv_part.a
+   Hash Key: phv_part.b
+   ->  Seq Scan on phv_part_null phv_part
+         Filter: (a IS NULL)
+(5 rows)
+
+select * from (select a, b from phv_part) t
+  where a is null
+  group by grouping sets (a, b);
+ a |  b   
+---+------
+   | 
+   | null
+(2 rows)
+
+drop table phv_part;
+-- BooleanTest: IS TRUE with PHV-wrapped boolean partition key
+create table phv_boolpart (a bool, b text) partition by list (a);
+create table phv_boolpart_t partition of phv_boolpart for values in (true);
+create table phv_boolpart_f partition of phv_boolpart for values in (false);
+create table phv_boolpart_null partition of phv_boolpart default;
+insert into phv_boolpart values (true, 'yes'), (false, 'no'), (null, 'unknown');
+explain (costs off)
+select * from (select a, b from phv_boolpart) t
+  where a is true
+  group by grouping sets (a, b);
+                  QUERY PLAN                   
+-----------------------------------------------
+ HashAggregate
+   Hash Key: phv_boolpart.a
+   Hash Key: phv_boolpart.b
+   ->  Seq Scan on phv_boolpart_t phv_boolpart
+         Filter: (a IS TRUE)
+(5 rows)
+
+select * from (select a, b from phv_boolpart) t
+  where a is true
+  group by grouping sets (a, b);
+ a |  b  
+---+-----
+ t | 
+   | yes
+(2 rows)
+
+drop table phv_boolpart;
index d93c0c03babad11ef9f022a12ad9696e2b3143b2..359a92080562166085cdd879cef7d5d327e49c31 100644 (file)
@@ -1447,3 +1447,74 @@ select min(a) over (partition by a order by a) from part_abc where a >= stable_o
 
 drop view part_abc_view;
 drop table part_abc;
+
+--
+-- Check that operands wrapped in PlaceHolderVars are matched to partition
+-- keys, allowing partition pruning to occur.  PlaceHolderVars can be
+-- introduced when a subquery's output is used with grouping sets.
+--
+create table phv_part (a int, b text) partition by list (a);
+create table phv_part_1 partition of phv_part for values in (1);
+create table phv_part_2 partition of phv_part for values in (2);
+create table phv_part_null partition of phv_part for values in (null);
+insert into phv_part values (1, 'one'), (2, 'two'), (null, 'null');
+
+-- OpExpr: PHV-wrapped operand matched via equal()
+explain (costs off)
+select * from (select a, b from phv_part) t
+  where a = 1
+  group by grouping sets (a, b);
+
+select * from (select a, b from phv_part) t
+  where a = 1
+  group by grouping sets (a, b);
+
+-- OpExpr with RelabelType: PHV wrapped around a casted column
+explain (costs off)
+select * from (select a::oid as x, b from phv_part) t
+  where x::int = 1
+  group by grouping sets (x, b);
+
+select * from (select a::oid as x, b from phv_part) t
+  where x::int = 1
+  group by grouping sets (x, b);
+
+-- ScalarArrayOpExpr: IN clause with PHV-wrapped operand
+explain (costs off)
+select * from (select a, b from phv_part) t
+  where a in (1, null)
+  group by grouping sets (a, b);
+
+select * from (select a, b from phv_part) t
+  where a in (1, null)
+  group by grouping sets (a, b);
+
+-- NullTest: IS NULL with PHV-wrapped operand
+explain (costs off)
+select * from (select a, b from phv_part) t
+  where a is null
+  group by grouping sets (a, b);
+
+select * from (select a, b from phv_part) t
+  where a is null
+  group by grouping sets (a, b);
+
+drop table phv_part;
+
+-- BooleanTest: IS TRUE with PHV-wrapped boolean partition key
+create table phv_boolpart (a bool, b text) partition by list (a);
+create table phv_boolpart_t partition of phv_boolpart for values in (true);
+create table phv_boolpart_f partition of phv_boolpart for values in (false);
+create table phv_boolpart_null partition of phv_boolpart default;
+insert into phv_boolpart values (true, 'yes'), (false, 'no'), (null, 'unknown');
+
+explain (costs off)
+select * from (select a, b from phv_boolpart) t
+  where a is true
+  group by grouping sets (a, b);
+
+select * from (select a, b from phv_boolpart) t
+  where a is true
+  group by grouping sets (a, b);
+
+drop table phv_boolpart;