Fix hashjoin costing mistake introduced with inner_unique optimization.

tglsfdc · tglsfdc · commit 0bb28ca36eaf · 2018-07-14T11:59:12.000-04:00
In final_cost_hashjoin(), commit 9c7f522 allowed inner_unique cases to follow a code path previously used only for SEMI/ANTI joins; but it neglected to fix an if-test within that path that assumed SEMI and ANTI were the only possible cases. This resulted in a wrong value for hashjointuples, and an ensuing bad cost estimate, for inner_unique normal joins. Fortunately, for inner_unique normal joins we can assume the number of joined tuples is the same as for a SEMI join; so there's no need for more code, we just have to invert the test to check for ANTI not SEMI. It turns out that in two contrib tests in which commit 9c7f522 changed the plan expected for a query, the change was actually wrong and induced by this estimation error, not by any real improvement. Hence this patch also reverts those changes. Per report from RK Korlapati. Backpatch to v10 where the error was introduced. David Rowley Discussion: https://postgr.es/m/CA+SNy03bhq0fodsfOkeWDCreNjJVjsdHwUsb7AG=jpe0PtZc_g@mail.gmail.com
diff --git a/contrib/citext/expected/citext.out b/contrib/citext/expected/citext.out
@@ -2336,8 +2336,8 @@ SELECT *
   WHERE t.id IS NULL OR m.id IS NULL;
  id | name | id | name 
 ----+------+----+------
-  2 | two  |    | 
     |      |  2 | Two
+  2 | two  |    | 
 (2 rows)
 
 REFRESH MATERIALIZED VIEW CONCURRENTLY citext_matview;
diff --git a/contrib/citext/expected/citext_1.out b/contrib/citext/expected/citext_1.out
@@ -2336,8 +2336,8 @@ SELECT *
   WHERE t.id IS NULL OR m.id IS NULL;
  id | name | id | name 
 ----+------+----+------
-  2 | two  |    | 
     |      |  2 | Two
+  2 | two  |    | 
 (2 rows)
 
 REFRESH MATERIALIZED VIEW CONCURRENTLY citext_matview;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -4226,18 +4226,21 @@ explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
 
 explain (verbose, costs off) select * from ft3 f, loct3 l
   where f.f3 = l.f3 COLLATE "POSIX" and l.f1 = 'foo';
-                       QUERY PLAN                        
----------------------------------------------------------
- Nested Loop
+                         QUERY PLAN                          
+-------------------------------------------------------------
+ Hash Join
    Output: f.f1, f.f2, f.f3, l.f1, l.f2, l.f3
-   Join Filter: ((f.f3)::text = (l.f3)::text)
-   ->  Index Scan using loct3_f1_key on public.loct3 l
-         Output: l.f1, l.f2, l.f3
-         Index Cond: (l.f1 = 'foo'::text)
+   Inner Unique: true
+   Hash Cond: ((f.f3)::text = (l.f3)::text)
    ->  Foreign Scan on public.ft3 f
          Output: f.f1, f.f2, f.f3
          Remote SQL: SELECT f1, f2, f3 FROM public.loct3
-(9 rows)
+   ->  Hash
+         Output: l.f1, l.f2, l.f3
+         ->  Index Scan using loct3_f1_key on public.loct3 l
+               Output: l.f1, l.f2, l.f3
+               Index Cond: (l.f1 = 'foo'::text)
+(12 rows)
 
 -- ===================================================================
 -- test writable foreign table stuff
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
@@ -3180,10 +3180,10 @@ final_cost_hashjoin(PlannerInfo *root, HashPath *path,
 			clamp_row_est(inner_path_rows / virtualbuckets) * 0.05;
 
 		/* Get # of tuples that will pass the basic join */
-		if (path->jpath.jointype == JOIN_SEMI)
-			hashjointuples = outer_matched_rows;
-		else
+		if (path->jpath.jointype == JOIN_ANTI)
 			hashjointuples = outer_path_rows - outer_matched_rows;
+		else
+			hashjointuples = outer_matched_rows;
 	}
 	else
 	{

Original file line number	Diff line number	Diff line change
`@@ -3180,10 +3180,10 @@ final_cost_hashjoin(PlannerInfo root, HashPath path,`
`3180`	`3180`	`clamp_row_est(inner_path_rows / virtualbuckets) * 0.05;`
`3181`	`3181`
`3182`	`3182`	`/* Get # of tuples that will pass the basic join */`
`3183`		`- if (path->jpath.jointype == JOIN_SEMI)`
`3184`		`- hashjointuples = outer_matched_rows;`
`3185`		`- else`
	`3183`	`+ if (path->jpath.jointype == JOIN_ANTI)`
`3186`	`3184`	`hashjointuples = outer_path_rows - outer_matched_rows;`
	`3185`	`+ else`
	`3186`	`+ hashjointuples = outer_matched_rows;`
`3187`	`3187`	`}`
`3188`	`3188`	`else`
`3189`	`3189`	`{`