From 46b31c6b5d2efdebaf8b994ae099488c30f91add Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Fri, 23 Oct 2026 10:00:38 +0900 Subject: [PATCH v5] Don't replace clone clauses with constant TRUE When reconsider_outer_join_clauses() finds an outer-join clause OUTERVAR = INNERVAR whose OUTERVAR is known to equal a constant, it pushes INNERVAR = CONSTANT into the inner side and replaces the clause with constant TRUE. The replacement assumes that OUTERVAR cannot have gone to NULL where the clause is evaluated. That assumption fails for a clone made for outer join identity 3. When an outer join is pushed into the RHS of a lower outer join, its relid is not added to the joinrel until the commuted pair is complete, so a version of a clause that lacks that join's nulling bit can be applied above it, after it has null-extended OUTERVAR. This is fine for the original clause, because it is strict, which is what allowed the commutation in the first place; but it is not fine for constant TRUE. As a result, inner rows could be joined to null-extended outer rows, producing wrong query results. To fix, throw back a clone as-is rather than the TRUE replacement, as v15 and earlier did. The derived INNERVAR = CONSTANT clause remains valid and is still pushed down. This can leave a redundant join filter in some plans, as seen in an existing regression test. To keep the thrown-back clause from affecting selectivity estimates, including for parameterized scans, where the cached selectivity is not used, restore the redundancy marker removed by b448f1c8d: a norm_selec greater than 1 marks a redundant clause, for which clause_selectivity_ext() always returns 1.0. Back-patch to v16, where clone clauses were introduced. Bug: #19765 Reported-by: Viktor Leis Diagnosed-by: Tender Wang Author: Richard Guo Co-authored-by: Nitin Motiani Reviewed-by: Tender Wang Discussion: https://postgr.es/m/19765-5778b18918040583@postgresql.org Backpatch-through: 16 --- src/backend/optimizer/path/clausesel.c | 6 +++ src/backend/optimizer/path/equivclass.c | 24 +++++++++ src/include/nodes/pathnodes.h | 5 +- src/test/regress/expected/join.out | 68 ++++++++++++++++++++++++- src/test/regress/sql/join.sql | 24 +++++++++ 5 files changed, 125 insertions(+), 2 deletions(-) diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c index 25c4d177ad9..c22cbfc9ca0 100644 --- a/src/backend/optimizer/path/clausesel.c +++ b/src/backend/optimizer/path/clausesel.c @@ -713,6 +713,12 @@ clause_selectivity_ext(PlannerInfo *root, return (Selectivity) 1.0; } + /* + * If the clause is marked redundant, always return 1.0. + */ + if (rinfo->norm_selec > 1) + return (Selectivity) 1.0; + /* * If possible, cache the result of the selectivity calculation for * the clause. We can cache if varRelid is zero or the clause diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index f0a35c56bae..598b62b6764 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -2156,6 +2156,12 @@ create_join_clause(PlannerInfo *root, * We handle this by generating a constant-TRUE clause that is marked with * the same required_relids etc as the removed outer-join clause, thus * making it a join clause between the correct relations. + * + * However, we can't do that for a clone clause, since it may be applied + * above a commuted outer join that has already null-extended OUTERVAR but + * whose relid has not yet been added to the joinrel. The original strict + * clause is needed there to reject the null-extended rows, so we throw + * back a clone as-is, marked redundant for selectivity estimation. */ void reconsider_outer_join_clauses(PlannerInfo *root) @@ -2181,6 +2187,15 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->left_join_clauses = foreach_delete_current(root->left_join_clauses, cell); + /* throw back a clone as-is (see notes above) */ + if (rinfo->has_clone || rinfo->is_clone) + { + /* but the thrown-back clause has no extra selectivity */ + rinfo->norm_selec = 2.0; + rinfo->outer_selec = 1.0; + distribute_restrictinfo_to_rels(root, rinfo); + continue; + } /* throw back a dummy replacement clause (see notes above) */ rinfo = make_restrictinfo(root, (Expr *) makeBoolConst(true, false), @@ -2209,6 +2224,15 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->right_join_clauses = foreach_delete_current(root->right_join_clauses, cell); + /* throw back a clone as-is (see notes above) */ + if (rinfo->has_clone || rinfo->is_clone) + { + /* but the thrown-back clause has no extra selectivity */ + rinfo->norm_selec = 2.0; + rinfo->outer_selec = 1.0; + distribute_restrictinfo_to_rels(root, rinfo); + continue; + } /* throw back a dummy replacement clause (see notes above) */ rinfo = make_restrictinfo(root, (Expr *) makeBoolConst(true, false), diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 1140c27356a..ff46a9f7419 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -3004,7 +3004,10 @@ typedef struct RestrictInfo /* eval cost of clause; -1 if not yet set */ QualCost eval_cost pg_node_attr(equal_ignore); - /* selectivity for "normal" (JOIN_INNER) semantics; -1 if not yet set */ + /* + * selectivity for "normal" (JOIN_INNER) semantics; -1 if not yet set; >1 + * means a redundant clause + */ Selectivity norm_selec pg_node_attr(equal_ignore); /* selectivity for outer join semantics; -1 if not yet set */ Selectivity outer_selec pg_node_attr(equal_ignore); diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 1e6aa380257..34a386237be 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -4296,6 +4296,71 @@ where f2 = 53; 53 | | | (1 row) +-- the zt3/zt1b join clause must be kept when zt2/zt3 is commuted into zt1/zt2 +explain (costs off) +select * from + zt1 left join zt2 on (zt1.f1 = f2) + left join zt3 on (f2 = f3) + left join zt1 zt1b on (f3 = zt1b.f1) +where zt1.f1 = 53; + QUERY PLAN +--------------------------------------------------------- + Nested Loop Left Join + Join Filter: (zt3.f3 = zt1b.f1) + -> Nested Loop Left Join + Join Filter: (zt2.f2 = zt3.f3) + -> Nested Loop Left Join + -> Index Only Scan using zt1_pkey on zt1 + Index Cond: (f1 = 53) + -> Index Only Scan using zt2_pkey on zt2 + Index Cond: (f2 = 53) + -> Index Only Scan using zt3_pkey on zt3 + Index Cond: (f3 = 53) + -> Index Only Scan using zt1_pkey on zt1 zt1b + Index Cond: (f1 = 53) +(13 rows) + +select * from + zt1 left join zt2 on (zt1.f1 = f2) + left join zt3 on (f2 = f3) + left join zt1 zt1b on (f3 = zt1b.f1) +where zt1.f1 = 53; + f1 | f2 | f3 | f1 +----+----+----+---- + 53 | 53 | | +(1 row) + +-- likewise for the zt3/zt2 join clause when zt1b/zt3 is commuted into zt1/zt1b +explain (costs off) +select * from + zt1 left join zt1 zt1b on (zt1.f1 = zt1b.f1 and zt1b.f1 = 53) + left join (zt3 left join zt2 on (f3 = f2)) on (zt1b.f1 = f3); + QUERY PLAN +-------------------------------------------------------------------- + Hash Left Join + Hash Cond: (zt1.f1 = zt1b.f1) + -> Seq Scan on zt1 + -> Hash + -> Nested Loop Left Join + Join Filter: (zt3.f3 = zt2.f2) + -> Nested Loop Left Join + Join Filter: (zt1b.f1 = zt3.f3) + -> Index Only Scan using zt1_pkey on zt1 zt1b + Index Cond: (f1 = 53) + -> Index Only Scan using zt3_pkey on zt3 + Index Cond: (f3 = 53) + -> Index Only Scan using zt2_pkey on zt2 + Index Cond: (f2 = 53) +(14 rows) + +select * from + zt1 left join zt1 zt1b on (zt1.f1 = zt1b.f1 and zt1b.f1 = 53) + left join (zt3 left join zt2 on (f3 = f2)) on (zt1b.f1 = f3); + f1 | f1 | f3 | f2 +----+----+----+---- + 53 | 53 | | +(1 row) + -- -- regression test for improper extraction of OR indexqual conditions -- (as seen in early 8.3.x releases) @@ -6962,6 +7027,7 @@ where tt1.f1 = ss1.c0; Output: tt4.f1 -> Nested Loop Left Join Output: tt4.f1 + Join Filter: (tt3.f1 = tt4.f1) -> Seq Scan on public.text_tbl tt3 Output: tt3.f1 Filter: (tt3.f1 = 'foo'::text) @@ -6979,7 +7045,7 @@ where tt1.f1 = ss1.c0; Output: (tt4.f1) -> Seq Scan on public.text_tbl tt5 Output: tt4.f1 -(32 rows) +(33 rows) select 1 from text_tbl as tt1 diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 6264f23e081..0eb8862bb18 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1305,6 +1305,30 @@ select * from left join zv1 on (f3 = f1) where f2 = 53; +-- the zt3/zt1b join clause must be kept when zt2/zt3 is commuted into zt1/zt2 +explain (costs off) +select * from + zt1 left join zt2 on (zt1.f1 = f2) + left join zt3 on (f2 = f3) + left join zt1 zt1b on (f3 = zt1b.f1) +where zt1.f1 = 53; + +select * from + zt1 left join zt2 on (zt1.f1 = f2) + left join zt3 on (f2 = f3) + left join zt1 zt1b on (f3 = zt1b.f1) +where zt1.f1 = 53; + +-- likewise for the zt3/zt2 join clause when zt1b/zt3 is commuted into zt1/zt1b +explain (costs off) +select * from + zt1 left join zt1 zt1b on (zt1.f1 = zt1b.f1 and zt1b.f1 = 53) + left join (zt3 left join zt2 on (f3 = f2)) on (zt1b.f1 = f3); + +select * from + zt1 left join zt1 zt1b on (zt1.f1 = zt1b.f1 and zt1b.f1 = 53) + left join (zt3 left join zt2 on (f3 = f2)) on (zt1b.f1 = f3); + -- -- regression test for improper extraction of OR indexqual conditions -- (as seen in early 8.3.x releases) -- 2.37.1 (Apple Git-137.1)