postgres_fdw: Fix assertion in estimate_path_cost_size().

Etsuro Fujita · Etsuro Fujita · commit 69f75bf825f8 · 2022-12-15T21:15:00.000+09:00
Commit 08d2d58 added this assertion assuming that the retrieved_rows estimate for a foreign relation is set to at least one row in estimate_path_cost_size(), but the assumption isn't correct because if the relation is a foreign table with tuples=0, the estimate would be set to 0 in there when using local stats, and if the query's WHERE clause has a NULL condition, the estimate could be set to 0 in there when using remote estimates. (Note: even in the latter case the assertion could be reachable when costing foreign final paths.) Repair. Per bug #17713 from Robins Tharakan. This patch was already applied to v13 or later; apply it to v12 as well where the aforementioned commit was added. Thanks to Richard Guo for investigation. Discussion: http://postgr.es/m/17713-92cce66de7e81c04%40postgresql.org Discussion: http://postgr.es/m/CAEP4nAza%2B0fTCLkgkKYux3JDo3tUBTQORehP%2BaCxSNURpSFpHw%40mail.gmail.com
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -590,6 +590,24 @@ SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 full join ft1 t2 full join ft2
 
 RESET enable_hashjoin;
 RESET enable_nestloop;
+-- Test executing assertion in estimate_path_cost_size() that makes sure that
+-- retrieved_rows for foreign rel re-used to cost pre-sorted foreign paths is
+-- a sensible value even when the rel has tuples=0
+CREATE TABLE loct_empty (c1 int NOT NULL, c2 text);
+CREATE FOREIGN TABLE ft_empty (c1 int NOT NULL, c2 text)
+  SERVER loopback OPTIONS (table_name 'loct_empty');
+INSERT INTO loct_empty
+  SELECT id, 'AAA' || to_char(id, 'FM000') FROM generate_series(1, 100) id;
+DELETE FROM loct_empty;
+ANALYZE ft_empty;
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft_empty ORDER BY c1;
+                                  QUERY PLAN                                   
+-------------------------------------------------------------------------------
+ Foreign Scan on public.ft_empty
+   Output: c1, c2
+   Remote SQL: SELECT c1, c2 FROM public.loct_empty ORDER BY c1 ASC NULLS LAST
+(3 rows)
+
 -- ===================================================================
 -- WHERE with remotely-executable conditions
 -- ===================================================================
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
@@ -2749,7 +2749,7 @@ estimate_path_cost_size(PlannerInfo *root,
 		 */
 		if (fpinfo->rel_startup_cost >= 0 && fpinfo->rel_total_cost >= 0)
 		{
-			Assert(fpinfo->retrieved_rows >= 1);
+			Assert(fpinfo->retrieved_rows >= 0);
 
 			rows = fpinfo->rows;
 			retrieved_rows = fpinfo->retrieved_rows;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -285,6 +285,18 @@ SELECT t1."C 1", t2.c1, t3.c1 FROM "S 1"."T 1" t1 full join ft1 t2 full join ft2
 RESET enable_hashjoin;
 RESET enable_nestloop;
 
+-- Test executing assertion in estimate_path_cost_size() that makes sure that
+-- retrieved_rows for foreign rel re-used to cost pre-sorted foreign paths is
+-- a sensible value even when the rel has tuples=0
+CREATE TABLE loct_empty (c1 int NOT NULL, c2 text);
+CREATE FOREIGN TABLE ft_empty (c1 int NOT NULL, c2 text)
+  SERVER loopback OPTIONS (table_name 'loct_empty');
+INSERT INTO loct_empty
+  SELECT id, 'AAA' || to_char(id, 'FM000') FROM generate_series(1, 100) id;
+DELETE FROM loct_empty;
+ANALYZE ft_empty;
+EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft_empty ORDER BY c1;
+
 -- ===================================================================
 -- WHERE with remotely-executable conditions
 -- ===================================================================

Original file line number	Diff line number	Diff line change
`@@ -2749,7 +2749,7 @@ estimate_path_cost_size(PlannerInfo *root,`
`2749`	`2749`	`*/`
`2750`	`2750`	`if (fpinfo->rel_startup_cost >= 0 && fpinfo->rel_total_cost >= 0)`
`2751`	`2751`	`{`
`2752`		`- Assert(fpinfo->retrieved_rows >= 1);`
	`2752`	`+ Assert(fpinfo->retrieved_rows >= 0);`
`2753`	`2753`
`2754`	`2754`	`rows = fpinfo->rows;`
`2755`	`2755`	`retrieved_rows = fpinfo->retrieved_rows;`