In clause_is_computable_at(), test required_relids for clone clauses.
authorTom Lane <[email protected]>
Sun, 21 May 2023 19:25:43 +0000 (15:25 -0400)
committerTom Lane <[email protected]>
Sun, 21 May 2023 19:25:52 +0000 (15:25 -0400)
Use the clause's required_relids not clause_relids for testing
whether it is computable at the current join level, if it is a
clone clause generated by deconstruct_distribute_oj_quals().

Arguably, this is more correct and we should do it for all clauses;
that would at least remove the handwavy claim that we are doing
it to save cycles compared to inspecting Vars individually.
However, attempting to do that exposes that we are not being careful
to compute an accurate value for required_relids in all cases.
I'm unsure whether it's a good idea to attempt to do that for v16,
or leave it as future clean-up.  In the meantime, this quick hack
demonstrably fixes some cases, so let's squeeze it in for beta1.

Patch by me, but great thanks to Richard Guo for investigation
and testing.  The new test cases are all modeled on his examples.

Discussion: https://postgr.es/m/CAMbWs4-_vwkBij4XOQ5ukxUvLgwTm0kS5_DO9CicUeKbEfKjUw@mail.gmail.com

src/backend/optimizer/util/restrictinfo.c
src/test/regress/expected/join.out
src/test/regress/sql/join.sql

index d2bc096e1cc2bebc48b9c4d8a01568fecf5c797d..760d24bebf3a94d0c76f684c72511e35c04ebee4 100644 (file)
@@ -544,13 +544,24 @@ clause_is_computable_at(PlannerInfo *root,
                        RestrictInfo *rinfo,
                        Relids eval_relids)
 {
-   Relids      clause_relids = rinfo->clause_relids;
+   Relids      clause_relids;
    ListCell   *lc;
 
    /* Nothing to do if no outer joins have been performed yet. */
    if (!bms_overlap(eval_relids, root->outer_join_rels))
        return true;
 
+   /*
+    * For an ordinary qual clause, we consider the actual clause_relids as
+    * explained above.  However, it's possible for multiple members of a
+    * group of clone quals to have the same clause_relids, so for clones use
+    * the required_relids instead to ensure we select just one of them.
+    */
+   if (rinfo->has_clone || rinfo->is_clone)
+       clause_relids = rinfo->required_relids;
+   else
+       clause_relids = rinfo->clause_relids;
+
    foreach(lc, root->join_info_list)
    {
        SpecialJoinInfo *sjinfo = (SpecialJoinInfo *) lfirst(lc);
index 8fa2b376f3aaca4792bc57c82ce56ac411190a0a..1fd75c8f5825b218f5b2b55f0339c069205a9110 100644 (file)
@@ -2500,6 +2500,74 @@ select * from int4_tbl t1
                      ->  Seq Scan on int4_tbl t4
 (12 rows)
 
+explain (costs off)
+select * from int4_tbl t1
+  left join (int4_tbl t2 left join int4_tbl t3 on t2.f1 > 0) on t2.f1 > 1
+  left join int4_tbl t4 on t2.f1 > 2 and t3.f1 > 3
+where t1.f1 = coalesce(t2.f1, 1);
+                     QUERY PLAN                     
+----------------------------------------------------
+ Nested Loop Left Join
+   Join Filter: ((t2.f1 > 2) AND (t3.f1 > 3))
+   ->  Nested Loop Left Join
+         Join Filter: (t2.f1 > 0)
+         ->  Nested Loop Left Join
+               Filter: (t1.f1 = COALESCE(t2.f1, 1))
+               ->  Seq Scan on int4_tbl t1
+               ->  Materialize
+                     ->  Seq Scan on int4_tbl t2
+                           Filter: (f1 > 1)
+         ->  Seq Scan on int4_tbl t3
+   ->  Materialize
+         ->  Seq Scan on int4_tbl t4
+(13 rows)
+
+explain (costs off)
+select * from int4_tbl t1
+  left join ((select t2.f1 from int4_tbl t2
+                left join int4_tbl t3 on t2.f1 > 0
+                where t3.f1 is null) s
+             left join tenk1 t4 on s.f1 > 1)
+    on s.f1 = t1.f1;
+                   QUERY PLAN                    
+-------------------------------------------------
+ Nested Loop Left Join
+   Join Filter: (t2.f1 > 1)
+   ->  Hash Right Join
+         Hash Cond: (t2.f1 = t1.f1)
+         ->  Nested Loop Left Join
+               Join Filter: (t2.f1 > 0)
+               Filter: (t3.f1 IS NULL)
+               ->  Seq Scan on int4_tbl t2
+               ->  Materialize
+                     ->  Seq Scan on int4_tbl t3
+         ->  Hash
+               ->  Seq Scan on int4_tbl t1
+   ->  Seq Scan on tenk1 t4
+(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 on t3.unique1 = t4.unique1 and t2.unique2 = t4.unique2;
+                               QUERY PLAN                               
+------------------------------------------------------------------------
+ Hash Left Join
+   Hash Cond: ((t3.unique1 = t4.unique1) AND (t2.unique2 = t4.unique2))
+   ->  Hash Left Join
+         Hash Cond: (t2.unique1 = t3.unique1)
+         ->  Hash Left Join
+               Hash Cond: (t1.unique1 = t2.unique1)
+               ->  Seq Scan on onek t1
+               ->  Hash
+                     ->  Seq Scan on onek t2
+         ->  Hash
+               ->  Seq Scan on onek t3
+   ->  Hash
+         ->  Seq Scan on onek t4
+(13 rows)
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys
index 641fd1a21b9f78cf9772f9569ff7b21d946e0eea..84547c7dffa150822c5ad663b435457222875852 100644 (file)
@@ -488,6 +488,26 @@ select * from int4_tbl t1
   left join int4_tbl t3 on t2.f1 = t3.f1
   left join int4_tbl t4 on t3.f1 != t4.f1;
 
+explain (costs off)
+select * from int4_tbl t1
+  left join (int4_tbl t2 left join int4_tbl t3 on t2.f1 > 0) on t2.f1 > 1
+  left join int4_tbl t4 on t2.f1 > 2 and t3.f1 > 3
+where t1.f1 = coalesce(t2.f1, 1);
+
+explain (costs off)
+select * from int4_tbl t1
+  left join ((select t2.f1 from int4_tbl t2
+                left join int4_tbl t3 on t2.f1 > 0
+                where t3.f1 is null) s
+             left join tenk1 t4 on s.f1 > 1)
+    on s.f1 = t1.f1;
+
+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 t2.unique2 = t4.unique2;
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys