Fix old bug in get_loop_count().

tglsfdc · tglsfdc · commit b746d0c32d4f · 2015-03-11T22:53:32.000-04:00
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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -1886,7 +1886,7 @@ get_loop_count(PlannerInfo *root, Index cur_relid, Relids outer_relids)`
`1886`	`1886`	`if (outer_relids == NULL)`
`1887`	`1887`	`return 1.0;`
`1888`	`1888`
`1889`		`- result = 1.0;`
	`1889`	`+ result = 0.0;`
`1890`	`1890`	`outer_relid = -1;`
`1891`	`1891`	`while ((outer_relid = bms_next_member(outer_relids, outer_relid)) >= 0)`
`1892`	`1892`	`{`
`@@ -1915,10 +1915,11 @@ get_loop_count(PlannerInfo *root, Index cur_relid, Relids outer_relids)`
`1915`	`1915`	`outer_rel->rows);`
`1916`	`1916`
`1917`	`1917`	`/* Remember smallest row count estimate among the outer rels */`
`1918`		`- if (result == 1.0 \|\| result > rowcount)`
	`1918`	`+ if (result == 0.0 \|\| result > rowcount)`
`1919`	`1919`	`result = rowcount;`
`1920`	`1920`	`}`
`1921`		`- return result;`
	`1921`	`+ /* Return 1.0 if we found no valid relations (shouldn't happen) */`
	`1922`	`+ return (result > 0.0) ? result : 1.0;`
`1922`	`1923`	`}`
`1923`	`1924`
`1924`	`1925`	`/*`