From 802c20938f04d677d44ca0105c1589efb89a4cc5 Mon Sep 17 00:00:00 2001 From: Richard Guo Date: Fri, 17 Mar 2023 10:29:27 +0800 Subject: [PATCH v2] Check group_clause_relids to see if a clause is computable --- src/backend/optimizer/plan/initsplan.c | 15 ++++++++++++ src/backend/optimizer/util/relnode.c | 2 +- src/backend/optimizer/util/restrictinfo.c | 1 + src/include/nodes/pathnodes.h | 6 +++++ src/test/regress/expected/join.out | 28 +++++++++++++++++++++++ src/test/regress/sql/join.sql | 10 ++++++++ 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c index c6c21a7338..d9527caa01 100644 --- a/src/backend/optimizer/plan/initsplan.c +++ b/src/backend/optimizer/plan/initsplan.c @@ -123,6 +123,7 @@ static void distribute_qual_to_rels(PlannerInfo *root, Node *clause, bool allow_equivalence, bool has_clone, bool is_clone, + Relids group_clause_relids, List **postponed_oj_qual_list); static bool check_redundant_nullability_qual(PlannerInfo *root, Node *clause); static Relids get_join_domain_min_rels(PlannerInfo *root, Relids domain_relids); @@ -2100,6 +2101,14 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses, List **postponed_oj_qual_list) { ListCell *lc; + Relids group_clause_relids = NULL; + + /* + * Retrieve all relids mentioned within the clauses if they are clone + * clauses. + */ + if (has_clone || is_clone) + group_clause_relids = pull_varnos(root, (Node *) clauses); foreach(lc, clauses) { @@ -2115,6 +2124,7 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses, allow_equivalence, has_clone, is_clone, + group_clause_relids, postponed_oj_qual_list); } } @@ -2147,6 +2157,8 @@ distribute_quals_to_rels(PlannerInfo *root, List *clauses, * EquivalenceClass * 'has_clone': has_clone property to assign to the qual * 'is_clone': is_clone property to assign to the qual + * 'group_clause_relids': relids actually referenced in all the clauses from + * the same jointree node if a cloned clause, otherwise NULL * 'postponed_oj_qual_list': if not NULL, non-degenerate outer join clauses * should be added to this list instead of being processed (list entries * are just the bare clauses) @@ -2170,6 +2182,7 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause, bool allow_equivalence, bool has_clone, bool is_clone, + Relids group_clause_relids, List **postponed_oj_qual_list) { Relids relids; @@ -2393,6 +2406,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause, /* Apply appropriate clone marking, too */ restrictinfo->has_clone = has_clone; restrictinfo->is_clone = is_clone; + /* and set the group clause relids */ + restrictinfo->group_clause_relids = group_clause_relids; /* * If it's a join clause, add vars used in the clause to targetlists of diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 68fd033595..06d280e477 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1311,7 +1311,7 @@ subbuild_joinrel_restrictlist(PlannerInfo *root, Assert(!RINFO_IS_PUSHED_DOWN(rinfo, joinrel->relids)); if (!bms_is_subset(rinfo->required_relids, both_input_relids)) continue; - if (!clause_is_computable_at(root, rinfo->clause_relids, + if (!clause_is_computable_at(root, rinfo->group_clause_relids, both_input_relids)) continue; } diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c index c44bd2f815..16eadc8410 100644 --- a/src/backend/optimizer/util/restrictinfo.c +++ b/src/backend/optimizer/util/restrictinfo.c @@ -119,6 +119,7 @@ make_restrictinfo_internal(PlannerInfo *root, restrictinfo->can_join = false; /* may get set below */ restrictinfo->security_level = security_level; restrictinfo->outer_relids = outer_relids; + restrictinfo->group_clause_relids = NULL; /* * If it's potentially delayable by lower-level security quals, figure out diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index d61a62da19..33d84959f6 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -2553,6 +2553,12 @@ typedef struct RestrictInfo Relids left_relids pg_node_attr(equal_ignore); Relids right_relids pg_node_attr(equal_ignore); + /* + * Relids actually referenced in all the clauses from the same jointree + * node. This field is set only for clauses that have multiple versions. + */ + Relids group_clause_relids pg_node_attr(copy_as_scalar, equal_ignore, read_write_ignore); + /* * Modified clause with RestrictInfos. This field is NULL unless clause * is an OR clause. diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 5d59ed7890..824561c2eb 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -5173,6 +5173,34 @@ from int8_tbl t1 -> Seq Scan on onek t4 (13 rows) +-- another example of wrong outer join qual placement +explain (costs off) +select * +from int8_tbl t1 + left join (int8_tbl t2 left join onek t3 on t2.q1 > t3.unique1) + on t1.q2 = t2.q2 + left join onek t4 + on t2.q2 = 1 and t3.unique2 = 1 +where t1.q2 = coalesce(t2.q2,1); + QUERY PLAN +-------------------------------------------------------------- + Nested Loop Left Join + Join Filter: ((t2.q2 = 1) AND (t3.unique2 = 1)) + -> Nested Loop Left Join + -> Hash Left Join + Hash Cond: (t1.q2 = t2.q2) + Filter: (t1.q2 = COALESCE(t2.q2, '1'::bigint)) + -> Seq Scan on int8_tbl t1 + -> Hash + -> Seq Scan on int8_tbl t2 + -> Bitmap Heap Scan on onek t3 + Recheck Cond: (t2.q1 > unique1) + -> Bitmap Index Scan on onek_unique1 + Index Cond: (unique1 < t2.q1) + -> Materialize + -> Seq Scan on onek t4 +(15 rows) + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition explain (costs off) diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index a630f58b57..be1e03982e 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -1862,6 +1862,16 @@ from int8_tbl t1 left join onek t4 on t2.q2 < t3.unique2; +-- another example of wrong outer join qual placement +explain (costs off) +select * +from int8_tbl t1 + left join (int8_tbl t2 left join onek t3 on t2.q1 > t3.unique1) + on t1.q2 = t2.q2 + left join onek t4 + on t2.q2 = 1 and t3.unique2 = 1 +where t1.q2 = coalesce(t2.q2,1); + -- More tests of correct placement of pseudoconstant quals -- simple constant-false condition -- 2.31.0