Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 6 Oct 2025 12:37:21 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
	kristianlejao@gmail.com
Subject: Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
Message-ID: <aOM5cXaj24EE-ryO@paquier.xyz>
References: 
 <CAD21AoDv6ffWfVTBQgHGNR1rhywpKH246hJWiJvwBtPsGfxmJQ@mail.gmail.com>
 <CAPmGK164pycWPoMH8JeeWv0J8dF8kp=E89VJePYXh+Fhm+H0Hg@mail.gmail.com>
 <aMtRUQqnowiYFIha@paquier.xyz>
 <CAD21AoCD=Fg6cx9HQGY1JUy+ZVd5_k_zTqPn7HjPPL2CjncbHg@mail.gmail.com>
 <CAPmGK15rRYceZQ5DHcXXxMQLSU9r2CW=wXJc8+w76gkrkuFamw@mail.gmail.com>
 <CAPmGK14uTcaer74Cr5n62i0Lis9bnbX4F+YKrttrLq0Zj9+Eow@mail.gmail.com>
 <CAD21AoALtwy43TYR7tXoOPHKEPkutagYYY1TmbNzoR_6+fzA1g@mail.gmail.com>
 <CAPmGK16fkr0QhnV0dvX_car_tx9fe2JGTyWpH3zG0N41w-vqVQ@mail.gmail.com>
 <CAD21AoDbY_nYAas_cfR1PQ-ZrN+mBCkYWHAoe6p1Q3E_7=PJfg@mail.gmail.com>
 <CAPmGK17eOxqzi8YQDh-q2myyS_zvd602iBEV_fV=byzskYZygg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="czw/LW3xU5bI+ZFQ"
Content-Disposition: inline
In-Reply-To: 
 <CAPmGK17eOxqzi8YQDh-q2myyS_zvd602iBEV_fV=byzskYZygg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aOM5cXaj24EE-ryO%40paquier.xyz>
Precedence: bulk


--czw/LW3xU5bI+ZFQ
Content-Type: multipart/mixed; boundary="H1oKjlqe5SrUrfjW"
Content-Disposition: inline


--H1oKjlqe5SrUrfjW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Oct 06, 2025 at 02:15:25AM +0900, Etsuro Fujita wrote:
> The isolation test fails.  I think you failed to update the expected
> file.  Also, you left the useless alias for table a alone...

Yep, the EXPLAIN plan was missing.  Another thing that you have missed
is to update postgres_fdw's .gitignore so as it discards /output_iso/.

The updated patch attached, labelled v6, addresses all that.
--
Michael

--H1oKjlqe5SrUrfjW
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v6-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch"
Content-Transfer-Encoding: quoted-printable

=46rom bb403a2867bd8e33cfee10902d073a95f4431bbe Mon Sep 17 00:00:00 2001
=46rom: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 23 Sep 2025 12:23:16 -0700
Subject: [PATCH v6] postgres_fdw: Fix assertion failure when EvalPlanQual w=
ith
 a SubLink sub-select.

Reported-by: Kristian Lejao <kristianlejao@gmail.com>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CAD21AoBpo6Gx55FBOW+9s5X=3DnUw3Xpq64v35fpDE=
KsTERnc4TQ@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/execSca=
n.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 =3D node->ss_ScanTupleSlot;
=20
-			TupleTableSlot *slot =3D 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/.gitign=
ore
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 =3D $(libpq)
 EXTENSION =3D postgres_fdw
 DATA =3D postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.=
1--1.2.sql
=20
+ISOLATION =3D eval_plan_qual
+ISOLATION_OPTS =3D --load-extension=3Dpostgres_fdw
+
 REGRESS =3D postgres_fdw query_cancel
 TAP_TESTS =3D 1
=20
diff --git a/contrib/postgres_fdw/expected/eval_plan_qual.out b/contrib/pos=
tgres_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 =3D i + 1;
+step s1_begin: BEGIN ISOLATION LEVEL READ COMMITTED;
+step s1_tuplock:=20
+    -- Verify if the sub-select has a foreign-join
+    EXPLAIN (VERBOSE, COSTS OFF)
+    SELECT a.i,
+       (SELECT 1 FROM fb, fc WHERE a.i =3D fb.i AND fb.i =3D fc.i)
+    FROM a FOR UPDATE;
+    SELECT a.i,
+       (SELECT 1 FROM fb, fc WHERE a.i =3D fb.i AND fb.i =3D fc.i)
+    FROM a FOR UPDATE;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock: <... completed>
+QUERY PLAN                                                                =
                                                             =20
+--------------------------------------------------------------------------=
--------------------------------------------------------------
+LockRows                                                                  =
                                                             =20
+  Output: a.i, ((SubPlan 1)), a.ctid                                      =
                                                             =20
+  ->  Seq Scan on public.a                                                =
                                                             =20
+        Output: a.i, (SubPlan 1), a.ctid                                  =
                                                             =20
+        SubPlan 1                                                         =
                                                             =20
+          ->  Foreign Scan                                                =
                                                             =20
+                Output: 1                                                 =
                                                             =20
+                Relations: (public.fb) INNER JOIN (public.fc)             =
                                                             =20
+                Remote SQL: SELECT NULL FROM (public.b r1 INNER JOIN publi=
c.c r2 ON (((r2.i =3D $1::integer)) AND ((r1.i =3D $1::integer))))
+(9 rows)
+
+i|?column?
+-+--------
+2|       =20
+(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 +=3D {
     ],
     'regress_args': ['--dlpath', meson.project_build_root() / 'src/test/re=
gress'],
   },
+  'isolation': {
+    'specs': [
+      'eval_plan_qual',
+    ],
+    'regress_args': ['--load-extension=3Dpostgres_fdw'],
+  },
   'tap': {
     'tests': [
       't/001_auth_scram.pl',
diff --git a/contrib/postgres_fdw/specs/eval_plan_qual.spec b/contrib/postg=
res_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 =3D 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 =3D fb.i AND fb.i =3D fc.i)
+    FROM a FOR UPDATE;
+    SELECT a.i,
+       (SELECT 1 FROM fb, fc WHERE a.i =3D fb.i AND fb.i =3D 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
--=20
2.51.0


--H1oKjlqe5SrUrfjW--

--czw/LW3xU5bI+ZFQ
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmjjOXEACgkQnvQgOdby
QH3kVQ//cnSMxp93ID952Ji+stlrhmyu8VJ2SBLXWN4ceGGk03jk0bqUU/lzIG1t
l3lDUQAZ30O//VenhW0h5dmKUSJZA1DFGlJVm+ij1oD3pyPCJOwso/Rg2vfcGV/k
1qqSZSQ+ynCNw/IyvTwOgZO6FLTvMmcqpuF+JIdRgtAqrhY/tvwe1my9qF2ezzNx
NBp8JZFLE2zDE6ykYnQR2AIU4cjuUKl5qEftoLkkvkkgT/W62+MegcKid3TLbBWj
LgemoValySYMSpDKfwhFq75qTLdOQO6HpYL67u9DOXE1mohhr5bGBj+v9M5TvspP
WMYFWqBolgj25/kIPBqBZgNkiLr8ZQv2qainFKvUOaSU1eLivAtnABgp7xtjjw1O
zbl7YZcDb9BifXVrF3izz5bwPDzAGNsvJIf8cSXcztK5ur/1OcbrbejVEn29rNAd
mRwa/siWKTdwj+8HILoFJQFL97Wr0PbLQmqbbYZnVQGVrXOjEOvvfYgYQBIDVo1H
xEy7IgItCxchm3wmwctg+nBLDi+lQha+VGGzHOQE1+n3sszP3XNHAhqVGN3SNHnK
pUaoZaHbNTefq72BhrVeV23xudG6506kI3hjRouLCg05rWv5zYi/6UFzXqNojHgE
8GAMKtel5y96Igcok8dOy1P4L0hcGL6a9Q0j7KhOqsjlnHfkgMo=
=pZ+Z
-----END PGP SIGNATURE-----

--czw/LW3xU5bI+ZFQ--