Fix old bug in get_loop_count().

While poking at David Kubečka's issue I noticed an ancient logic error
in get_loop_count(): it used 1.0 as a "no data yet" indicator, but since
that is actually a valid rowcount estimate, this doesn't work.  If we
have one input relation with 1.0 as rowcount and then another one with
a larger rowcount, we should use 1.0 as the result, but we picked the
larger rowcount instead.  (I think when I coded this, I recognized the
conflict, but mistakenly thought that the logic would pick the desired
count anyway.)

Fixing this changed the plan for one existing regression test case.
Since the point of that test is to exercise creation of a particular
shape of nestloop plan, I tweaked the query a little bit so it still
results in the same plan choice.

This is definitely a bug, but I'm hesitant to back-patch since it might
change plan choices unexpectedly, and anyway failure to implement a
heuristic precisely as intended is a pretty low-grade bug.

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index da4d7c528455f9732f10ab0fe813631af2f7aef1..49ab3666b93b881c5895c319dd0fd1be3da80b88 100644 (file)
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1886,7 +1886,7 @@ get_loop_count(PlannerInfo *root, Index cur_relid, Relids outer_relids)
    if (outer_relids == NULL)
        return 1.0;
 
-   result = 1.0;
+   result = 0.0;
    outer_relid = -1;
    while ((outer_relid = bms_next_member(outer_relids, outer_relid)) >= 0)
    {
@@ -1915,10 +1915,11 @@ get_loop_count(PlannerInfo *root, Index cur_relid, Relids outer_relids)
                                                 outer_rel->rows);
 
        /* Remember smallest row count estimate among the outer rels */
-       if (result == 1.0 || result > rowcount)
+       if (result == 0.0 || result > rowcount)
            result = rowcount;
    }
-   return result;
+   /* Return 1.0 if we found no valid relations (shouldn't happen) */
+   return (result > 0.0) ? result : 1.0;
 }
 
 /*

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 1e3fe073504225e8528bc3f456199002ebcdfc83..8c17ef41b9204be3250f485e756af6bfd198267e 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3074,14 +3074,14 @@ select f1, unique2, case when unique2 is null then f1 else 0 end
 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 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+  where a.unique2 < 10 and coalesce(b.twothousand, a.twothousand) = 44;
                                          QUERY PLAN                                          
 ---------------------------------------------------------------------------------------------
  Nested Loop Left Join
    ->  Nested Loop Left Join
          Filter: (COALESCE(b.twothousand, a.twothousand) = 44)
          ->  Index Scan using tenk1_unique2 on tenk1 a
-               Index Cond: (unique2 = 5530)
+               Index Cond: (unique2 < 10)
          ->  Bitmap Heap Scan on tenk1 b
                Recheck Cond: (thousand = a.unique1)
                ->  Bitmap Index Scan on tenk1_thous_tenthous
@@ -3092,7 +3092,7 @@ select a.unique1, b.unique1, c.unique1, coalesce(b.twothousand, a.twothousand)
 
 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 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+  where a.unique2 < 10 and coalesce(b.twothousand, a.twothousand) = 44;
  unique1 | unique1 | unique1 | coalesce 
 ---------+---------+---------+----------
 (0 rows)

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 7a08bdff7bfb0e159e35fc10dd8d5ee0069fd74a..db7aefee28385da373024c866c01dd3fc3b08a4a 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -880,11 +880,11 @@ select f1, unique2, case when unique2 is null then f1 else 0 end
 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 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+  where a.unique2 < 10 and coalesce(b.twothousand, a.twothousand) = 44;
 
 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 = 5530 and coalesce(b.twothousand, a.twothousand) = 44;
+  where a.unique2 < 10 and coalesce(b.twothousand, a.twothousand) = 44;
 
 --
 -- check handling of join aliases when flattening multiple levels of subquery