From c4d87c2acf0a6175b910e528874c6644d743069a Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 23 Sep 2025 12:23:16 -0700 Subject: [PATCH v3] Fix assertion failure in postgresGetForeignJoinPaths(). --- contrib/postgres_fdw/Makefile | 2 + .../expected/concurrent_update.out | 21 ++++++++ contrib/postgres_fdw/meson.build | 5 ++ .../postgres_fdw/specs/concurrent_update.spec | 54 +++++++++++++++++++ src/include/executor/execScan.h | 23 +++++--- 5 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 contrib/postgres_fdw/expected/concurrent_update.out create mode 100644 contrib/postgres_fdw/specs/concurrent_update.spec diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile index adfbd2ef758..f9018da9c1d 100644 --- a/contrib/postgres_fdw/Makefile +++ b/contrib/postgres_fdw/Makefile @@ -16,6 +16,8 @@ SHLIB_LINK_INTERNAL = $(libpq) EXTENSION = postgres_fdw DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.1--1.2.sql +ISOLATION = concurrent_update + REGRESS = postgres_fdw query_cancel TAP_TESTS = 1 diff --git a/contrib/postgres_fdw/expected/concurrent_update.out b/contrib/postgres_fdw/expected/concurrent_update.out new file mode 100644 index 00000000000..0d39c85edec --- /dev/null +++ b/contrib/postgres_fdw/expected/concurrent_update.out @@ -0,0 +1,21 @@ +unused step name: s1_commit +Parsed test spec with 2 sessions + +starting permutation: s0_begin s0_update s1_begin s1_lock s0_commit +step s0_begin: BEGIN ISOLATION LEVEL READ COMMITTED; +step s0_update: UPDATE sch1.a SET i = i + 1; +step s1_begin: BEGIN ISOLATION LEVEL READ COMMITTED; +step s1_lock: + SELECT a.i, + (SELECT 1 FROM sch1.b AS b, sch1.c AS c WHERE a.i = b.i AND b.i = c.i) + FROM sch1.a as a + FOR UPDATE; + +step s0_commit: COMMIT; +step s1_lock: <... completed> +i|?column? +-+-------- +2| +(1 row) + +teardown failed: \ No newline at end of file diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build index 5c11bc6496f..7ccc9107197 100644 --- a/contrib/postgres_fdw/meson.build +++ b/contrib/postgres_fdw/meson.build @@ -41,6 +41,11 @@ tests += { ], 'regress_args': ['--dlpath', meson.project_build_root() / 'src/test/regress'], }, + 'isolation': { + 'specs': [ + 'concurrent_update', + ], + }, 'tap': { 'tests': [ 't/001_auth_scram.pl', diff --git a/contrib/postgres_fdw/specs/concurrent_update.spec b/contrib/postgres_fdw/specs/concurrent_update.spec new file mode 100644 index 00000000000..b8c73378b56 --- /dev/null +++ b/contrib/postgres_fdw/specs/concurrent_update.spec @@ -0,0 +1,54 @@ +setup +{ + CREATE EXTENSION postgres_fdw; + DO $d$ + BEGIN + EXECUTE $$CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw + OPTIONS (dbname '$$||current_database()||$$', + port '$$||current_setting('port')||$$' + )$$; + END; + $d$; + CREATE USER MAPPING FOR public SERVER loopback; + + CREATE SCHEMA sch1; + CREATE TABLE sch1.a (i int); + CREATE FOREIGN TABLE sch1.b (i int) SERVER loopback OPTIONS (schema_name 'sch2', table_name 'b'); + CREATE FOREIGN TABLE sch1.c (i int) SERVER loopback OPTIONS (schema_name 'sch2', table_name 'c'); + + CREATE SCHEMA sch2; + CREATE TABLE sch2.b (i int); + CREATE TABLE sch2.c (i int); + + INSERT INTO sch1.a VALUES (1); + INSERT INTO sch2.b VALUES (1); + INSERT INTO sch2.c VALUES (1); +} + +teardown +{ +} + +session "s0" +step "s0_begin" { BEGIN ISOLATION LEVEL READ COMMITTED; } +step "s0_commit" { COMMIT; } +step "s0_update" { UPDATE sch1.a SET i = i + 1; } + + +session "s1" +step "s1_begin" { BEGIN ISOLATION LEVEL READ COMMITTED; } +step "s1_lock" { + SELECT a.i, + (SELECT 1 FROM sch1.b AS b, sch1.c AS c WHERE a.i = b.i AND b.i = c.i) + FROM sch1.a as a + FOR UPDATE; +} +step "s1_commit" { COMMIT; } + +# "s1_lock" waits for concurrent update on the tuple on sch1.a table by +# "s0_update", and once s0 transaction is committed it resumes and does EPQ +# recheck for the locked tuple, which should not use postgresRecheckForeignScan +# as the remote join is not a descendant join in the EPQ plan tree. +permutation "s0_begin" "s0_update" "s1_begin" "s1_lock" "s0_commit" + + diff --git a/src/include/executor/execScan.h b/src/include/executor/execScan.h index 837ea7785bb..95866e13ed4 100644 --- a/src/include/executor/execScan.h +++ b/src/include/executor/execScan.h @@ -49,16 +49,23 @@ ExecScanFetch(ScanState *node, { /* * This is a ForeignScan or CustomScan which has pushed down a - * join to the remote side. The recheck method is responsible not - * only for rechecking the scan/join quals but also for storing - * the correct tuple in the slot. + * join to the remote side. If it is a descendant node in the EPQ + * recheck plan tree, run the recheck method function. Otherwise, + * run the access method function below. */ + if (bms_is_member(epqstate->epqParam, node->ps.plan->extParam)) + { + /* + * The recheck method is responsible not only for rechecking + * the scan/join quals but also for storing the correct tuple + * in the slot. + */ + TupleTableSlot *slot = node->ss_ScanTupleSlot; - TupleTableSlot *slot = node->ss_ScanTupleSlot; - - if (!(*recheckMtd) (node, slot)) - ExecClearTuple(slot); /* would not be returned by scan */ - return slot; + if (!(*recheckMtd) (node, slot)) + ExecClearTuple(slot); /* would not be returned by scan */ + return slot; + } } else if (epqstate->relsubs_done[scanrelid - 1]) { -- 2.47.3