From 2deda1edcc717a531b79231ce83da67c9ad92db2 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 15 Jun 2023 15:20:15 +1200 Subject: [PATCH v1] Don't use partial unique indexes for unique proofs in the planner Here we adjust relation_has_unique_index_for() so that it no longer makes use of partial unique indexes as uniqueness proofs. It is incorrect to use these as the predicates used by check_index_predicates() to set predOK makes use of not only baserestrictinfo quals as proofs, but also qual from join conditions. For relation_has_unique_index_for()'s case, we need to know the relation is unique for a given set of columns before any joins are evaluated, so if predOK was only set to true due to some join qual, then it's unsafe to use such indexes in relation_has_unique_index_for() as the final plan may not even make use of that index, which could result in reading tuples that break the uniqueness proofs that the index was supposedly enforcing. We could maybe work a bit harder and add another field to IndexOptInfo to record if the predicate is ok by just using the given baserestrictinfos as proofs that the partial index can be used and make use of that field instead in relation_has_unique_index_for(). However, uniqueness proofs from such indexes are probably fairly rare. Bug: #17975 Reported-by: Tor Erik Linnerud Backpatch-through: 11, all supported versions Discussion: https://postgr.es/m/17975-98a90c156f25c952%40postgresql.org --- src/backend/optimizer/path/indxpath.c | 8 +++++--- src/backend/optimizer/plan/analyzejoins.c | 9 ++++----- src/test/regress/expected/join.out | 17 +++++++++++++++++ src/test/regress/sql/join.sql | 9 +++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 1436dbc2f2..a1dcc63b37 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -3549,10 +3549,12 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, /* * If the index is not unique, or not immediately enforced, or if it's - * a partial index that doesn't match the query, it's useless here. + * a partial index, it's useless here. We're unable to make use of predOK + * partial unique indexes due to the fact that check_index_predicates() also + * makes use of join predicates to determine if the partial index is usable. + * Here we need proofs that hold true before any joins are evaluated. */ - if (!ind->unique || !ind->immediate || - (ind->indpred != NIL && !ind->predOK)) + if (!ind->unique || !ind->immediate || ind->indpred != NIL) continue; /* diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 6476e55e56..cd16bba63e 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -733,9 +733,9 @@ rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel) /* * For a plain relation, we only know how to prove uniqueness by * reference to unique indexes. Make sure there's at least one - * suitable unique index. It must be immediately enforced, and if - * it's a partial index, it must match the query. (Keep these - * conditions in sync with relation_has_unique_index_for!) + * suitable unique index. It must be immediately enforced, and not a + * partial index. (Keep these conditions in sync with + * relation_has_unique_index_for!) */ ListCell *lc; @@ -743,8 +743,7 @@ rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel) { IndexOptInfo *ind = (IndexOptInfo *) lfirst(lc); - if (ind->unique && ind->immediate && - (ind->indpred == NIL || ind->predOK)) + if (ind->unique && ind->immediate && ind->indpred == NIL) return true; } } diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 98b2667821..b4d2ede4e8 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -7601,6 +7601,23 @@ 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; +-- ensure we don't use a partial unique index as unique proofs +explain (verbose, costs off) +select * from j1 +inner join j2 on j1.id2 = j2.id2; + QUERY PLAN +------------------------------------------ + Nested Loop + Output: j1.id1, j1.id2, j2.id1, j2.id2 + Join Filter: (j2.id2 = j1.id2) + -> Seq Scan on public.j2 + Output: j2.id1, j2.id2 + -> Seq Scan on public.j1 + Output: j1.id1, j1.id2 +(7 rows) + +drop index j1_id2_idx; -- validate logic in merge joins which skips mark and restore. -- it should only do this if all quals which were used to detect the unique -- are present as join quals, and not plain quals. diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 7daa390b1d..6191c05a92 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2734,6 +2734,15 @@ 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; + +-- ensure we don't use a partial unique index as unique proofs +explain (verbose, costs off) +select * from j1 +inner join j2 on j1.id2 = j2.id2; + +drop index j1_id2_idx; + -- validate logic in merge joins which skips mark and restore. -- it should only do this if all quals which were used to detect the unique -- are present as join quals, and not plain quals. -- 2.39.2