pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #19007: Planner fails to choose partial index with spurious 'not null'
10+ messages / 4 participants
[nested] [flat]
* BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-03 16:15 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 10+ messages in thread
From: PG Bug reporting form @ 2025-08-03 16:15 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: bryfox@gmail.com
The following bug has been logged on the website:
Bug reference: 19007
Logged by: Bryan Fox
Email address: bryfox@gmail.com
PostgreSQL version: 17.5
Operating system: Linux; macOS
Description:
I'm not sure if this is a bug, but the behavior was unexpected to me and
changed since v16. Documentation doesn't mention this as far as I can see.
This example has a partial index over one column where another column is not
null. The latter column is in fact 'not null' in the schema.
Prior to v17, this index would be used; in v17, the planner will choose a
sequential scan instead.
Of course, this setup is a little silly and easy to remedy. In reality, we
had a more complicated index and the column was nullable; later, someone
made a column 'not null'; later, we upgraded to v17. `last_idx_scan` did
make this easier to spot, though.
-- setup
create table example (id int, value float not null, flag bool not null);
insert into example
select generate_series(1, 100_000) id, random() value, true flag;
create index new_idx on example using btree (value) where flag is not null;
-- query
explain analyze
select * from example where value < 0.1 and flag is not null;
v17 plan:
QUERY PLAN
------------------------------------------------------------------------------------------------------------
Seq Scan on example (cost=0.00..1887.00 rows=33333 width=13) (actual
time=0.010..5.816 rows=9951 loops=1)
Filter: (value < '0.1'::double precision)
Rows Removed by Filter: 90049
v16 plan:
QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------
Bitmap Heap Scan on example (cost=625.34..1676.92 rows=33167 width=13)
(actual time=1.023..2.710 rows=9979 loops=1)
Recheck Cond: ((value < '0.1'::double precision) AND (flag IS NOT NULL))
Heap Blocks: exact=637
-> Bitmap Index Scan on new_idx (cost=0.00..617.04 rows=33167 width=0)
(actual time=0.936..0.937 rows=9979 loops=1)
Index Cond: (value < '0.1'::double precision)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-03 17:19 Tom Lane <tgl@sss.pgh.pa.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 2 replies; 10+ messages in thread
From: Tom Lane @ 2025-08-03 17:19 UTC (permalink / raw)
To: bryfox@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org, Richard Guo <guofenglinux@gmail.com>
PG Bug reporting form <noreply@postgresql.org> writes:
> I'm not sure if this is a bug, but the behavior was unexpected to me and
> changed since v16. Documentation doesn't mention this as far as I can see.
> This example has a partial index over one column where another column is not
> null. The latter column is in fact 'not null' in the schema.
> Prior to v17, this index would be used; in v17, the planner will choose a
> sequential scan instead.
Yeah. What is happening is that the query's "flag is not null"
condition is thrown away as redundant fairly early in planning,
and then it's not available to match to the index's predicate,
so the index doesn't appear to be applicable.
I had thought that e2debb643 would fix this in HEAD, since it moved
the throw-it-away behavior into eval_const_expressions which is also
applied to index predicates. Testing shows not, which feels like a
bug. It is not okay that eval_const_expressions' behavior varies
depending on context; we need it to act the same on index
expressions/predicates as on query expressions. Maybe we should
give up on trying to pre-apply eval_const_expressions to the
relcache's copies of the index expressions, and just do that part
during plancat's collection of the info?
(I'm also quite displeased that e2debb643 didn't nuke
restriction_is_always_true/false. But that's a separate discussion.)
regards, tom lane
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-04 04:28 David Rowley <dgrowleyml@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 10+ messages in thread
From: David Rowley @ 2025-08-04 04:28 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: bryfox@gmail.com, pgsql-bugs@lists.postgresql.org, Richard Guo <guofenglinux@gmail.com>
On Mon, 4 Aug 2025 at 05:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I had thought that e2debb643 would fix this in HEAD, since it moved
> the throw-it-away behavior into eval_const_expressions which is also
> applied to index predicates. Testing shows not, which feels like a
> bug. It is not okay that eval_const_expressions' behavior varies
> depending on context; we need it to act the same on index
> expressions/predicates as on query expressions. Maybe we should
> give up on trying to pre-apply eval_const_expressions to the
> relcache's copies of the index expressions, and just do that part
> during plancat's collection of the info?
I guess it doesn't because in RelationGetIndexPredicate(), the call to
eval_const_expressions() passes NULL for the PlannerInfo, resulting in
the NOT NULL information not being available during
eval_const_expressions().
Looks like we keep the indpred varnos as 1 during
RelationGetIndexPredicate() and only change them to the actual varno
of the relation after that call inside get_relation_info(), so it
wouldn't be valid to pass the PlannerInfo down without first rewriting
the varnos and it doesn't seem correct to rewrite the varnos in the
relcache code.
Seems like what it would take to make this work is *another* call to
eval_const_expressions() inside get_relation_info() just after the
varnos have been changed. The attached seems to work. However, it
seems a bit annoying to have to call eval_const_expressions() all over
again for such a rare case. I think it only really covers this rather
silly case, which Bryan seems well aware that the fix is to make the
index non-partial again.
David
Attachments:
[application/octet-stream] further_eval_const_expressions_processing_on_partial_indexes.patch (861B, ../../CAApHDvo7ytYDKAAE28EhcNXc4mLDq4oMtYk9MGsOtN9Ef3uugA@mail.gmail.com/2-further_eval_const_expressions_processing_on_partial_indexes.patch)
download | inline diff:
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index c6a58afc5e5..a80506a5879 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -435,6 +435,16 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
if (info->indpred && varno != 1)
ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+ info->indpred = (List *) eval_const_expressions(root, (Node *) info->indpred);
+ if (list_length(info->indpred) == 1 &&
+ IsA(linitial(info->indpred), Const))
+ {
+ Const *expr = (Const *) linitial(info->indpred);
+
+ if (!expr->constisnull && DatumGetBool(expr->constvalue))
+ info->indpred = NIL;
+ }
+
/* Build targetlist using the completed indexprs data */
info->indextlist = build_index_tlist(root, info, relation);
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-04 09:14 Richard Guo <guofenglinux@gmail.com>
parent: David Rowley <dgrowleyml@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2025-08-04 09:14 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
On Mon, Aug 4, 2025 at 1:29 PM David Rowley <dgrowleyml@gmail.com> wrote:
> I guess it doesn't because in RelationGetIndexPredicate(), the call to
> eval_const_expressions() passes NULL for the PlannerInfo, resulting in
> the NOT NULL information not being available during
> eval_const_expressions().
Exactly. The comment of eval_const_expressions also notes that when
"root" is NULL, NullTest quals will not be reduced.
* NOTE: "root" can be passed as NULL if the caller never wants to do any
* Param substitutions nor receive info about inlined functions nor reduce
* NullTest for Vars to constant true or constant false.
> Looks like we keep the indpred varnos as 1 during
> RelationGetIndexPredicate() and only change them to the actual varno
> of the relation after that call inside get_relation_info(), so it
> wouldn't be valid to pass the PlannerInfo down without first rewriting
> the varnos and it doesn't seem correct to rewrite the varnos in the
> relcache code.
Yeah. And in addition, the fact that we cache the pg_index.indpred in
the relcache entry means that we cannot rewrite the varnos within
RelationGetIndexPredicate().
> Seems like what it would take to make this work is *another* call to
> eval_const_expressions() inside get_relation_info() just after the
> varnos have been changed.
I wonder if we could move const-simplification and canonicalization of
index predicates out of RelationGetIndexPredicate() and into its
callers, after the varnos have been updated. That would require
changing every caller, though.
BTW, I wonder if we should also be concerned about index expressions,
in cases like:
create table t (a int, b int not null);
create index on t ((b is not null));
explain select * from t where b is not null;
Thanks
Richard
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-04 09:24 Richard Guo <guofenglinux@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 10+ messages in thread
From: Richard Guo @ 2025-08-04 09:24 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
On Mon, Aug 4, 2025 at 2:19 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> (I'm also quite displeased that e2debb643 didn't nuke
> restriction_is_always_true/false. But that's a separate discussion.)
Hmm, I tried to do that in e2debb643, but failed, as the commit
message explains:
Note that this patch does not get rid of restriction_is_always_true
and restriction_is_always_false. Removing them would prevent us from
reducing some IS [NOT] NULL quals that we were previously able to
reduce, because (a) the self-join elimination may introduce new IS NOT
NULL quals after constant folding, and (b) if some outer joins are
converted to inner joins, previously irreducible NullTest quals may
become reducible.
Thanks
Richard
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-20 09:37 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2025-08-20 09:37 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
I've had some time to look at this issue again. There are several
cases in plancat.c where specific expressions need to be run through
eval_const_expressions because the planner will be comparing them to
similarly-processed qual clauses. This includes constraint
expressions, statistics expressions, index expressions and index
predicates -- as highlighted by this bug report.
For a constraint expression, we run it through eval_const_expressions
before fixing its Vars to have the correct varno. This is problematic
because eval_const_expressions reduces NullTest quals based on varno,
so we must ensure the Vars have the correct varnos before calling it.
I think we can fix it by reordering the calls to eval_const_expressions
and ChangeVarNodes in get_relation_constraints().
For statistics expressions, we have the same issue as with constraint
expressions, plus we don't pass a valid root to eval_const_expressions.
I think we can fix this by adding "root" as a new parameter to
get_relation_statistics() and passing it to eval_const_expressions.
For index expressions and index predicates, however, the situation is
more complex, because we cache the constant-simplified expressions in
the relcache entry. This means we can't fix the Vars before applying
eval_const_expressions to them. One possible solution is to run
eval_const_expressions twice: first with "root" set to NULL, before
fixing the Vars, and then again with a valid "root" after the Vars
have been fixed. Please see the attached draft patch.
(This is basically what David's proposed patch does. However, I don't
think we should limit ourselves to the case where info->indpred
contains only a single reducible NullTest qual, since there may be
more than one.)
However, running eval_const_expressions twice for index expressions and
index predicates looks ugly, and one could argue that it might have a
performance impact.
BTW, all of these solutions are only possible after e2debb643, which
is only in master. In v17 and v18, we don't have a way to reduce
NullTest quals for these expressions, since that reduction doesn't
occur during const-simplification. I don't know what we should do for
v17 and v18.
Thoughts?
Thanks
Richard
Attachments:
[application/octet-stream] v1-0001-Fix-const-simplification-of-expressions-loaded-fr.patch (5.5K, ../../CAMbWs4-Vg+S1=b1TYQ2dWK3H=ZFiPjzSNh1YLdVDxsz=0RiE9g@mail.gmail.com/2-v1-0001-Fix-const-simplification-of-expressions-loaded-fr.patch)
download | inline diff:
From 2e0120ab571281ac69fdf20356ea5e0f2bc9a84d Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Wed, 20 Aug 2025 18:10:00 +0900
Subject: [PATCH v1] Fix const-simplification of expressions loaded from
catalogs
---
src/backend/optimizer/util/plancat.c | 58 +++++++++++++++++++---------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 6ce4efea118..c507d3ec8e6 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -77,7 +77,8 @@ static List *get_relation_constraints(PlannerInfo *root,
bool include_partition);
static List *build_index_tlist(PlannerInfo *root, IndexOptInfo *index,
Relation heapRelation);
-static List *get_relation_statistics(RelOptInfo *rel, Relation relation);
+static List *get_relation_statistics(PlannerInfo *root, RelOptInfo *rel,
+ Relation relation);
static void set_relation_partition_info(PlannerInfo *root, RelOptInfo *rel,
Relation relation);
static PartitionScheme find_partition_scheme(PlannerInfo *root,
@@ -435,6 +436,18 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
if (info->indpred && varno != 1)
ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+ /*
+ * We need to run the index expressions and predicate through
+ * const-simplification again after fixing the Vars to have the
+ * correct varno, and with a valid "root". This ensures that
+ * NullTest quals for Vars can be properly reduced.
+ */
+ info->indexprs = (List *)
+ eval_const_expressions(root, (Node *) info->indexprs);
+ info->indpred = (List *)
+ eval_const_expressions(root, (Node *) make_ands_explicit(info->indpred));
+ info->indpred = make_ands_implicit((Expr *) info->indpred);
+
/* Build targetlist using the completed indexprs data */
info->indextlist = build_index_tlist(root, info, relation);
@@ -508,7 +521,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
rel->indexlist = indexinfos;
- rel->statlist = get_relation_statistics(rel, relation);
+ rel->statlist = get_relation_statistics(root, rel, relation);
/* Grab foreign-table info using the relcache, while we have it */
if (relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
@@ -1406,6 +1419,14 @@ get_relation_constraints(PlannerInfo *root,
cexpr = stringToNode(constr->check[i].ccbin);
+ /*
+ * Fix Vars to have the desired varno. This must be done before
+ * const-simplification because eval_const_expressions reduces
+ * NullTest for Vars based on varno.
+ */
+ if (varno != 1)
+ ChangeVarNodes(cexpr, 1, varno, 0);
+
/*
* Run each expression through const-simplification and
* canonicalization. This is not just an optimization, but is
@@ -1420,10 +1441,6 @@ get_relation_constraints(PlannerInfo *root,
cexpr = (Node *) canonicalize_qual((Expr *) cexpr, true);
- /* Fix Vars to have the desired varno */
- if (varno != 1)
- ChangeVarNodes(cexpr, 1, varno, 0);
-
/*
* Finally, convert to implicit-AND format (that is, a List) and
* append the resulting item(s) to our output list.
@@ -1572,7 +1589,8 @@ get_relation_statistics_worker(List **stainfos, RelOptInfo *rel,
* just the identifying metadata. Only stats actually built are considered.
*/
static List *
-get_relation_statistics(RelOptInfo *rel, Relation relation)
+get_relation_statistics(PlannerInfo *root, RelOptInfo *rel,
+ Relation relation)
{
Index varno = rel->relid;
List *statoidlist;
@@ -1604,8 +1622,8 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
keys = bms_add_member(keys, staForm->stxkeys.values[i]);
/*
- * Preprocess expressions (if any). We read the expressions, run them
- * through eval_const_expressions, and fix the varnos.
+ * Preprocess expressions (if any). We read the expressions, fix the
+ * varnos, and run them through eval_const_expressions.
*
* XXX We don't know yet if there are any data for this stats object,
* with either stxdinherit value. But it's reasonable to assume there
@@ -1628,6 +1646,18 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
exprs = (List *) stringToNode(exprsString);
pfree(exprsString);
+ /*
+ * Modify the copies we obtain from the relcache to have the
+ * correct varno for the parent relation, so that they match
+ * up correctly against qual clauses.
+ *
+ * This must be done before const-simplification because
+ * eval_const_expressions reduces NullTest for Vars based on
+ * varno.
+ */
+ if (varno != 1)
+ ChangeVarNodes((Node *) exprs, 1, varno, 0);
+
/*
* Run the expressions through eval_const_expressions. This is
* not just an optimization, but is necessary, because the
@@ -1636,18 +1666,10 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
* We must not use canonicalize_qual, however, since these
* aren't qual expressions.
*/
- exprs = (List *) eval_const_expressions(NULL, (Node *) exprs);
+ exprs = (List *) eval_const_expressions(root, (Node *) exprs);
/* May as well fix opfuncids too */
fix_opfuncids((Node *) exprs);
-
- /*
- * Modify the copies we obtain from the relcache to have the
- * correct varno for the parent relation, so that they match
- * up correctly against qual clauses.
- */
- if (varno != 1)
- ChangeVarNodes((Node *) exprs, 1, varno, 0);
}
}
--
2.43.0
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-08-22 07:25 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2025-08-22 07:25 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
On Wed, Aug 20, 2025 at 6:37 PM Richard Guo <guofenglinux@gmail.com> wrote:
> I've had some time to look at this issue again. There are several
> cases in plancat.c where specific expressions need to be run through
> eval_const_expressions because the planner will be comparing them to
> similarly-processed qual clauses. This includes constraint
> expressions, statistics expressions, index expressions and index
> predicates -- as highlighted by this bug report.
I've split the patch into two. 0001 fixes const-simplification for
constraint expressions and statistics expressions by ensuring that
Vars are updated to have the correct varno before
const-simplification, and that a valid root is passed to
eval_const_expressions. 0002 fixes const-simplification for index
expressions and predicate by running eval_const_expressions a second
time after the Vars have been fixed and with a valid root.
I think 0001 is in pretty good shape. I'm concerned that in 0002 we
have to run eval_const_expressions twice. I once considered caching
the untransformed index expressions and predicate, and running
const-simplification outside of relcache.c. However, that doesn't
seem like a better solution, since we would need to re-run
const-simplification every time we fetch the index expressions and
predicate. Any suggestions?
Thanks
Richard
Attachments:
[application/octet-stream] v2-0001-Fix-const-simplification-for-constraints-and-stat.patch (7.9K, ../../CAMbWs48x=GsGan_bvQ+j0rLqYFNi9W725EmJpWUZv1RrmaicZQ@mail.gmail.com/2-v2-0001-Fix-const-simplification-for-constraints-and-stat.patch)
download | inline diff:
From 97c4fa04e7e8458271be5048fdd460ab6ec24dc5 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 21 Aug 2025 16:14:48 +0900
Subject: [PATCH v2 1/2] Fix const-simplification for constraints and stats
Constraint expressions and statistics expressions loaded from the
system catalogs need to be run through const-simplification, because
the planner will be comparing them to similarly-processed qual
clauses. Without this step, the planner may fail to detect valid
matches.
Currently, NullTest clauses in these expressions may not be reduced
correctly during const-simplification. This happens because their Var
nodes do not yet have the correct varno when eval_const_expressions is
applied. Since eval_const_expressions relies on varno to reduce
NullTest quals, incorrect varno can cause problems.
Additionally, for statistics expressions, eval_const_expressions is
called with root set to NULL, which also inhibits NullTest reduction.
This patch fixes the issue by ensuring that Vars are updated to have
the correct varno before const-simplification, and that a valid root
is passed to eval_const_expressions when needed.
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/19007-4cc6e252ed8aa54a@postgresql.org
---
src/backend/optimizer/util/plancat.c | 46 +++++++++++++++----------
src/test/regress/expected/predicate.out | 27 +++++++++++++++
src/test/regress/sql/predicate.sql | 20 +++++++++++
3 files changed, 75 insertions(+), 18 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 6ce4efea118..4536bdd6cb4 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -77,7 +77,8 @@ static List *get_relation_constraints(PlannerInfo *root,
bool include_partition);
static List *build_index_tlist(PlannerInfo *root, IndexOptInfo *index,
Relation heapRelation);
-static List *get_relation_statistics(RelOptInfo *rel, Relation relation);
+static List *get_relation_statistics(PlannerInfo *root, RelOptInfo *rel,
+ Relation relation);
static void set_relation_partition_info(PlannerInfo *root, RelOptInfo *rel,
Relation relation);
static PartitionScheme find_partition_scheme(PlannerInfo *root,
@@ -508,7 +509,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
rel->indexlist = indexinfos;
- rel->statlist = get_relation_statistics(rel, relation);
+ rel->statlist = get_relation_statistics(root, rel, relation);
/* Grab foreign-table info using the relcache, while we have it */
if (relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
@@ -1406,6 +1407,14 @@ get_relation_constraints(PlannerInfo *root,
cexpr = stringToNode(constr->check[i].ccbin);
+ /*
+ * Fix Vars to have the desired varno. This must be done before
+ * const-simplification because eval_const_expressions reduces
+ * NullTest for Vars based on varno.
+ */
+ if (varno != 1)
+ ChangeVarNodes(cexpr, 1, varno, 0);
+
/*
* Run each expression through const-simplification and
* canonicalization. This is not just an optimization, but is
@@ -1420,10 +1429,6 @@ get_relation_constraints(PlannerInfo *root,
cexpr = (Node *) canonicalize_qual((Expr *) cexpr, true);
- /* Fix Vars to have the desired varno */
- if (varno != 1)
- ChangeVarNodes(cexpr, 1, varno, 0);
-
/*
* Finally, convert to implicit-AND format (that is, a List) and
* append the resulting item(s) to our output list.
@@ -1572,7 +1577,8 @@ get_relation_statistics_worker(List **stainfos, RelOptInfo *rel,
* just the identifying metadata. Only stats actually built are considered.
*/
static List *
-get_relation_statistics(RelOptInfo *rel, Relation relation)
+get_relation_statistics(PlannerInfo *root, RelOptInfo *rel,
+ Relation relation)
{
Index varno = rel->relid;
List *statoidlist;
@@ -1604,8 +1610,8 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
keys = bms_add_member(keys, staForm->stxkeys.values[i]);
/*
- * Preprocess expressions (if any). We read the expressions, run them
- * through eval_const_expressions, and fix the varnos.
+ * Preprocess expressions (if any). We read the expressions, fix the
+ * varnos, and run them through eval_const_expressions.
*
* XXX We don't know yet if there are any data for this stats object,
* with either stxdinherit value. But it's reasonable to assume there
@@ -1628,6 +1634,18 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
exprs = (List *) stringToNode(exprsString);
pfree(exprsString);
+ /*
+ * Modify the copies we obtain from the relcache to have the
+ * correct varno for the parent relation, so that they match
+ * up correctly against qual clauses.
+ *
+ * This must be done before const-simplification because
+ * eval_const_expressions reduces NullTest for Vars based on
+ * varno.
+ */
+ if (varno != 1)
+ ChangeVarNodes((Node *) exprs, 1, varno, 0);
+
/*
* Run the expressions through eval_const_expressions. This is
* not just an optimization, but is necessary, because the
@@ -1636,18 +1654,10 @@ get_relation_statistics(RelOptInfo *rel, Relation relation)
* We must not use canonicalize_qual, however, since these
* aren't qual expressions.
*/
- exprs = (List *) eval_const_expressions(NULL, (Node *) exprs);
+ exprs = (List *) eval_const_expressions(root, (Node *) exprs);
/* May as well fix opfuncids too */
fix_opfuncids((Node *) exprs);
-
- /*
- * Modify the copies we obtain from the relcache to have the
- * correct varno for the parent relation, so that they match
- * up correctly against qual clauses.
- */
- if (varno != 1)
- ChangeVarNodes((Node *) exprs, 1, varno, 0);
}
}
diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out
index 59bfe33bb1c..1aff0b59ff8 100644
--- a/src/test/regress/expected/predicate.out
+++ b/src/test/regress/expected/predicate.out
@@ -409,3 +409,30 @@ SELECT * FROM pred_tab t1
DROP TABLE pred_tab;
DROP TABLE pred_tab_notnull;
+-- Validate that NullTest quals in constraint expressions are reduced correctly
+CREATE TABLE pred_tab1 (a int NOT NULL, b int,
+ CONSTRAINT check_tab1 CHECK (a IS NULL OR b > 2));
+CREATE TABLE pred_tab2 (a int, b int,
+ CONSTRAINT check_a CHECK (a IS NOT NULL));
+SET constraint_exclusion TO ON;
+-- Ensure that we get a dummy plan
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab1, pred_tab2 WHERE pred_tab2.a IS NULL;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+-- Ensure that we get a dummy plan
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
+ QUERY PLAN
+--------------------------
+ Result
+ One-Time Filter: false
+(2 rows)
+
+RESET constraint_exclusion;
+DROP TABLE pred_tab1;
+DROP TABLE pred_tab2;
diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql
index d92277353a0..32302d60b6d 100644
--- a/src/test/regress/sql/predicate.sql
+++ b/src/test/regress/sql/predicate.sql
@@ -201,3 +201,23 @@ SELECT * FROM pred_tab t1
DROP TABLE pred_tab;
DROP TABLE pred_tab_notnull;
+
+-- Validate that NullTest quals in constraint expressions are reduced correctly
+CREATE TABLE pred_tab1 (a int NOT NULL, b int,
+ CONSTRAINT check_tab1 CHECK (a IS NULL OR b > 2));
+CREATE TABLE pred_tab2 (a int, b int,
+ CONSTRAINT check_a CHECK (a IS NOT NULL));
+
+SET constraint_exclusion TO ON;
+
+-- Ensure that we get a dummy plan
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab1, pred_tab2 WHERE pred_tab2.a IS NULL;
+
+-- Ensure that we get a dummy plan
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
+
+RESET constraint_exclusion;
+DROP TABLE pred_tab1;
+DROP TABLE pred_tab2;
--
2.43.0
[application/octet-stream] v2-0002-Fix-const-simplification-for-index-expressions-an.patch (7.1K, ../../CAMbWs48x=GsGan_bvQ+j0rLqYFNi9W725EmJpWUZv1RrmaicZQ@mail.gmail.com/3-v2-0002-Fix-const-simplification-for-index-expressions-an.patch)
download | inline diff:
From 3a0060e61ce1000da062ddb055eb0f74955f1ed3 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 21 Aug 2025 18:48:20 +0900
Subject: [PATCH v2 2/2] Fix const-simplification for index expressions and
predicate
As with constraint expressions and statistics expressions, index
expressions and predicate also suffer from the issue where NullTest
clauses may not be reduced correctly during const-simplification.
Because we need to cache the const-simplified index expressions and
predicate in the relcache entry, we cannot fix the Vars before
applying eval_const_expressions. To ensure proper reduction of
NullTest clauses, this patch runs eval_const_expressions a second time
-- after the Vars have been fixed and with a valid root.
Bug: #19007
Reported-by: Bryan Fox <bryfox@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/19007-4cc6e252ed8aa54a@postgresql.org
---
src/backend/optimizer/util/plancat.c | 48 ++++++++++++++++++++-----
src/test/regress/expected/join.out | 2 +-
src/test/regress/expected/predicate.out | 24 +++++++++++++
src/test/regress/sql/join.sql | 2 +-
src/test/regress/sql/predicate.sql | 13 +++++++
5 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 4536bdd6cb4..9330c4c6306 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -428,13 +428,32 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
* modify the copies we obtain from the relcache to have the
* correct varno for the parent relation, so that they match up
* correctly against qual clauses.
+ *
+ * We need to run the index expressions and predicate through
+ * const-simplification again after fixing the Vars to have the
+ * correct varno, and with a valid "root". This ensures that
+ * NullTest quals for Vars can be properly reduced.
*/
info->indexprs = RelationGetIndexExpressions(indexRelation);
info->indpred = RelationGetIndexPredicate(indexRelation);
- if (info->indexprs && varno != 1)
- ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
- if (info->indpred && varno != 1)
- ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+ if (info->indexprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
+
+ info->indexprs = (List *)
+ eval_const_expressions(root, (Node *) info->indexprs);
+ }
+ if (info->indpred)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+
+ info->indpred = (List *)
+ eval_const_expressions(root,
+ (Node *) make_ands_explicit(info->indpred));
+ info->indpred = make_ands_implicit((Expr *) info->indpred);
+ }
/* Build targetlist using the completed indexprs data */
info->indextlist = build_index_tlist(root, info, relation);
@@ -966,8 +985,13 @@ infer_arbiter_indexes(PlannerInfo *root)
/* Expression attributes (if any) must match */
idxExprs = RelationGetIndexExpressions(idxRel);
- if (idxExprs && varno != 1)
- ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
+ if (idxExprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
+
+ idxExprs = (List *) eval_const_expressions(root, (Node *) idxExprs);
+ }
foreach(el, onconflict->arbiterElems)
{
@@ -1020,8 +1044,16 @@ infer_arbiter_indexes(PlannerInfo *root)
* CONFLICT's WHERE clause.
*/
predExprs = RelationGetIndexPredicate(idxRel);
- if (predExprs && varno != 1)
- ChangeVarNodes((Node *) predExprs, 1, varno, 0);
+ if (predExprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) predExprs, 1, varno, 0);
+
+ predExprs = (List *)
+ eval_const_expressions(root,
+ (Node *) make_ands_explicit(predExprs));
+ predExprs = make_ands_implicit((Expr *) predExprs);
+ }
if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false))
goto next;
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 98b05c94a11..90c8fbdb4f7 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -9355,7 +9355,7 @@ left join j2 on j1.id1 = j2.id1 where j1.id2 = 1;
Output: j2.id1, j2.id2
(8 rows)
-create unique index j1_id2_idx on j1(id2) where id2 is not null;
+create unique index j1_id2_idx on j1(id2) where id2 > 0;
-- ensure we don't use a partial unique index as unique proofs
explain (verbose, costs off)
select * from j1
diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out
index 1aff0b59ff8..c22016af171 100644
--- a/src/test/regress/expected/predicate.out
+++ b/src/test/regress/expected/predicate.out
@@ -436,3 +436,27 @@ SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
RESET constraint_exclusion;
DROP TABLE pred_tab1;
DROP TABLE pred_tab2;
+-- Validate that NullTest quals in index predicate are reduced correctly
+CREATE TABLE pred_tab (a int, b int NOT NULL, c int NOT NULL);
+INSERT INTO pred_tab SELECT i, i, i FROM generate_series(1, 1000) i;
+CREATE INDEX ON pred_tab (a) WHERE b IS NOT NULL AND c IS NOT NULL;
+ANALYZE pred_tab;
+-- Ensure that the partial index is usable
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+ QUERY PLAN
+---------------------------------------------
+ Index Scan using pred_tab_a_idx on pred_tab
+ Index Cond: (a < 5)
+(2 rows)
+
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+ a | b | c
+---+---+---
+ 1 | 1 | 1
+ 2 | 2 | 2
+ 3 | 3 | 3
+ 4 | 4 | 4
+(4 rows)
+
+DROP TABLE pred_tab;
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 5f0a475894d..0adb58455c8 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3509,7 +3509,7 @@ explain (verbose, costs off)
select * from j1
left join j2 on j1.id1 = j2.id1 where j1.id2 = 1;
-create unique index j1_id2_idx on j1(id2) where id2 is not null;
+create unique index j1_id2_idx on j1(id2) where id2 > 0;
-- ensure we don't use a partial unique index as unique proofs
explain (verbose, costs off)
diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql
index 32302d60b6d..4b776c6d09d 100644
--- a/src/test/regress/sql/predicate.sql
+++ b/src/test/regress/sql/predicate.sql
@@ -221,3 +221,16 @@ SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
RESET constraint_exclusion;
DROP TABLE pred_tab1;
DROP TABLE pred_tab2;
+
+-- Validate that NullTest quals in index predicate are reduced correctly
+CREATE TABLE pred_tab (a int, b int NOT NULL, c int NOT NULL);
+INSERT INTO pred_tab SELECT i, i, i FROM generate_series(1, 1000) i;
+CREATE INDEX ON pred_tab (a) WHERE b IS NOT NULL AND c IS NOT NULL;
+ANALYZE pred_tab;
+
+-- Ensure that the partial index is usable
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+
+DROP TABLE pred_tab;
--
2.43.0
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-09-01 06:00 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2025-09-01 06:00 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
On Fri, Aug 22, 2025 at 4:25 PM Richard Guo <guofenglinux@gmail.com> wrote:
> I think 0001 is in pretty good shape. I'm concerned that in 0002 we
> have to run eval_const_expressions twice. I once considered caching
> the untransformed index expressions and predicate, and running
> const-simplification outside of relcache.c. However, that doesn't
> seem like a better solution, since we would need to re-run
> const-simplification every time we fetch the index expressions and
> predicate. Any suggestions?
0001 has been pushed; here is a rebase of 0002.
I think the additional call to eval_const_expressions() is acceptable
performance-wise, because 1) it only runs when index expressions or
predicates are present, and 2) it's relatively cheap when run on small
expression trees, which is typically the case for index expressions
and predicates. In return, in cases such as the reported, it enables
the planner to match and use partial indexes, which can be a big win
in execution.
- Richard
Attachments:
[application/octet-stream] v3-0001-Fix-const-simplification-for-index-expressions-an.patch (7.3K, ../../CAMbWs4-o9CUDe2dERGxmq7D4Ha0bC=QGhZfAwyxomw_Me08KoQ@mail.gmail.com/2-v3-0001-Fix-const-simplification-for-index-expressions-an.patch)
download | inline diff:
From abcf1edfe2c2169e92ccef71c2d8030471827b25 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 21 Aug 2025 18:48:20 +0900
Subject: [PATCH v3] Fix const-simplification for index expressions and
predicate
Similar to the issue with constraint and statistics expressions fixed
in 317c117d6, index expressions and predicate can also suffer from
incorrect reduction of NullTest clauses during const-simplification,
due to unfixed varnos and the use of a NULL root. It has been
reported that this issue can cause the planner to fail to pick up a
partial index that it previously matched successfully.
Because we need to cache the const-simplified index expressions and
predicate in the relcache entry, we cannot fix the Vars before
applying eval_const_expressions. To ensure proper reduction of
NullTest clauses, this patch runs eval_const_expressions a second time
-- after the Vars have been fixed and with a valid root.
Bug: #19007
Reported-by: Bryan Fox <bryfox@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/19007-4cc6e252ed8aa54a@postgresql.org
---
src/backend/optimizer/util/plancat.c | 48 ++++++++++++++++++++-----
src/test/regress/expected/join.out | 2 +-
src/test/regress/expected/predicate.out | 24 +++++++++++++
src/test/regress/sql/join.sql | 2 +-
src/test/regress/sql/predicate.sql | 13 +++++++
5 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 4536bdd6cb4..23d731ce5f8 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -428,13 +428,32 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
* modify the copies we obtain from the relcache to have the
* correct varno for the parent relation, so that they match up
* correctly against qual clauses.
+ *
+ * After fixing the varnos, we need to run the index expressions
+ * and predicate through const-simplification again, using a valid
+ * "root". This ensures that NullTest quals for Vars can be
+ * properly reduced.
*/
info->indexprs = RelationGetIndexExpressions(indexRelation);
info->indpred = RelationGetIndexPredicate(indexRelation);
- if (info->indexprs && varno != 1)
- ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
- if (info->indpred && varno != 1)
- ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+ if (info->indexprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
+
+ info->indexprs = (List *)
+ eval_const_expressions(root, (Node *) info->indexprs);
+ }
+ if (info->indpred)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+
+ info->indpred = (List *)
+ eval_const_expressions(root,
+ (Node *) make_ands_explicit(info->indpred));
+ info->indpred = make_ands_implicit((Expr *) info->indpred);
+ }
/* Build targetlist using the completed indexprs data */
info->indextlist = build_index_tlist(root, info, relation);
@@ -966,8 +985,13 @@ infer_arbiter_indexes(PlannerInfo *root)
/* Expression attributes (if any) must match */
idxExprs = RelationGetIndexExpressions(idxRel);
- if (idxExprs && varno != 1)
- ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
+ if (idxExprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
+
+ idxExprs = (List *) eval_const_expressions(root, (Node *) idxExprs);
+ }
foreach(el, onconflict->arbiterElems)
{
@@ -1020,8 +1044,16 @@ infer_arbiter_indexes(PlannerInfo *root)
* CONFLICT's WHERE clause.
*/
predExprs = RelationGetIndexPredicate(idxRel);
- if (predExprs && varno != 1)
- ChangeVarNodes((Node *) predExprs, 1, varno, 0);
+ if (predExprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) predExprs, 1, varno, 0);
+
+ predExprs = (List *)
+ eval_const_expressions(root,
+ (Node *) make_ands_explicit(predExprs));
+ predExprs = make_ands_implicit((Expr *) predExprs);
+ }
if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false))
goto next;
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 04079268b98..084aa3bb67d 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -9495,7 +9495,7 @@ left join j2 on j1.id1 = j2.id1 where j1.id2 = 1;
Output: j2.id1, j2.id2
(8 rows)
-create unique index j1_id2_idx on j1(id2) where id2 is not null;
+create unique index j1_id2_idx on j1(id2) where id2 > 0;
-- ensure we don't use a partial unique index as unique proofs
explain (verbose, costs off)
select * from j1
diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out
index 1aff0b59ff8..c22016af171 100644
--- a/src/test/regress/expected/predicate.out
+++ b/src/test/regress/expected/predicate.out
@@ -436,3 +436,27 @@ SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
RESET constraint_exclusion;
DROP TABLE pred_tab1;
DROP TABLE pred_tab2;
+-- Validate that NullTest quals in index predicate are reduced correctly
+CREATE TABLE pred_tab (a int, b int NOT NULL, c int NOT NULL);
+INSERT INTO pred_tab SELECT i, i, i FROM generate_series(1, 1000) i;
+CREATE INDEX ON pred_tab (a) WHERE b IS NOT NULL AND c IS NOT NULL;
+ANALYZE pred_tab;
+-- Ensure that the partial index is usable
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+ QUERY PLAN
+---------------------------------------------
+ Index Scan using pred_tab_a_idx on pred_tab
+ Index Cond: (a < 5)
+(2 rows)
+
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+ a | b | c
+---+---+---
+ 1 | 1 | 1
+ 2 | 2 | 2
+ 3 | 3 | 3
+ 4 | 4 | 4
+(4 rows)
+
+DROP TABLE pred_tab;
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index b1732453e8d..82d9ee473a9 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3579,7 +3579,7 @@ explain (verbose, costs off)
select * from j1
left join j2 on j1.id1 = j2.id1 where j1.id2 = 1;
-create unique index j1_id2_idx on j1(id2) where id2 is not null;
+create unique index j1_id2_idx on j1(id2) where id2 > 0;
-- ensure we don't use a partial unique index as unique proofs
explain (verbose, costs off)
diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql
index 32302d60b6d..4b776c6d09d 100644
--- a/src/test/regress/sql/predicate.sql
+++ b/src/test/regress/sql/predicate.sql
@@ -221,3 +221,16 @@ SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
RESET constraint_exclusion;
DROP TABLE pred_tab1;
DROP TABLE pred_tab2;
+
+-- Validate that NullTest quals in index predicate are reduced correctly
+CREATE TABLE pred_tab (a int, b int NOT NULL, c int NOT NULL);
+INSERT INTO pred_tab SELECT i, i, i FROM generate_series(1, 1000) i;
+CREATE INDEX ON pred_tab (a) WHERE b IS NOT NULL AND c IS NOT NULL;
+ANALYZE pred_tab;
+
+-- Ensure that the partial index is usable
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+SELECT * FROM pred_tab WHERE a < 5 AND b IS NOT NULL AND c IS NOT NULL;
+
+DROP TABLE pred_tab;
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-09-26 09:20 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Richard Guo @ 2025-09-26 09:20 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
On Mon, Sep 1, 2025 at 3:00 PM Richard Guo <guofenglinux@gmail.com> wrote:
> 0001 has been pushed; here is a rebase of 0002.
>
> I think the additional call to eval_const_expressions() is acceptable
> performance-wise, because 1) it only runs when index expressions or
> predicates are present, and 2) it's relatively cheap when run on small
> expression trees, which is typically the case for index expressions
> and predicates. In return, in cases such as the reported, it enables
> the planner to match and use partial indexes, which can be a big win
> in execution.
Here is an updated version of the patch. In the commit message, I've
explained why I think the additional call to eval_const_expressions is
not a performance concern. I've also added a test case to validate
that NullTest quals in index expressions can now be reduced.
One concern I have is that this fix can only be applied to master;
there's no way to backpatch it to v17 or v18. This means that the
issue won't exist in v16 or earlier, nor in v19 and later, but it will
persist in v17 and v18. I have no idea what we should do for v17 and
v18.
- Richard
Attachments:
[application/octet-stream] v4-0001-Fix-const-simplification-for-index-expressions-an.patch (8.9K, ../../CAMbWs49mcpD7kX_MaC3zgWs2R-71FQwG-Beo99OJHQQ5rBo7dA@mail.gmail.com/2-v4-0001-Fix-const-simplification-for-index-expressions-an.patch)
download | inline diff:
From 3a40cb2a7eaf2d0f3e8a9294fad35a54f245d960 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 21 Aug 2025 18:48:20 +0900
Subject: [PATCH v4] Fix const-simplification for index expressions and
predicate
Similar to the issue with constraint and statistics expressions fixed
in 317c117d6, index expressions and predicate can also suffer from
incorrect reduction of NullTest clauses during const-simplification,
due to unfixed varnos and the use of a NULL root. It has been
reported that this issue can cause the planner to fail to pick up a
partial index that it previously matched successfully.
Because we need to cache the const-simplified index expressions and
predicate in the relcache entry, we cannot fix the Vars before
applying eval_const_expressions. To ensure proper reduction of
NullTest clauses, this patch runs eval_const_expressions a second time
-- after the Vars have been fixed and with a valid root.
It could be argued that the additional call to eval_const_expressions
might increase planning time, but I don't think that's a concern. It
only runs when index expressions and predicate are present; it is
relatively cheap when run on small expression trees (which is
typically the case for index expressions and predicate), and it runs
on expressions that have already been const-simplified once, making
the second pass even cheaper. In return, in cases like the one
reported, it allows the planner to match and use partial indexes,
which can lead to significant execution-time improvements.
Bug: #19007
Reported-by: Bryan Fox <bryfox@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://postgr.es/m/19007-4cc6e252ed8aa54a@postgresql.org
---
src/backend/optimizer/util/plancat.c | 48 ++++++++++++++++++++-----
src/test/regress/expected/join.out | 2 +-
src/test/regress/expected/predicate.out | 41 +++++++++++++++++++++
src/test/regress/sql/join.sql | 2 +-
src/test/regress/sql/predicate.sql | 19 ++++++++++
5 files changed, 102 insertions(+), 10 deletions(-)
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index da5d901ec3c..c257e2a221f 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -429,13 +429,32 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
* modify the copies we obtain from the relcache to have the
* correct varno for the parent relation, so that they match up
* correctly against qual clauses.
+ *
+ * After fixing the varnos, we need to run the index expressions
+ * and predicate through const-simplification again, using a valid
+ * "root". This ensures that NullTest quals for Vars can be
+ * properly reduced.
*/
info->indexprs = RelationGetIndexExpressions(indexRelation);
info->indpred = RelationGetIndexPredicate(indexRelation);
- if (info->indexprs && varno != 1)
- ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
- if (info->indpred && varno != 1)
- ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+ if (info->indexprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) info->indexprs, 1, varno, 0);
+
+ info->indexprs = (List *)
+ eval_const_expressions(root, (Node *) info->indexprs);
+ }
+ if (info->indpred)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) info->indpred, 1, varno, 0);
+
+ info->indpred = (List *)
+ eval_const_expressions(root,
+ (Node *) make_ands_explicit(info->indpred));
+ info->indpred = make_ands_implicit((Expr *) info->indpred);
+ }
/* Build targetlist using the completed indexprs data */
info->indextlist = build_index_tlist(root, info, relation);
@@ -967,8 +986,13 @@ infer_arbiter_indexes(PlannerInfo *root)
/* Expression attributes (if any) must match */
idxExprs = RelationGetIndexExpressions(idxRel);
- if (idxExprs && varno != 1)
- ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
+ if (idxExprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
+
+ idxExprs = (List *) eval_const_expressions(root, (Node *) idxExprs);
+ }
foreach(el, onconflict->arbiterElems)
{
@@ -1021,8 +1045,16 @@ infer_arbiter_indexes(PlannerInfo *root)
* CONFLICT's WHERE clause.
*/
predExprs = RelationGetIndexPredicate(idxRel);
- if (predExprs && varno != 1)
- ChangeVarNodes((Node *) predExprs, 1, varno, 0);
+ if (predExprs)
+ {
+ if (varno != 1)
+ ChangeVarNodes((Node *) predExprs, 1, varno, 0);
+
+ predExprs = (List *)
+ eval_const_expressions(root,
+ (Node *) make_ands_explicit(predExprs));
+ predExprs = make_ands_implicit((Expr *) predExprs);
+ }
if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false))
goto next;
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index cd37f549b5a..b84964de6d2 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -9518,7 +9518,7 @@ left join j2 on j1.id1 = j2.id1 where j1.id2 = 1;
Output: j2.id1, j2.id2
(8 rows)
-create unique index j1_id2_idx on j1(id2) where id2 is not null;
+create unique index j1_id2_idx on j1(id2) where id2 > 0;
-- ensure we don't use a partial unique index as unique proofs
explain (verbose, costs off)
select * from j1
diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out
index 304b6868b90..b0923d84390 100644
--- a/src/test/regress/expected/predicate.out
+++ b/src/test/regress/expected/predicate.out
@@ -443,3 +443,44 @@ SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
RESET constraint_exclusion;
DROP TABLE pred_tab1;
DROP TABLE pred_tab2;
+-- Validate that NullTest quals in index expressions and predicate are reduced correctly
+CREATE TABLE pred_tab (a int, b int NOT NULL, c int NOT NULL);
+INSERT INTO pred_tab SELECT i, i, i FROM generate_series(1, 1000) i;
+CREATE INDEX pred_tab_exprs_idx ON pred_tab ((a < 5 AND b IS NOT NULL AND c IS NOT NULL));
+CREATE INDEX pred_tab_pred_idx ON pred_tab (a) WHERE b IS NOT NULL AND c IS NOT NULL;
+ANALYZE pred_tab;
+-- Ensure that index pred_tab_exprs_idx is used
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE (a < 5 AND b IS NOT NULL AND c IS NOT NULL) IS TRUE;
+ QUERY PLAN
+-------------------------------------------------
+ Index Scan using pred_tab_exprs_idx on pred_tab
+ Index Cond: ((a < 5) = true)
+(2 rows)
+
+SELECT * FROM pred_tab WHERE (a < 5 AND b IS NOT NULL AND c IS NOT NULL) IS TRUE;
+ a | b | c
+---+---+---
+ 1 | 1 | 1
+ 2 | 2 | 2
+ 3 | 3 | 3
+ 4 | 4 | 4
+(4 rows)
+
+-- Ensure that index pred_tab_pred_idx is used
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE a < 3 AND b IS NOT NULL AND c IS NOT NULL;
+ QUERY PLAN
+------------------------------------------------
+ Index Scan using pred_tab_pred_idx on pred_tab
+ Index Cond: (a < 3)
+(2 rows)
+
+SELECT * FROM pred_tab WHERE a < 3 AND b IS NOT NULL AND c IS NOT NULL;
+ a | b | c
+---+---+---
+ 1 | 1 | 1
+ 2 | 2 | 2
+(2 rows)
+
+DROP TABLE pred_tab;
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index b1732453e8d..82d9ee473a9 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -3579,7 +3579,7 @@ explain (verbose, costs off)
select * from j1
left join j2 on j1.id1 = j2.id1 where j1.id2 = 1;
-create unique index j1_id2_idx on j1(id2) where id2 is not null;
+create unique index j1_id2_idx on j1(id2) where id2 > 0;
-- ensure we don't use a partial unique index as unique proofs
explain (verbose, costs off)
diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql
index 32302d60b6d..7d4fda1bc18 100644
--- a/src/test/regress/sql/predicate.sql
+++ b/src/test/regress/sql/predicate.sql
@@ -221,3 +221,22 @@ SELECT * FROM pred_tab2, pred_tab1 WHERE pred_tab1.a IS NULL OR pred_tab1.b < 2;
RESET constraint_exclusion;
DROP TABLE pred_tab1;
DROP TABLE pred_tab2;
+
+-- Validate that NullTest quals in index expressions and predicate are reduced correctly
+CREATE TABLE pred_tab (a int, b int NOT NULL, c int NOT NULL);
+INSERT INTO pred_tab SELECT i, i, i FROM generate_series(1, 1000) i;
+CREATE INDEX pred_tab_exprs_idx ON pred_tab ((a < 5 AND b IS NOT NULL AND c IS NOT NULL));
+CREATE INDEX pred_tab_pred_idx ON pred_tab (a) WHERE b IS NOT NULL AND c IS NOT NULL;
+ANALYZE pred_tab;
+
+-- Ensure that index pred_tab_exprs_idx is used
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE (a < 5 AND b IS NOT NULL AND c IS NOT NULL) IS TRUE;
+SELECT * FROM pred_tab WHERE (a < 5 AND b IS NOT NULL AND c IS NOT NULL) IS TRUE;
+
+-- Ensure that index pred_tab_pred_idx is used
+EXPLAIN (COSTS OFF)
+SELECT * FROM pred_tab WHERE a < 3 AND b IS NOT NULL AND c IS NOT NULL;
+SELECT * FROM pred_tab WHERE a < 3 AND b IS NOT NULL AND c IS NOT NULL;
+
+DROP TABLE pred_tab;
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #19007: Planner fails to choose partial index with spurious 'not null'
@ 2025-12-10 02:11 Richard Guo <guofenglinux@gmail.com>
parent: Richard Guo <guofenglinux@gmail.com>
0 siblings, 0 replies; 10+ messages in thread
From: Richard Guo @ 2025-12-10 02:11 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; bryfox@gmail.com; pgsql-bugs@lists.postgresql.org
On Fri, Sep 26, 2025 at 6:20 PM Richard Guo <guofenglinux@gmail.com> wrote:
> Here is an updated version of the patch. In the commit message, I've
> explained why I think the additional call to eval_const_expressions is
> not a performance concern. I've also added a test case to validate
> that NullTest quals in index expressions can now be reduced.
>
> One concern I have is that this fix can only be applied to master;
> there's no way to backpatch it to v17 or v18. This means that the
> issue won't exist in v16 or earlier, nor in v19 and later, but it will
> persist in v17 and v18. I have no idea what we should do for v17 and
> v18.
I've pushed this fix to master. Still no idea what to do for v17 and
v18.
- Richard
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2025-12-10 02:11 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-08-03 16:15 BUG #19007: Planner fails to choose partial index with spurious 'not null' PG Bug reporting form <noreply@postgresql.org>
2025-08-03 17:19 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-08-04 04:28 ` David Rowley <dgrowleyml@gmail.com>
2025-08-04 09:14 ` Richard Guo <guofenglinux@gmail.com>
2025-08-20 09:37 ` Richard Guo <guofenglinux@gmail.com>
2025-08-22 07:25 ` Richard Guo <guofenglinux@gmail.com>
2025-09-01 06:00 ` Richard Guo <guofenglinux@gmail.com>
2025-09-26 09:20 ` Richard Guo <guofenglinux@gmail.com>
2025-12-10 02:11 ` Richard Guo <guofenglinux@gmail.com>
2025-08-04 09:24 ` Richard Guo <guofenglinux@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox