Re: Problems with plan estimates in postgres_fdw

(2019/03/20 20:47), Etsuro Fujita wrote:
> Attached is an updated version of the patch set.

One thing I noticed while self-reviewing the patch for UPPERREL_FINAL 
is: the previous versions of the patch don't show EPQ plans in EXPLAIN, 
as shown in the below example, so we can't check if those plans are 
constructed correctly, which I'll explain below:

* HEAD
EXPLAIN (VERBOSE, COSTS OFF)
SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY 
t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR UPDATE OF t1;
 
 
                                                                  QUERY 
PLAN 
 


-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Limit
    Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
    ->  LockRows
          Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
          ->  Foreign Scan
                Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
                Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
                Remote SQL: SELECT r1."C 1", r2."C 1", r1.c3, CASE WHEN 
(r1.*)::text IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, 
r1.c6, r1.c7, r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN 
ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM 
("S 1"."T 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) 
ORDER BY r1.c3 ASC NULLS LAST, r1."C 1" ASC NULLS LAST FOR UPDATE OF r1
-->            ->  Result
-->                  Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
                      ->  Sort
                            Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
                            Sort Key: t1.c3, t1.c1
                            ->  Merge Join
                                  Output: t1.c1, t1.c3, t1.*, t2.c1, t2.*
                                  Merge Cond: (t1.c1 = t2.c1)
                                  ->  Sort
                                        Output: t1.c1, t1.c3, t1.*
                                        Sort Key: t1.c1
                                        ->  Foreign Scan on public.ft1 t1
                                              Output: t1.c1, t1.c3, t1.*
                                              Remote SQL: SELECT "C 1", 
c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" FOR UPDATE
                                  ->  Sort
                                        Output: t2.c1, t2.*
                                        Sort Key: t2.c1
                                        ->  Foreign Scan on public.ft2 t2
                                              Output: t2.c1, t2.*
                                              Remote SQL: SELECT "C 1", 
c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
(28 rows)

* Patched
EXPLAIN (VERBOSE, COSTS OFF)
SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY 
t1.c3, t1.c1 OFFSET 100 LIMIT 10 FOR UPDATE OF t1;
 
 
 
       QUERY PLAN 
 
 


-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Foreign Scan
    Output: t1.c1, t2.c1, t1.c3, t1.*, t2.*
    Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
    Remote SQL: SELECT r1."C 1", r2."C 1", r1.c3, CASE WHEN (r1.*)::text 
IS NOT NULL THEN ROW(r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c7, 
r1.c8) END, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, 
r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END FROM ("S 1"."T 1" r1 INNER 
JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) ORDER BY r1.c3 ASC 
NULLS LAST, r1."C 1" ASC NULLS LAST LIMIT 10::bigint OFFSET 100::bigint 
FOR UPDATE OF r1
(4 rows)

In HEAD, this test case checks that a Result node is inserted atop of an 
EPQ plan for a foreign join (if necessary) when the foreign join is at 
the topmost join level (see discussion [1]), but the patched version 
doesn't show EPQ plans in EXPLAIN, so we can't check that as-is.  Should 
we show EPQ plans in EXPLAIN?  My inclination would be to not show them, 
because that information would be completely useless for the user.

Another thing I noticed is: the previous versions make pointless another 
test case added by commit 4bbf6edfb (and adjusted by commit 99f6a17dd) 
that checks an EPQ plan constructed from multiple merge joins, because 
they changed a query plan for that test case like the above.  (Actually, 
though, that test case doesn't need that EPQ plan already since it 
doesn't involve regular tables and it never fires EPQ rechecks.)  To 
avoid that, I modified that test case accordingly.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/message-id/8946.1544644803%40sss.pgh.pa.us