From a784859f4480ceaa05a00ca35311071ca33483d1 Mon Sep 17 00:00:00 2001 From: Etsuro Fujita Date: Thu, 13 May 2021 20:00:00 +0900 Subject: [PATCH] Prevent asynchronous execution of direct foreign-table modifications. Commits 27e1f1456 and 86dc90056, which were independently discussed, cause a crash when executing an inherited foreign UPDATE/DELETE query with asynchronous execution enabled, where children of an Append node that is the direct/indirect child of the ModifyTable node are rewritten so as to modify foreign tables directly by postgresPlanDirectModify(); as in that case the direct modifications are executed asynchronously, which is not currently supported by asynchronous execution. Fix by disabling asynchronous execution of the direct modifications in that function. Author: Etsuro Fujita Reviewed-by: Amit Langote Discussion: https://postgr.es/m/CAPmGK158e9sJOfuWxfn%2B0ynrspXQU3JhNjSCbaoeSzMvnga%2Bbw%40mail.gmail.com --- .../postgres_fdw/expected/postgres_fdw.out | 55 +++++++++++++++++++ contrib/postgres_fdw/postgres_fdw.c | 7 +++ contrib/postgres_fdw/sql/postgres_fdw.sql | 8 +++ 3 files changed, 70 insertions(+) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 0b0c45f0d9a..7df30010f25 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -10154,6 +10154,61 @@ DROP TABLE base_tbl3; DROP TABLE base_tbl4; RESET enable_mergejoin; RESET enable_hashjoin; +-- Test that UPDATE/DELETE with inherited target works with async_capable enabled +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *; + QUERY PLAN +---------------------------------------------------------------------------------------------------------- + Update on public.async_pt + Output: async_pt_1.a, async_pt_1.b, async_pt_1.c + Foreign Update on public.async_p1 async_pt_1 + Foreign Update on public.async_p2 async_pt_2 + Update on public.async_p3 async_pt_3 + -> Append + -> Foreign Update on public.async_p1 async_pt_1 + Remote SQL: UPDATE public.base_tbl1 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c + -> Foreign Update on public.async_p2 async_pt_2 + Remote SQL: UPDATE public.base_tbl2 SET c = (c || c) WHERE ((b = 0)) RETURNING a, b, c + -> Seq Scan on public.async_p3 async_pt_3 + Output: (async_pt_3.c || async_pt_3.c), async_pt_3.tableoid, async_pt_3.ctid, NULL::record + Filter: (async_pt_3.b = 0) +(13 rows) + +UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *; + a | b | c +------+---+---------- + 1000 | 0 | 00000000 + 2000 | 0 | 00000000 + 3000 | 0 | 00000000 +(3 rows) + +EXPLAIN (VERBOSE, COSTS OFF) +DELETE FROM async_pt WHERE b = 0 RETURNING *; + QUERY PLAN +------------------------------------------------------------------------------------------ + Delete on public.async_pt + Output: async_pt_1.a, async_pt_1.b, async_pt_1.c + Foreign Delete on public.async_p1 async_pt_1 + Foreign Delete on public.async_p2 async_pt_2 + Delete on public.async_p3 async_pt_3 + -> Append + -> Foreign Delete on public.async_p1 async_pt_1 + Remote SQL: DELETE FROM public.base_tbl1 WHERE ((b = 0)) RETURNING a, b, c + -> Foreign Delete on public.async_p2 async_pt_2 + Remote SQL: DELETE FROM public.base_tbl2 WHERE ((b = 0)) RETURNING a, b, c + -> Seq Scan on public.async_p3 async_pt_3 + Output: async_pt_3.tableoid, async_pt_3.ctid + Filter: (async_pt_3.b = 0) +(13 rows) + +DELETE FROM async_pt WHERE b = 0 RETURNING *; + a | b | c +------+---+---------- + 1000 | 0 | 00000000 + 2000 | 0 | 00000000 + 3000 | 0 | 00000000 +(3 rows) + -- Check EXPLAIN ANALYZE for a query that scans empty partitions asynchronously DELETE FROM async_p1; DELETE FROM async_p2; diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index cb757d94045..c48a421e88b 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2530,6 +2530,13 @@ postgresPlanDirectModify(PlannerInfo *root, rebuild_fdw_scan_tlist(fscan, returningList); } + /* + * Finally, unset the async-capable flag if it is set, as we currently + * don't support asynchronous execution of direct modifications. + */ + if (fscan->scan.plan.async_capable) + fscan->scan.plan.async_capable = false; + table_close(rel, NoLock); return true; } diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 53adfe2abc8..78379bdea5b 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -3237,6 +3237,14 @@ DROP TABLE base_tbl4; RESET enable_mergejoin; RESET enable_hashjoin; +-- Test that UPDATE/DELETE with inherited target works with async_capable enabled +EXPLAIN (VERBOSE, COSTS OFF) +UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *; +UPDATE async_pt SET c = c || c WHERE b = 0 RETURNING *; +EXPLAIN (VERBOSE, COSTS OFF) +DELETE FROM async_pt WHERE b = 0 RETURNING *; +DELETE FROM async_pt WHERE b = 0 RETURNING *; + -- Check EXPLAIN ANALYZE for a query that scans empty partitions asynchronously DELETE FROM async_p1; DELETE FROM async_p2;