(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
