diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index ef0172b2f82..b07c7f03aaa 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1731,36 +1731,60 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, */ joinrelids = bms_union(baserel->relids, required_outer); pclauses = NIL; + pserials = NULL; foreach(lc, baserel->joininfo) { RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); - /* - * A clone clause must not be enforced here if an outer join it is - * incompatible with has already been computed below the point of - * evaluation; some other clone is the right one to apply. - */ - if ((rinfo->has_clone || rinfo->is_clone) && - bms_overlap(rinfo->incompatible_relids, joinrelids)) + if (!join_clause_is_movable_into(rinfo, + baserel->relids, + joinrelids)) continue; - if (join_clause_is_movable_into(rinfo, - baserel->relids, - joinrelids)) - pclauses = lappend(pclauses, rinfo); + /* + * If it's a clone clause, drop variants that are incompatible with an + * outer join already computed below the point of evaluation; some + * other variant is the right one to apply. + * + * Multiple variants can survive that test under one parameterization, + * but only when they are parse-tree identical, which happens when a + * commuting outer join nulls no Var actually referenced by the + * clause. (Otherwise, a variant's extra nullingrels put that outer + * join into its clause_relids, so being movable here means the join + * is part of the parameterization, and the lesser variant is rejected + * above.) Identical variants still differ in required_relids and + * incompatible_relids, and join-level clause selection needs all of + * them: each is the sole legal choice in some join order. Here, + * though, that distinction is not meaningful, since the same + * ParamPathInfo serves every join order that can use the path, and an + * identical variant is correct in any of them. So enforce just the + * first survivor, identifying later ones by matching rinfo_serial; + * enforcing them too would waste execution effort and apply the + * clause's selectivity multiple times. + */ + if (rinfo->has_clone || rinfo->is_clone) + { + if (bms_overlap(rinfo->incompatible_relids, joinrelids)) + continue; + if (bms_is_member(rinfo->rinfo_serial, pserials)) + continue; + } + + pclauses = lappend(pclauses, rinfo); + pserials = bms_add_member(pserials, rinfo->rinfo_serial); } /* - * Add in joinclauses generated by EquivalenceClasses, too. (These - * necessarily satisfy join_clause_is_movable_into; but in assert-enabled - * builds, let's verify that.) + * Add in joinclauses generated by EquivalenceClasses, too, folding their + * serial numbers into pserials. (These clauses necessarily satisfy + * join_clause_is_movable_into; but in assert-enabled builds, let's verify + * that.) */ eqclauses = generate_join_implied_equalities(root, joinrelids, required_outer, baserel, NULL); -#ifdef USE_ASSERT_CHECKING foreach(lc, eqclauses) { RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); @@ -1768,18 +1792,9 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, Assert(join_clause_is_movable_into(rinfo, baserel->relids, joinrelids)); - } -#endif - pclauses = list_concat(pclauses, eqclauses); - - /* Compute set of serial numbers of the enforced clauses */ - pserials = NULL; - foreach(lc, pclauses) - { - RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); - pserials = bms_add_member(pserials, rinfo->rinfo_serial); } + pclauses = list_concat(pclauses, eqclauses); /* Estimate the number of rows returned by the parameterized scan */ rows = get_parameterized_baserel_size(root, baserel, pclauses); @@ -1836,6 +1851,7 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel, Relids outer_and_req; Relids inner_and_req; List *pclauses; + Bitmapset *pserials; List *eclauses; List *dropped_ecs; double rows; @@ -1872,25 +1888,33 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel, inner_and_req = NULL; /* inner path does not accept parameters */ pclauses = NIL; + pserials = NULL; foreach(lc, joinrel->joininfo) { RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); - /* As above, reject clones incompatible with a computed outer join */ - if ((rinfo->has_clone || rinfo->is_clone) && - bms_overlap(rinfo->incompatible_relids, join_and_req)) + if (!join_clause_is_movable_into(rinfo, + joinrel->relids, + join_and_req) || + join_clause_is_movable_into(rinfo, + outer_path->parent->relids, + outer_and_req) || + join_clause_is_movable_into(rinfo, + inner_path->parent->relids, + inner_and_req)) continue; - if (join_clause_is_movable_into(rinfo, - joinrel->relids, - join_and_req) && - !join_clause_is_movable_into(rinfo, - outer_path->parent->relids, - outer_and_req) && - !join_clause_is_movable_into(rinfo, - inner_path->parent->relids, - inner_and_req)) - pclauses = lappend(pclauses, rinfo); + /* As above, apply only one variant of a clone clause */ + if (rinfo->has_clone || rinfo->is_clone) + { + if (bms_overlap(rinfo->incompatible_relids, join_and_req)) + continue; + if (bms_is_member(rinfo->rinfo_serial, pserials)) + continue; + } + + pclauses = lappend(pclauses, rinfo); + pserials = bms_add_member(pserials, rinfo->rinfo_serial); } /* Consider joinclauses generated by EquivalenceClasses, too */ @@ -1949,6 +1973,12 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel, * has nothing that needs to be enforced here, while if the clause can be * moved into the LHS then it should have been enforced within that path.) * + * In cases where an EC needs to constrain EC members that are newly + * computable at this join, it can emit clauses that it already returned + * above and we accepted into pclauses. Hence, do a final list-membership + * check before accepting more clauses. (Pointer comparison should be + * enough to detect duplicates, since ECs cache derived clauses.) + * * Note that we don't need similar processing for ECs whose clause was * considered to be movable into the LHS, because the LHS can't refer to * the RHS so there is no comparable ambiguity about what it might @@ -1973,10 +2003,13 @@ get_joinrel_parampathinfo(PlannerInfo *root, RelOptInfo *joinrel, Assert(join_clause_is_movable_into(rinfo, outer_path->parent->relids, real_outer_and_req)); - if (!join_clause_is_movable_into(rinfo, - outer_path->parent->relids, - outer_and_req)) - pclauses = lappend(pclauses, rinfo); + if (join_clause_is_movable_into(rinfo, + outer_path->parent->relids, + outer_and_req)) + continue; /* drop if movable into LHS */ + if (list_member_ptr(pclauses, rinfo)) + continue; /* drop if already accepted */ + pclauses = lappend(pclauses, rinfo); } } diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 4353f8cb9ec..f89ab203fb3 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5965,6 +5965,61 @@ select count(*) from int4_tbl t1 left join 25 (1 row) +-- +-- check that identical clone variants of an outer-join qual are not +-- enforced multiple times in a parameterized path +-- +explain (costs off) +select * from onek t1 + left join onek t2 on t1.unique1 = t2.unique1 + left join onek t3 on t2.unique1 = t3.unique1 + left join onek t4 on t3.unique1 = t4.unique1 and t3.ten = t4.ten + 0 + and t2.unique2 = t4.unique2 + 0 +where t1.unique1 < 1; + QUERY PLAN +------------------------------------------------------------ + Nested Loop Left Join + Join Filter: (t2.unique2 = (t4.unique2 + 0)) + -> Nested Loop Left Join + -> Nested Loop Left Join + -> Index Scan using onek_unique1 on onek t1 + Index Cond: (unique1 < 1) + -> Index Scan using onek_unique1 on onek t2 + Index Cond: (unique1 = t1.unique1) + -> Index Scan using onek_unique1 on onek t3 + Index Cond: (unique1 = t2.unique1) + -> Index Scan using onek_unique1 on onek t4 + Index Cond: (unique1 = t3.unique1) + Filter: (t3.ten = (ten + 0)) +(13 rows) + +explain (costs off) +select * from onek t1 + left join onek t2 on t1.unique1 = t2.unique1 + left join onek t3 on t2.unique1 = t3.unique1 + left join (onek t4 join onek t5 on t4.ten = t5.ten) + on t3.unique1 = t5.unique1 and t3.hundred = t4.unique1 + t5.two + and t2.unique2 = t4.unique2 +where t1.unique1 < 1; + QUERY PLAN +----------------------------------------------------------------------------------- + Nested Loop Left Join + -> Nested Loop Left Join + -> Nested Loop Left Join + -> Index Scan using onek_unique1 on onek t1 + Index Cond: (unique1 < 1) + -> Index Scan using onek_unique1 on onek t2 + Index Cond: (unique1 = t1.unique1) + -> Index Scan using onek_unique1 on onek t3 + Index Cond: (unique1 = t2.unique1) + -> Nested Loop + Join Filter: ((t4.ten = t5.ten) AND (t3.hundred = (t4.unique1 + t5.two))) + -> Index Scan using onek_unique2 on onek t4 + Index Cond: (unique2 = t2.unique2) + -> Index Scan using onek_unique1 on onek t5 + Index Cond: (unique1 = t3.unique1) +(15 rows) + -- -- test successful handling of full join underneath left join (bug #14105) -- diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index 75e4d66453a..83f9bf84555 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2135,6 +2135,28 @@ select count(*) from int4_tbl t1 left join join int4_tbl t4 on t3.cnt = t4.f1) on t2.bx = t3.cnt + t4.f1; +-- +-- check that identical clone variants of an outer-join qual are not +-- enforced multiple times in a parameterized path +-- + +explain (costs off) +select * from onek t1 + left join onek t2 on t1.unique1 = t2.unique1 + left join onek t3 on t2.unique1 = t3.unique1 + left join onek t4 on t3.unique1 = t4.unique1 and t3.ten = t4.ten + 0 + and t2.unique2 = t4.unique2 + 0 +where t1.unique1 < 1; + +explain (costs off) +select * from onek t1 + left join onek t2 on t1.unique1 = t2.unique1 + left join onek t3 on t2.unique1 = t3.unique1 + left join (onek t4 join onek t5 on t4.ten = t5.ten) + on t3.unique1 = t5.unique1 and t3.hundred = t4.unique1 + t5.two + and t2.unique2 = t4.unique2 +where t1.unique1 < 1; + -- -- test successful handling of full join underneath left join (bug #14105) --