From bb403a2867bd8e33cfee10902d073a95f4431bbe Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 23 Sep 2025 12:23:16 -0700 Subject: [PATCH v6] postgres_fdw: Fix assertion failure when EvalPlanQual with a SubLink sub-select. Reported-by: Kristian Lejao Author: Masahiko Sawada Reviewed-by: Etsuro Fujita Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com --- src/include/executor/execScan.h | 23 +++++--- contrib/postgres_fdw/.gitignore | 1 + contrib/postgres_fdw/Makefile | 3 + .../postgres_fdw/expected/eval_plan_qual.out | 37 +++++++++++++ contrib/postgres_fdw/meson.build | 6 ++ .../postgres_fdw/specs/eval_plan_qual.spec | 55 +++++++++++++++++++ 6 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 contrib/postgres_fdw/expected/eval_plan_qual.out create mode 100644 contrib/postgres_fdw/specs/eval_plan_qual.spec diff --git a/src/include/executor/execScan.h b/src/include/executor/execScan.h index 837ea7785bb4..95866e13ed46 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]) { diff --git a/contrib/postgres_fdw/.gitignore b/contrib/postgres_fdw/.gitignore index 5dcb3ff97235..f84085454a0f 100644 --- a/contrib/postgres_fdw/.gitignore +++ b/contrib/postgres_fdw/.gitignore @@ -1,4 +1,5 @@ # Generated subdirectories /log/ +/output_iso/ /results/ /tmp_check/ diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile index adfbd2ef758e..81df6ca90857 100644 --- a/contrib/postgres_fdw/Makefile +++ b/contrib/postgres_fdw/Makefile @@ -16,6 +16,9 @@ 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 = eval_plan_qual +ISOLATION_OPTS = --load-extension=postgres_fdw + REGRESS = postgres_fdw query_cancel TAP_TESTS = 1 diff --git a/contrib/postgres_fdw/expected/eval_plan_qual.out b/contrib/postgres_fdw/expected/eval_plan_qual.out new file mode 100644 index 000000000000..b80ad17fc5ad --- /dev/null +++ b/contrib/postgres_fdw/expected/eval_plan_qual.out @@ -0,0 +1,37 @@ +Parsed test spec with 2 sessions + +starting permutation: s0_begin s0_update s1_begin s1_tuplock s0_commit s1_commit +step s0_begin: BEGIN ISOLATION LEVEL READ COMMITTED; +step s0_update: UPDATE a SET i = i + 1; +step s1_begin: BEGIN ISOLATION LEVEL READ COMMITTED; +step s1_tuplock: + -- Verify if the sub-select has a foreign-join + EXPLAIN (VERBOSE, COSTS OFF) + SELECT a.i, + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i) + FROM a FOR UPDATE; + SELECT a.i, + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i) + FROM a FOR UPDATE; + +step s0_commit: COMMIT; +step s1_tuplock: <... completed> +QUERY PLAN +---------------------------------------------------------------------------------------------------------------------------------------- +LockRows + Output: a.i, ((SubPlan 1)), a.ctid + -> Seq Scan on public.a + Output: a.i, (SubPlan 1), a.ctid + SubPlan 1 + -> Foreign Scan + Output: 1 + Relations: (public.fb) INNER JOIN (public.fc) + Remote SQL: SELECT NULL FROM (public.b r1 INNER JOIN public.c r2 ON (((r2.i = $1::integer)) AND ((r1.i = $1::integer)))) +(9 rows) + +i|?column? +-+-------- +2| +(1 row) + +step s1_commit: COMMIT; diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build index 5c11bc6496fa..aac89ffdde88 100644 --- a/contrib/postgres_fdw/meson.build +++ b/contrib/postgres_fdw/meson.build @@ -41,6 +41,12 @@ tests += { ], 'regress_args': ['--dlpath', meson.project_build_root() / 'src/test/regress'], }, + 'isolation': { + 'specs': [ + 'eval_plan_qual', + ], + 'regress_args': ['--load-extension=postgres_fdw'], + }, 'tap': { 'tests': [ 't/001_auth_scram.pl', diff --git a/contrib/postgres_fdw/specs/eval_plan_qual.spec b/contrib/postgres_fdw/specs/eval_plan_qual.spec new file mode 100644 index 000000000000..87d7790a6fda --- /dev/null +++ b/contrib/postgres_fdw/specs/eval_plan_qual.spec @@ -0,0 +1,55 @@ +# Tests for the EvalPlanQual mechanism involving foreign tables + +setup +{ + 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 TABLE a (i int); + CREATE TABLE b (i int); + CREATE TABLE c (i int); + CREATE FOREIGN TABLE fb (i int) SERVER loopback OPTIONS (table_name 'b'); + CREATE FOREIGN TABLE fc (i int) SERVER loopback OPTIONS (table_name 'c'); + + INSERT INTO a VALUES (1); + INSERT INTO b VALUES (1); + INSERT INTO c VALUES (1); +} + +teardown +{ + DROP TABLE a; + DROP TABLE b; + DROP TABLE c; + DROP SERVER loopback CASCADE; +} + +session s0 +step s0_begin { BEGIN ISOLATION LEVEL READ COMMITTED; } +step s0_commit { COMMIT; } +step s0_update { UPDATE a SET i = i + 1; } + +session s1 +step s1_begin { BEGIN ISOLATION LEVEL READ COMMITTED; } +step s1_tuplock { + -- Verify if the sub-select has a foreign-join + EXPLAIN (VERBOSE, COSTS OFF) + SELECT a.i, + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i) + FROM a FOR UPDATE; + SELECT a.i, + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i) + FROM a FOR UPDATE; +} +step s1_commit { COMMIT; } + +# This test exercises EvalPlanQual with a SubLink sub-select (which should +# be unaffected by any EPQ recheck behavior in the outer query). +permutation s0_begin s0_update s1_begin s1_tuplock s0_commit s1_commit -- 2.51.0