Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1elZuM-0000G1-Bg for pgsql-hackers@arkaria.postgresql.org; Tue, 13 Feb 2018 12:42:02 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1elZuK-0006De-7v for pgsql-hackers@arkaria.postgresql.org; Tue, 13 Feb 2018 12:42:00 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1elZuK-0006DU-0x for pgsql-hackers@lists.postgresql.org; Tue, 13 Feb 2018 12:42:00 +0000 Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]) by magus.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1elZuF-0004TD-7a for pgsql-hackers@postgresql.org; Tue, 13 Feb 2018 12:41:59 +0000 Received: from vc1.ecl.ntt.co.jp (vc1.ecl.ntt.co.jp [129.60.86.153]) by tama500.ecl.ntt.co.jp (8.13.8/8.13.8) with ESMTP id w1DCfm0X007538; Tue, 13 Feb 2018 21:41:48 +0900 Received: from vc1.ecl.ntt.co.jp (localhost [127.0.0.1]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id 8E612EA8716; Tue, 13 Feb 2018 21:41:48 +0900 (JST) Received: from jcms-pop21.ecl.ntt.co.jp (jcms-pop21.ecl.ntt.co.jp [129.60.87.134]) by vc1.ecl.ntt.co.jp (Postfix) with ESMTP id 83CB4EA85CF; Tue, 13 Feb 2018 21:41:48 +0900 (JST) Received: from [IPv6:::1] (unknown [129.60.241.75]) by jcms-pop21.ecl.ntt.co.jp (Postfix) with ESMTPSA id 75F2140054D; Tue, 13 Feb 2018 21:41:48 +0900 (JST) Message-ID: <5A82DCCE.3060107@lab.ntt.co.jp> Date: Tue, 13 Feb 2018 21:40:46 +0900 From: Etsuro Fujita User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 MIME-Version: 1.0 Subject: Re: postgres_fdw: perform UPDATE/DELETE .. RETURNING on a join directly References: <5A438A45.2000909@lab.ntt.co.jp> <5A57193A.2080003@lab.ntt.co.jp> <6166.1518044513@sss.pgh.pa.us> <5A7BE8E6.200@lab.ntt.co.jp> <28606.1518105934@sss.pgh.pa.us> <5A7CFDDA.8050906@lab.ntt.co.jp> <5A7D76CF.8080601@lab.ntt.co.jp> <20134.1518201188@sss.pgh.pa.us> <18653.1518297871@sss.pgh.pa.us> In-Reply-To: <18653.1518297871@sss.pgh.pa.us> Content-Type: multipart/mixed; boundary="------------000608000105000104070908" X-CC-Mail-RelayStamp: 1 To: Tom Lane Cc: Robert Haas , Pg Hackers , Rushabh Lathia , Michael Paquier , Ashutosh Bapat X-TM-AS-MML: disable List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk This is a multi-part message in MIME format. --------------000608000105000104070908 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit (2018/02/11 6:24), Tom Lane wrote: > However, jaguarundi still shows a problem: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jaguarundi&dt=2018-02-10%2008%3A41%3A32 > > (previous run similar, so it's semi-reproducible even after this patch). > jaguarundi uses -DCLOBBER_CACHE_ALWAYS, so you might try a few repetitions > with that. I ran the postgres_fdw regression test with no sleep two times in a CLOBBER_CACHE_ALWAYS-enabled build, and then the regression test with the sleep (60 seconds) two times, but I couldn't reproduce that in both cases. I suspect the changes in the order of the RETURNING output there was still caused by autovacuum kicking in partway through the run. So to make the regression test more stable against autovacuum, I'd propose to modify the regression test to disable autovacuum for the remote table (ie "S 1"."T 1") (and perform VACUUM ANALYZE to that table manually instead) in hopes of getting that fixed. Attached is a patch for that. I think changes added by the previous patch wouldn't make sense anymore, so I removed those changes. Best regards, Etsuro Fujita --------------000608000105000104070908 Content-Type: text/x-diff; name="postgres-fdw-regress-2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="postgres-fdw-regress-2.patch" *** a/contrib/postgres_fdw/expected/postgres_fdw.out --- b/contrib/postgres_fdw/expected/postgres_fdw.out *************** *** 35,40 **** CREATE TABLE "S 1"."T 1" ( --- 35,43 ---- c8 user_enum, CONSTRAINT t1_pkey PRIMARY KEY ("C 1") ); + -- "S 1"."T 1" will be heavily updated below, so disable autovacuum for + -- the table to avoid unexpected effects of that + ALTER TABLE "S 1"."T 1" SET (autovacuum_enabled = 'false'); CREATE TABLE "S 1"."T 2" ( c1 int NOT NULL, c2 text, *************** *** 4244,4253 **** explain (verbose, costs off) select * from ft3 f, loct3 l -- =================================================================== -- test writable foreign table stuff -- =================================================================== - -- Autovacuum on the remote side might affect remote estimates, - -- so use local stats on ft2 as well - ALTER FOREIGN TABLE ft2 OPTIONS (SET use_remote_estimate 'false'); - ANALYZE ft2; EXPLAIN (verbose, costs off) INSERT INTO ft2 (c1,c2,c3) SELECT c1+1000,c2+100, c3 || c3 FROM ft2 LIMIT 20; QUERY PLAN --- 4247,4252 ---- *************** *** 5524,5555 **** UPDATE ft2 SET c3 = 'baz' FROM ft4 INNER JOIN ft5 ON (ft4.c1 = ft5.c1) WHERE ft2.c1 > 2000 AND ft2.c2 === ft4.c1 RETURNING ft2.*, ft4.*, ft5.*; -- can't be pushed down ! QUERY PLAN ! ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Update on public.ft2 Output: ft2.c1, ft2.c2, ft2.c3, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft4.c1, ft4.c2, ft4.c3, ft5.c1, ft5.c2, ft5.c3 Remote SQL: UPDATE "S 1"."T 1" SET c3 = $2 WHERE ctid = $1 RETURNING "C 1", c2, c3, c4, c5, c6, c7, c8 ! -> Hash Join Output: ft2.c1, ft2.c2, NULL::integer, 'baz'::text, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft2.ctid, ft4.*, ft5.*, ft4.c1, ft4.c2, ft4.c3, ft5.c1, ft5.c2, ft5.c3 ! Hash Cond: (ft4.c1 = ft5.c1) -> Foreign Scan ! Output: ft2.c1, ft2.c2, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft2.ctid, ft4.*, ft4.c1, ft4.c2, ft4.c3 ! Filter: (ft2.c2 === ft4.c1) ! Relations: (public.ft2) INNER JOIN (public.ft4) ! Remote SQL: SELECT r1."C 1", r1.c2, r1.c4, r1.c5, r1.c6, r1.c7, r1.c8, r1.ctid, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2.c1, r2.c2, r2.c3) END, r2.c1, r2.c2, r2.c3 FROM ("S 1"."T 1" r1 INNER JOIN "S 1"."T 3" r2 ON (((r1."C 1" > 2000)))) FOR UPDATE OF r1 ! -> Nested Loop ! Output: ft2.c1, ft2.c2, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft2.ctid, ft4.*, ft4.c1, ft4.c2, ft4.c3 ! -> Foreign Scan on public.ft2 ! Output: ft2.c1, ft2.c2, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft2.ctid ! Remote SQL: SELECT "C 1", c2, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" > 2000)) FOR UPDATE -> Foreign Scan on public.ft4 Output: ft4.*, ft4.c1, ft4.c2, ft4.c3 Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 3" ! -> Hash ! Output: ft5.*, ft5.c1, ft5.c2, ft5.c3 ! -> Foreign Scan on public.ft5 ! Output: ft5.*, ft5.c1, ft5.c2, ft5.c3 ! Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 4" (24 rows) UPDATE ft2 SET c3 = 'baz' --- 5523,5554 ---- FROM ft4 INNER JOIN ft5 ON (ft4.c1 = ft5.c1) WHERE ft2.c1 > 2000 AND ft2.c2 === ft4.c1 RETURNING ft2.*, ft4.*, ft5.*; -- can't be pushed down ! QUERY PLAN ! ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- Update on public.ft2 Output: ft2.c1, ft2.c2, ft2.c3, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft4.c1, ft4.c2, ft4.c3, ft5.c1, ft5.c2, ft5.c3 Remote SQL: UPDATE "S 1"."T 1" SET c3 = $2 WHERE ctid = $1 RETURNING "C 1", c2, c3, c4, c5, c6, c7, c8 ! -> Nested Loop Output: ft2.c1, ft2.c2, NULL::integer, 'baz'::text, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft2.ctid, ft4.*, ft5.*, ft4.c1, ft4.c2, ft4.c3, ft5.c1, ft5.c2, ft5.c3 ! Join Filter: (ft2.c2 === ft4.c1) ! -> Foreign Scan on public.ft2 ! Output: ft2.c1, ft2.c2, ft2.c4, ft2.c5, ft2.c6, ft2.c7, ft2.c8, ft2.ctid ! Remote SQL: SELECT "C 1", c2, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" > 2000)) FOR UPDATE -> Foreign Scan ! Output: ft4.*, ft4.c1, ft4.c2, ft4.c3, ft5.*, ft5.c1, ft5.c2, ft5.c3 ! Relations: (public.ft4) INNER JOIN (public.ft5) ! Remote SQL: SELECT CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2.c1, r2.c2, r2.c3) END, r2.c1, r2.c2, r2.c3, CASE WHEN (r3.*)::text IS NOT NULL THEN ROW(r3.c1, r3.c2, r3.c3) END, r3.c1, r3.c2, r3.c3 FROM ("S 1"."T 3" r2 INNER JOIN "S 1"."T 4" r3 ON (((r2.c1 = r3.c1)))) ! -> Hash Join ! Output: ft4.*, ft4.c1, ft4.c2, ft4.c3, ft5.*, ft5.c1, ft5.c2, ft5.c3 ! Hash Cond: (ft4.c1 = ft5.c1) -> Foreign Scan on public.ft4 Output: ft4.*, ft4.c1, ft4.c2, ft4.c3 Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 3" ! -> Hash ! Output: ft5.*, ft5.c1, ft5.c2, ft5.c3 ! -> Foreign Scan on public.ft5 ! Output: ft5.*, ft5.c1, ft5.c2, ft5.c3 ! Remote SQL: SELECT c1, c2, c3 FROM "S 1"."T 4" (24 rows) UPDATE ft2 SET c3 = 'baz' *************** *** 6003,6011 **** select c2, count(*) from "S 1"."T 1" where c2 < 500 group by 1 order by 1; 407 | 100 (13 rows) - -- Go back to use remote-estimate mode on ft2 VACUUM ANALYZE "S 1"."T 1"; - ALTER FOREIGN TABLE ft2 OPTIONS (SET use_remote_estimate 'true'); -- Above DMLs add data with c6 as NULL in ft1, so test ORDER BY NULLS LAST and NULLs -- FIRST behavior here. -- ORDER BY DESC NULLS LAST options --- 6002,6008 ---- *** a/contrib/postgres_fdw/sql/postgres_fdw.sql --- b/contrib/postgres_fdw/sql/postgres_fdw.sql *************** *** 39,44 **** CREATE TABLE "S 1"."T 1" ( --- 39,47 ---- c8 user_enum, CONSTRAINT t1_pkey PRIMARY KEY ("C 1") ); + -- "S 1"."T 1" will be heavily updated below, so disable autovacuum for + -- the table to avoid unexpected effects of that + ALTER TABLE "S 1"."T 1" SET (autovacuum_enabled = 'false'); CREATE TABLE "S 1"."T 2" ( c1 int NOT NULL, c2 text, *************** *** 1068,1078 **** explain (verbose, costs off) select * from ft3 f, loct3 l -- =================================================================== -- test writable foreign table stuff -- =================================================================== - -- Autovacuum on the remote side might affect remote estimates, - -- so use local stats on ft2 as well - ALTER FOREIGN TABLE ft2 OPTIONS (SET use_remote_estimate 'false'); - ANALYZE ft2; - EXPLAIN (verbose, costs off) INSERT INTO ft2 (c1,c2,c3) SELECT c1+1000,c2+100, c3 || c3 FROM ft2 LIMIT 20; INSERT INTO ft2 (c1,c2,c3) SELECT c1+1000,c2+100, c3 || c3 FROM ft2 LIMIT 20; --- 1071,1076 ---- *************** *** 1213,1221 **** commit; select c2, count(*) from ft2 where c2 < 500 group by 1 order by 1; select c2, count(*) from "S 1"."T 1" where c2 < 500 group by 1 order by 1; - -- Go back to use remote-estimate mode on ft2 VACUUM ANALYZE "S 1"."T 1"; - ALTER FOREIGN TABLE ft2 OPTIONS (SET use_remote_estimate 'true'); -- Above DMLs add data with c6 as NULL in ft1, so test ORDER BY NULLS LAST and NULLs -- FIRST behavior here. --- 1211,1217 ---- --------------000608000105000104070908--