commit 30d9a0d7c0201152cced5ba040ecd8d5a5bbb3b3 Author: Tom Lane Date: Mon Oct 31 13:44:10 2022 -0400 Use constant TRUE for "dummy" clauses when throwing back outer joins. This improves on a hack I introduced in commit 6a6522529. If we have a left-join clause l.x = r.y, and a WHERE clause l.x = constant, we generate r.y = constant and then don't really have a need for the join clause. Currently we throw the join clause back anyway after marking it redundant, so that the join search heuristics won't think this is a clauseless join and avoid it. That was a kluge introduced under time pressure, and after looking at it I thought of a better way: let's just introduce constant-TRUE "join clauses" instead, and get rid of them at the end. This improves the generated plans for such cases by not having to test a redundant join clause. We can also get rid of the ugly hack used to mark such clauses as redundant for selectivity estimation. The code added here should go away again, once we handle these cases as ordinary eclasses. But it seemed worth committing separately so as to get the regression changes and selectivity simplifications in place. diff --git a/src/backend/optimizer/path/clausesel.c b/src/backend/optimizer/path/clausesel.c index c08eb2b1c5..1cf565ee59 100644 --- a/src/backend/optimizer/path/clausesel.c +++ b/src/backend/optimizer/path/clausesel.c @@ -715,12 +715,6 @@ 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 0737cc355f..a9f5db3ef6 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -1954,14 +1954,11 @@ create_join_clause(PlannerInfo *root, * If we don't find any match for a set-aside outer join clause, we must * throw it back into the regular joinclause processing by passing it to * distribute_restrictinfo_to_rels(). If we do generate a derived clause, - * however, the outer-join clause is redundant. We still throw it back, - * because otherwise the join will be seen as a clauseless join and avoided - * during join order searching; but we mark it as redundant to keep from - * messing up the joinrel's size estimate. (This behavior means that the - * API for this routine is uselessly complex: we could have just put all - * the clauses into the regular processing initially. We keep it because - * someday we might want to do something else, such as inserting "dummy" - * joinclauses instead of real ones.) + * however, the outer-join clause is redundant. We must still put some + * clause into the regular processing, because otherwise the join will be + * seen as a clauseless join and avoided during join order searching. + * We handle this by generating a constant-TRUE clause that is marked with + * required_relids that make it a join between the correct relations. * * Outer join clauses that are marked outerjoin_delayed are special: this * condition means that one or both VARs might go to null due to a lower @@ -1994,10 +1991,15 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->left_join_clauses = foreach_delete_current(root->left_join_clauses, cell); - /* we throw it back anyway (see notes above) */ - /* but the thrown-back clause has no extra selectivity */ - rinfo->norm_selec = 2.0; - rinfo->outer_selec = 1.0; + /* throw back a dummy replacement clause (see notes above) */ + rinfo = make_restrictinfo(root, + (Expr *) makeBoolConst(true, false), + true, /* is_pushed_down */ + false, /* outerjoin_delayed */ + false, /* pseudoconstant */ + 0, /* security_level */ + rinfo->required_relids, + rinfo->outer_relids); distribute_restrictinfo_to_rels(root, rinfo); } } @@ -2013,10 +2015,15 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->right_join_clauses = foreach_delete_current(root->right_join_clauses, cell); - /* we throw it back anyway (see notes above) */ - /* but the thrown-back clause has no extra selectivity */ - rinfo->norm_selec = 2.0; - rinfo->outer_selec = 1.0; + /* throw back a dummy replacement clause (see notes above) */ + rinfo = make_restrictinfo(root, + (Expr *) makeBoolConst(true, false), + true, /* is_pushed_down */ + false, /* outerjoin_delayed */ + false, /* pseudoconstant */ + 0, /* security_level */ + rinfo->required_relids, + rinfo->outer_relids); distribute_restrictinfo_to_rels(root, rinfo); } } @@ -2034,10 +2041,15 @@ reconsider_outer_join_clauses(PlannerInfo *root) /* remove it from the list */ root->full_join_clauses = foreach_delete_current(root->full_join_clauses, cell); - /* we throw it back anyway (see notes above) */ - /* but the thrown-back clause has no extra selectivity */ - rinfo->norm_selec = 2.0; - rinfo->outer_selec = 1.0; + /* throw back a dummy replacement clause (see notes above) */ + rinfo = make_restrictinfo(root, + (Expr *) makeBoolConst(true, false), + true, /* is_pushed_down */ + false, /* outerjoin_delayed */ + false, /* pseudoconstant */ + 0, /* security_level */ + rinfo->required_relids, + rinfo->outer_relids); distribute_restrictinfo_to_rels(root, rinfo); } } diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c index 336a73d3b9..4b98692189 100644 --- a/src/backend/optimizer/util/orclauses.c +++ b/src/backend/optimizer/util/orclauses.c @@ -98,18 +98,13 @@ extract_restriction_or_clauses(PlannerInfo *root) * joinclause that is considered safe to move to this rel by the * parameterized-path machinery, even though what we are going to do * with it is not exactly a parameterized path. - * - * However, it seems best to ignore clauses that have been marked - * redundant (by setting norm_selec > 1). That likely can't happen - * for OR clauses, but let's be safe. */ foreach(lc, rel->joininfo) { RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); if (restriction_is_or_clause(rinfo) && - join_clause_is_movable_to(rinfo, rel) && - rinfo->norm_selec <= 1) + join_clause_is_movable_to(rinfo, rel)) { /* Try to extract a qual for this rel only */ Expr *orclause = extract_or_clause(rinfo, rel); @@ -356,7 +351,7 @@ consider_new_or_clause(PlannerInfo *root, RelOptInfo *rel, /* And hack cached selectivity so join size remains the same */ join_or_rinfo->norm_selec = orig_selec / or_selec; - /* ensure result stays in sane range, in particular not "redundant" */ + /* ensure result stays in sane range */ if (join_or_rinfo->norm_selec > 1) join_or_rinfo->norm_selec = 1; /* as explained above, we don't touch outer_selec */ diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index 1d8912608b..c3af845acd 100644 --- a/src/backend/optimizer/util/restrictinfo.c +++ b/src/backend/optimizer/util/restrictinfo.c @@ -424,6 +424,21 @@ restriction_is_securely_promotable(RestrictInfo *restrictinfo, return false; } +/* + * Detect whether a RestrictInfo's clause is constant TRUE (note that it's + * surely of type boolean). No such WHERE clause could survive qual + * canonicalization, but equivclass.c may generate such RestrictInfos for + * reasons discussed therein. We should drop them again when creating + * the finished plan, which is handled by the next few functions. + */ +static inline bool +rinfo_is_constant_true(RestrictInfo *rinfo) +{ + return IsA(rinfo->clause, Const) && + !((Const *) rinfo->clause)->constisnull && + DatumGetBool(((Const *) rinfo->clause)->constvalue); +} + /* * get_actual_clauses * @@ -443,6 +458,7 @@ get_actual_clauses(List *restrictinfo_list) RestrictInfo *rinfo = lfirst_node(RestrictInfo, l); Assert(!rinfo->pseudoconstant); + Assert(!rinfo_is_constant_true(rinfo)); result = lappend(result, rinfo->clause); } @@ -454,6 +470,7 @@ get_actual_clauses(List *restrictinfo_list) * * Extract bare clauses from 'restrictinfo_list', returning either the * regular ones or the pseudoconstant ones per 'pseudoconstant'. + * Constant-TRUE clauses are dropped in any case. */ List * extract_actual_clauses(List *restrictinfo_list, @@ -466,7 +483,8 @@ extract_actual_clauses(List *restrictinfo_list, { RestrictInfo *rinfo = lfirst_node(RestrictInfo, l); - if (rinfo->pseudoconstant == pseudoconstant) + if (rinfo->pseudoconstant == pseudoconstant && + !rinfo_is_constant_true(rinfo)) result = lappend(result, rinfo->clause); } return result; @@ -477,7 +495,7 @@ extract_actual_clauses(List *restrictinfo_list, * * Extract bare clauses from 'restrictinfo_list', separating those that * semantically match the join level from those that were pushed down. - * Pseudoconstant clauses are excluded from the results. + * Pseudoconstant and constant-TRUE clauses are excluded from the results. * * This is only used at outer joins, since for plain joins we don't care * about pushed-down-ness. @@ -499,13 +517,15 @@ extract_actual_join_clauses(List *restrictinfo_list, if (RINFO_IS_PUSHED_DOWN(rinfo, joinrelids)) { - if (!rinfo->pseudoconstant) + if (!rinfo->pseudoconstant && + !rinfo_is_constant_true(rinfo)) *otherquals = lappend(*otherquals, rinfo->clause); } else { /* joinquals shouldn't have been marked pseudoconstant */ Assert(!rinfo->pseudoconstant); + Assert(!rinfo_is_constant_true(rinfo)); *joinquals = lappend(*joinquals, rinfo->clause); } } diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index ea43313f11..6a5ff13d39 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -2534,10 +2534,7 @@ 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; >1 - * means a redundant clause - */ + /* selectivity for "normal" (JOIN_INNER) semantics; -1 if not yet set */ 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 340c57a270..dd0f622b78 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -3952,8 +3952,8 @@ explain (costs off) select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand) from tenk1 a left join tenk1 b on b.thousand = a.unique1 left join tenk1 c on c.unique2 = coalesce(b.twothousand, a.twothousand) where a.unique2 < 10 and coalesce(b.twothousand, a.twothousand) = 44; - QUERY PLAN ---------------------------------------------------------------------------------------------- + QUERY PLAN +--------------------------------------------------------------- Nested Loop Left Join -> Nested Loop Left Join Filter: (COALESCE(b.twothousand, a.twothousand) = 44) @@ -3964,7 +3964,7 @@ select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand) -> Bitmap Index Scan on tenk1_thous_tenthous Index Cond: (thousand = a.unique1) -> Index Scan using tenk1_unique2 on tenk1 c - Index Cond: ((unique2 = COALESCE(b.twothousand, a.twothousand)) AND (unique2 = 44)) + Index Cond: (unique2 = 44) (11 rows) select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand) @@ -4395,7 +4395,6 @@ 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) @@ -4413,7 +4412,7 @@ where tt1.f1 = ss1.c0; Output: (tt4.f1) -> Seq Scan on public.text_tbl tt5 Output: tt4.f1 -(33 rows) +(32 rows) select 1 from text_tbl as tt1 @@ -4520,24 +4519,22 @@ explain (costs off) QUERY PLAN ------------------------------------------------- Nested Loop Left Join - Join Filter: (a.f1 = b.unique2) -> Seq Scan on int4_tbl a Filter: (f1 = 0) -> Index Scan using tenk1_unique2 on tenk1 b Index Cond: (unique2 = 0) -(6 rows) +(5 rows) explain (costs off) select * from tenk1 a full join tenk1 b using(unique2) where unique2 = 42; QUERY PLAN ------------------------------------------------- Merge Full Join - Merge Cond: (a.unique2 = b.unique2) -> Index Scan using tenk1_unique2 on tenk1 a Index Cond: (unique2 = 42) -> Index Scan using tenk1_unique2 on tenk1 b Index Cond: (unique2 = 42) -(6 rows) +(5 rows) -- -- test that quals attached to an outer join have correct semantics,