From 9a3bc2432458b144977f0ebf7b1591f101bbecd3 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Fri, 13 Nov 2026 11:13:21 +0900 Subject: [PATCH v1] Simplify the uniqueness proof for self-join elimination Self-join elimination can remove a join of a relation to itself if each row of one side can match only its own row of the other. That holds if the self-join quals, together with restriction clauses that both sides have, constrain all columns of a unique index. The code proved this by passing the self-join quals and all of the kept relation's restriction clauses through innerrel_is_unique(), having relation_has_unique_index_for() report which restriction clauses it used, and then checking that the removed relation has those too. This required an extra_clauses output parameter threaded through innerrel_is_unique_ext(), is_innerrel_unique_for(), rel_is_distinct_for() and relation_has_unique_index_for(), plus self_join and extra_clauses fields in the UniqueRelInfo cache entries, which the cache lookups then had to treat specially. It also made the outcome depend on the order of the relation's indexes. If the first unique index found relied on a restriction clause that only the kept relation has, the join was not removed, even when another unique index would have proven it removable. To fix, pick out the restriction clauses that both relations have before making the proof, and pass only those to relation_has_unique_index_for(), which now takes the restriction clauses to consider as a parameter instead of always using rel->baserestrictinfo. Self-join elimination no longer goes through innerrel_is_unique() or its cache, so the extra_clauses parameters go away, and unique_for_rels reverts to a list of Relids, making UniqueRelInfo unnecessary. Also, stop requiring a join clause between the two relations. Shared restriction clauses can prove a join removable on their own, as in "WHERE s1.a = 1 AND s2.a = 1" with a unique index on a. Without this, removing joins in a different order than before could leave two relations related only through such constants, keeping a join that we used to remove. --- src/backend/optimizer/path/indxpath.c | 39 +---- src/backend/optimizer/plan/analyzejoins.c | 176 ++++++---------------- src/include/nodes/pathnodes.h | 35 +---- src/include/optimizer/paths.h | 2 +- src/test/regress/expected/join.out | 28 ++-- src/test/regress/sql/join.sql | 6 +- src/tools/pgindent/typedefs.list | 1 - 7 files changed, 82 insertions(+), 205 deletions(-) diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 3f5d4fa3182..28099e07116 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -4133,16 +4133,15 @@ ec_member_matches_indexcol(PlannerInfo *root, RelOptInfo *rel, * outer_is_left flag is used to identify which side we should look at: * left side if outer_is_left is false, right side if it is true. * - * The caller need only supply equality conditions arising from joins; - * this routine automatically adds in any usable baserestrictinfo clauses. + * The caller need only supply equality conditions arising from joins in + * restrictlist, plus a list of the relation's restriction clauses in + * base_restrictions, from which this routine picks out the usable ones. + * That is normally rel->baserestrictinfo, but can be a subset of it. * (Note that the passed-in restrictlist will be destructively modified!) - * - * If extra_clauses isn't NULL, return baserestrictinfo clauses which were used - * to derive uniqueness. */ bool relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, - List *restrictlist, List **extra_clauses) + List *restrictlist, List *base_restrictions) { ListCell *ic; @@ -4151,10 +4150,10 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, return false; /* - * Examine the rel's restriction clauses for usable var = const clauses + * Examine the given restriction clauses for usable var = const clauses * that we can add to the restrictlist. */ - foreach(ic, rel->baserestrictinfo) + foreach(ic, base_restrictions) { RestrictInfo *restrictinfo = (RestrictInfo *) lfirst(ic); @@ -4196,7 +4195,6 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, { IndexOptInfo *ind = (IndexOptInfo *) lfirst(ic); int c; - List *exprs = NIL; /* * If the index is not unique, or not immediately enforced, or if it's @@ -4247,26 +4245,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, rexpr = get_leftop(rinfo->clause); if (match_index_to_operand(rexpr, c, ind)) - { - if (bms_membership(rinfo->clause_relids) == BMS_SINGLETON) - { - MemoryContext oldMemCtx = - MemoryContextSwitchTo(root->planner_cxt); - - /* - * Add filter clause into a list allowing caller to - * know if uniqueness have made not only by join - * clauses. - */ - Assert(bms_is_empty(rinfo->left_relids) || - bms_is_empty(rinfo->right_relids)); - if (extra_clauses) - exprs = lappend(exprs, rinfo); - MemoryContextSwitchTo(oldMemCtx); - } - break; /* found a match; column is unique */ - } } if (lc == NULL) @@ -4275,11 +4254,7 @@ relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, /* Matched all key columns of this index? */ if (c == ind->nkeycolumns) - { - if (extra_clauses) - *extra_clauses = exprs; return true; - } } return false; diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c index 25a863efbde..437ee629767 100644 --- a/src/backend/optimizer/plan/analyzejoins.c +++ b/src/backend/optimizer/plan/analyzejoins.c @@ -71,23 +71,14 @@ static void remove_rels_from_query_tree(PlannerInfo *root, static bool reduce_semijoin_in_jointree(Node *jtnode, Relids syn_righthand); static bool rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel); static bool rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, - List *clause_list, List **extra_clauses); + List *clause_list); static DistinctColInfo *distinct_col_search(int colno, List *distinct_cols); -static bool innerrel_is_unique_ext(PlannerInfo *root, - Relids joinrelids, - Relids outerrelids, - RelOptInfo *innerrel, - JoinType jointype, - List *restrictlist, - bool force_cache, - List **extra_clauses); static bool is_innerrel_unique_for(PlannerInfo *root, Relids joinrelids, Relids outerrelids, RelOptInfo *innerrel, JoinType jointype, - List *restrictlist, - List **extra_clauses); + List *restrictlist); static Node *remove_rel_from_jointree(Node *jtnode, int relid, Node **orphan_quals, int *nremoved); static Node *merge_quals(Node *quals1, Node *quals2); @@ -354,7 +345,7 @@ join_is_removable(PlannerInfo *root, SpecialJoinInfo *sjinfo) * Now that we have the relevant equality join clauses, try to prove the * innerrel distinct. */ - if (rel_is_distinct_for(root, innerrel, clause_list, NULL)) + if (rel_is_distinct_for(root, innerrel, clause_list)) return true; /* @@ -652,15 +643,9 @@ rel_supports_distinctness(PlannerInfo *root, RelOptInfo *rel) * Note that the passed-in clause_list may be destructively modified! This * is OK for current uses, because the clause_list is built by the caller for * the sole purpose of passing to this function. - * - * (*extra_clauses) to be set to the right sides of baserestrictinfo clauses, - * looking like "x = const" if distinctness is derived from such clauses, not - * joininfo clauses. Pass NULL to the extra_clauses if this value is not - * needed. */ static bool -rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, List *clause_list, - List **extra_clauses) +rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, List *clause_list) { /* * We could skip a couple of tests here if we assume all callers checked @@ -673,10 +658,11 @@ rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, List *clause_list, { /* * Examine the indexes to see if we have a matching unique index. - * relation_has_unique_index_for automatically adds any usable - * restriction clauses for the rel, so we needn't do that here. + * relation_has_unique_index_for picks out the usable restriction + * clauses for the rel, so we needn't do that here. */ - if (relation_has_unique_index_for(root, rel, clause_list, extra_clauses)) + if (relation_has_unique_index_for(root, rel, clause_list, + rel->baserestrictinfo)) return true; } else if (rel->rtekind == RTE_SUBQUERY) @@ -1018,35 +1004,9 @@ innerrel_is_unique(PlannerInfo *root, JoinType jointype, List *restrictlist, bool force_cache) -{ - return innerrel_is_unique_ext(root, joinrelids, outerrelids, innerrel, - jointype, restrictlist, force_cache, NULL); -} - -/* - * innerrel_is_unique_ext - * Do the same as innerrel_is_unique(), but also set to (*extra_clauses) - * additional clauses from a baserestrictinfo list used to prove the - * uniqueness. - * - * A non-NULL extra_clauses indicates that we're checking for self-join and - * correspondingly dealing with filtered clauses. - */ -static bool -innerrel_is_unique_ext(PlannerInfo *root, - Relids joinrelids, - Relids outerrelids, - RelOptInfo *innerrel, - JoinType jointype, - List *restrictlist, - bool force_cache, - List **extra_clauses) { MemoryContext old_context; ListCell *lc; - UniqueRelInfo *uniqueRelInfo; - List *outer_exprs = NIL; - bool self_join = (extra_clauses != NULL); /* Certainly can't prove uniqueness when there are no joinclauses */ if (restrictlist == NIL) @@ -1061,28 +1021,17 @@ innerrel_is_unique_ext(PlannerInfo *root, /* * Query the cache to see if we've managed to prove that innerrel is - * unique for any subset of this outerrel. For non-self-join search, we - * don't need an exact match, as extra outerrels can't make the innerrel - * any less unique (or more formally, the restrictlist for a join to a - * superset outerrel must be a superset of the conditions we successfully - * used before). For self-join search, we require an exact match of - * outerrels because we need extra clauses to be valid for our case. Also, - * for self-join checking we've filtered the clauses list. Thus, we can - * match only the result cached for a self-join search for another - * self-join check. + * unique for any subset of this outerrel. We don't need an exact match, + * as extra outerrels can't make the innerrel any less unique (or more + * formally, the restrictlist for a join to a superset outerrel must be a + * superset of the conditions we successfully used before). */ foreach(lc, innerrel->unique_for_rels) { - uniqueRelInfo = (UniqueRelInfo *) lfirst(lc); + Relids unique_for_rels = (Relids) lfirst(lc); - if ((!self_join && bms_is_subset(uniqueRelInfo->outerrelids, outerrelids)) || - (self_join && bms_equal(uniqueRelInfo->outerrelids, outerrelids) && - uniqueRelInfo->self_join)) - { - if (extra_clauses) - *extra_clauses = uniqueRelInfo->extra_clauses; + if (bms_is_subset(unique_for_rels, outerrelids)) return true; /* Success! */ - } } /* @@ -1099,8 +1048,7 @@ innerrel_is_unique_ext(PlannerInfo *root, /* No cached information, so try to make the proof. */ if (is_innerrel_unique_for(root, joinrelids, outerrelids, innerrel, - jointype, restrictlist, - self_join ? &outer_exprs : NULL)) + jointype, restrictlist)) { /* * Cache the positive result for future probes, being sure to keep it @@ -1113,16 +1061,10 @@ innerrel_is_unique_ext(PlannerInfo *root, * supersets of them anyway. */ old_context = MemoryContextSwitchTo(root->planner_cxt); - uniqueRelInfo = makeNode(UniqueRelInfo); - uniqueRelInfo->outerrelids = bms_copy(outerrelids); - uniqueRelInfo->self_join = self_join; - uniqueRelInfo->extra_clauses = outer_exprs; innerrel->unique_for_rels = lappend(innerrel->unique_for_rels, - uniqueRelInfo); + bms_copy(outerrelids)); MemoryContextSwitchTo(old_context); - if (extra_clauses) - *extra_clauses = outer_exprs; return true; /* Success! */ } else @@ -1165,8 +1107,7 @@ is_innerrel_unique_for(PlannerInfo *root, Relids outerrelids, RelOptInfo *innerrel, JoinType jointype, - List *restrictlist, - List **extra_clauses) + List *restrictlist) { List *clause_list = NIL; ListCell *lc; @@ -1208,7 +1149,7 @@ is_innerrel_unique_for(PlannerInfo *root, } /* Let rel_is_distinct_for() do the hard work */ - return rel_is_distinct_for(root, innerrel, clause_list, extra_clauses); + return rel_is_distinct_for(root, innerrel, clause_list); } /* @@ -1718,43 +1659,45 @@ split_selfjoin_quals(List *joinquals, List **selfjoinquals, } /* - * Check that each base restriction used to prove uniqueness of relid has - * a matching restriction in outer->baserestrictinfo, after replacing - * relid with outer->relid. + * Return the mergejoinable restrictions of krel that have a matching + * restriction in rrel->baserestrictinfo, after replacing krel->relid with + * rrel->relid. * * Require equal() clauses, including the operator and input collation. * Formerly this tried to match commuted forms of clauses, but that added * a great deal of extra complication without much extra value. */ -static bool -match_unique_clauses(RelOptInfo *outer, List *uclauses, Index relid) +static List * +get_shared_restrictions(RelOptInfo *krel, RelOptInfo *rrel) { - Assert(outer->relid > 0 && relid > 0); + List *result = NIL; + + Assert(krel->relid > 0 && rrel->relid > 0); - foreach_node(RestrictInfo, rinfo, uclauses) + foreach_node(RestrictInfo, rinfo, krel->baserestrictinfo) { Expr *clause; - bool matched = false; - /* Convert the clause from uclauses to use outer's Vars */ + /* Others are of no use to relation_has_unique_index_for */ + if (rinfo->mergeopfamilies == NIL) + continue; + + /* Convert the clause to use rrel's Vars */ clause = (Expr *) copyObject(rinfo->clause); - ChangeVarNodes((Node *) clause, relid, outer->relid, 0); + ChangeVarNodes((Node *) clause, krel->relid, rrel->relid, 0); - /* Look for a matching restriction on the outer relation */ - foreach_node(RestrictInfo, orinfo, outer->baserestrictinfo) + /* Look for a matching restriction on rrel */ + foreach_node(RestrictInfo, orinfo, rrel->baserestrictinfo) { if (equal(clause, orinfo->clause)) { - matched = true; + result = lappend(result, rinfo); break; } } - - if (!matched) - return false; } - return true; + return result; } /* @@ -1794,11 +1737,11 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids) List *restrictlist; List *selfjoinquals; List *otherjoinquals; + List *sharedquals; ListCell *lc; bool jinfo_check = true; PlanRowMark *kmark = NULL; PlanRowMark *rmark = NULL; - List *uclauses = NIL; /* A sanity check: the relations have the same Oid. */ Assert(root->simple_rte_array[k]->relid == @@ -1872,8 +1815,6 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids) restrictlist = generate_join_implied_equalities(root, joinrelids, rrel->relids, krel, NULL); - if (restrictlist == NIL) - continue; /* * Process restrictlist to separate the self-join quals from the @@ -1886,38 +1827,21 @@ remove_self_joins_one_group(PlannerInfo *root, Relids relids) (list_length(selfjoinquals) + list_length(otherjoinquals))); /* - * To enable SJE for the only degenerate case without any self - * join clauses at all, add baserestrictinfo to this list. The - * degenerate case works only if both sides have the same clause. - * So doesn't matter which side to add. + * Each row of rrel must match nothing but its own row of krel. + * That holds if every key column of some unique index of krel is + * equated to the same column of rrel, either by a self-join qual + * or by a restriction that both relations have. A restriction of + * krel alone is no help: in "WHERE s1.b = s2.b AND s1.a = 1 AND + * s2.a = 2" with a unique index on (a,b), each row matches at + * most one row, but never its own. */ - selfjoinquals = list_concat(selfjoinquals, krel->baserestrictinfo); + foreach_node(RestrictInfo, rinfo, selfjoinquals) + rinfo->outer_is_left = bms_is_member(r, rinfo->left_relids); - /* - * Determine if the rrel can duplicate outer rows. We must bypass - * the unique rel cache here since we're possibly using a subset - * of join quals. We can use 'force_cache' == true when all join - * quals are self-join quals. Otherwise, we could end up putting - * false negatives in the cache. - */ - if (!innerrel_is_unique_ext(root, joinrelids, rrel->relids, - krel, JOIN_INNER, selfjoinquals, - list_length(otherjoinquals) == 0, - &uclauses)) - continue; + sharedquals = get_shared_restrictions(krel, rrel); - /* - * 'uclauses' is the copy of outer->baserestrictinfo that are - * associated with an index. We proved by matching selfjoinquals - * to a unique index that the outer relation has at most one - * matching row for each inner row. Sometimes that is not enough. - * e.g. "WHERE s1.b = s2.b AND s1.a = 1 AND s2.a = 2" when the - * unique index is (a,b). Having non-empty uclauses, we must - * validate that the inner baserestrictinfo contains the same - * expressions, or we won't match the same row on each side of the - * join. - */ - if (!match_unique_clauses(rrel, uclauses, krel->relid)) + if (!relation_has_unique_index_for(root, krel, selfjoinquals, + sharedquals)) continue; /* OK, remove rrel from the query */ diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 2ba1dbfb67f..05f4db37d46 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -872,7 +872,7 @@ typedef struct PartitionSchemeData *PartitionScheme; * populate these fields, for base rels; but someday they might be used for * join rels too: * - * unique_for_rels - list of UniqueRelInfo, each one being a set of other + * unique_for_rels - list of Relid sets, each one being a set of other * rels for which this one has been proven unique * non_unique_for_rels - list of Relid sets, each one being a set of * other rels for which we have tried and failed to prove @@ -1123,7 +1123,7 @@ typedef struct RelOptInfo /* * cache space for remembering if we have proven this relation unique */ - /* known unique for these other relid set(s) given in UniqueRelInfo(s) */ + /* known unique for these other relid set(s) */ List *unique_for_rels; /* known not unique for these set(s) */ List *non_unique_for_rels; @@ -3856,35 +3856,4 @@ typedef struct AggTransInfo bool initValueIsNull; } AggTransInfo; -/* - * UniqueRelInfo caches a fact that a relation is unique when being joined - * to other relation(s). - */ -typedef struct UniqueRelInfo -{ - pg_node_attr(no_copy_equal, no_read, no_query_jumble) - - NodeTag type; - - /* - * The relation in consideration is unique when being joined with this set - * of other relation(s). - */ - Relids outerrelids; - - /* - * The relation in consideration is unique when considering only clauses - * suitable for self-join (passed split_selfjoin_quals()). - */ - bool self_join; - - /* - * Additional clauses from a baserestrictinfo list that were used to prove - * the uniqueness. We cache it for the self-join checking procedure: a - * self-join can be removed if the outer relation contains strictly the - * same set of clauses. - */ - List *extra_clauses; -} UniqueRelInfo; - #endif /* PATHNODES_H */ diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 64f87da1674..1350b3b2d82 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -82,7 +82,7 @@ extern void generate_partitionwise_join_paths(PlannerInfo *root, extern void create_index_paths(PlannerInfo *root, RelOptInfo *rel); extern bool relation_has_unique_index_for(PlannerInfo *root, RelOptInfo *rel, List *restrictlist, - List **extra_clauses); + List *base_restrictions); extern bool indexcol_is_bool_constant_for_query(PlannerInfo *root, IndexOptInfo *index, int indexcol); diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 526430a5ea0..17ac5f99e34 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -9016,7 +9016,7 @@ select p.* from QUERY PLAN -------------------------- Result - Replaces: Join on p, x + Replaces: Scan on x One-Time Filter: false (3 rows) @@ -9954,6 +9954,15 @@ SELECT * FROM sj j1, sj j2 WHERE j1.b = j2.b AND 1 = j1.a AND j2.a = 1; ---+---+---+---+---+--- (0 rows) +-- Remove SJ without any join clause, defining uniqueness by a constant +EXPLAIN (COSTS OFF) +SELECT * FROM sj j1, sj j2 WHERE j1.a = 2 AND j2.a = 2; + QUERY PLAN +------------------- + Seq Scan on sj j2 + Filter: (a = 2) +(2 rows) + -- SJE Corner case: a 'a.x=a.x' clause, have replaced with 'a.x IS NOT NULL' -- after SJ elimination it shouldn't be a mergejoinable clause. EXPLAIN (COSTS OFF) @@ -10062,19 +10071,16 @@ SELECT * FROM sj j1, sj j2 Filter: ((b IS NOT NULL) AND (a = 2) AND (c = 3)) (2 rows) --- Don't remove SJ +-- Remove SJ by the unique index on c, though the restrictions on a differ EXPLAIN (COSTS OFF) SELECT * FROM sj j1, sj j2 WHERE j1.b = j2.b AND 2 = j1.a AND j1.c = 3 AND j2.a = 1 AND 3 = j2.c; - QUERY PLAN ---------------------------------------- - Nested Loop - Join Filter: (j1.b = j2.b) - -> Seq Scan on sj j1 - Filter: ((2 = a) AND (c = 3)) - -> Seq Scan on sj j2 - Filter: ((c = 3) AND (a = 1)) -(6 rows) + QUERY PLAN +-------------------------- + Result + Replaces: Scan on j2 + One-Time Filter: false +(3 rows) CREATE UNIQUE INDEX sj_temp_idx ON sj(a,b); -- Don't remove SJ diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 27b44016717..05d7e75d6c4 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -3817,6 +3817,10 @@ SELECT * FROM sj j1, sj j2 WHERE j1.b = j2.b AND 1 = j1.a AND j2.a = 1; -- Return no rows SELECT * FROM sj j1, sj j2 WHERE j1.b = j2.b AND 1 = j1.a AND j2.a = 1; +-- Remove SJ without any join clause, defining uniqueness by a constant +EXPLAIN (COSTS OFF) +SELECT * FROM sj j1, sj j2 WHERE j1.a = 2 AND j2.a = 2; + -- SJE Corner case: a 'a.x=a.x' clause, have replaced with 'a.x IS NOT NULL' -- after SJ elimination it shouldn't be a mergejoinable clause. EXPLAIN (COSTS OFF) @@ -3871,7 +3875,7 @@ EXPLAIN (COSTS OFF) SELECT * FROM sj j1, sj j2 WHERE j1.b = j2.b AND j1.a = 2 AND j1.c = 3 AND j2.a = 2 AND 3 = j2.c; --- Don't remove SJ +-- Remove SJ by the unique index on c, though the restrictions on a differ EXPLAIN (COSTS OFF) SELECT * FROM sj j1, sj j2 WHERE j1.b = j2.b AND 2 = j1.a AND j1.c = 3 AND j2.a = 1 AND 3 = j2.c; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index f10eb469266..278b2f3194b 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3318,7 +3318,6 @@ UnicodeNormalizationForm UnicodeNormalizationQC Unique UniquePath -UniqueRelInfo UniqueState UnlistenStmt UnresolvedTup -- 2.37.1 (Apple Git-137.1)