Subject: Re: BUG #17355: Server crashes on ExecReScanForeignScan in
 postgres_fdw when accessing foreign partition
To: Etsuro Fujita <etsuro.fujita@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
References: <17355-de8e362eb7001a96@postgresql.org>
 <CAPmGK14vYHWFLg=Lninw8W37gRsEzuwteemZfzkXqV8+g_qJ8A@mail.gmail.com>
 <9a8d0f3f-430d-024c-09f5-88a7519e9495@gmail.com>
 <CAPmGK16g9M1gBTQc2g6W4mYPcGdVWyJMcTbd=MB+huktgZciUg@mail.gmail.com>
 <CAPmGK16=dfQEJKthRHOXcP5QbzCP8T_fiJng6xdBrxeWSDwcYA@mail.gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
Message-ID: <c8b6f183-0cc1-feb4-8bfc-361c35983d8e@gmail.com>
Date: Mon, 24 Jan 2022 21:00:00 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: 
 <CAPmGK16=dfQEJKthRHOXcP5QbzCP8T_fiJng6xdBrxeWSDwcYA@mail.gmail.com>
Content-Type: multipart/mixed;
 boundary="------------EBA650673BDF677814060D7D"
Content-Language: en-US
Archived-At: 
 <https://www.postgresql.org/message-id/c8b6f183-0cc1-feb4-8bfc-361c35983d8e%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------EBA650673BDF677814060D7D
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Hello Etsura-san,
18.01.2022 11:01, Etsuro Fujita wrote:
> On Fri, Jan 7, 2022 at 3:19 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> On Thu, Jan 6, 2022 at 9:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
>>> 06.01.2022 12:56, Etsuro Fujita wrote:
>>>> I haven't tried to reproduce this yet
> I think a simple reproducer for this issue is:
>
> 1) Define the partitioned table async_pt as shown by Alexander.
> 2) Run concurrent transactions as follows.
>
> Session A: insert into async_pt values (3000);
> Session A: begin;
> Session A: update async_pt set a = a;
> Session B: delete from async_pt;
> Session A: commit;
>
> The commit in Session A would cause a server crash in Session B due to
> the segmentation fault.
>
> Also, I think a simple reproducer for the “cannot re-evaluate a
> Foreign Update or Delete during EvalPlanQual” error is:
>
> 1) Define the partitioned table async_pt as shown by Alexander.
> 2) Run concurrent transactions as follows.
>
> Session A: insert into async_pt values (3000);
> Session A: begin;
> Session A: update async_pt set a = a;
> Session B: update async_pt set a = a;
> Session A: commit;
>
> The commit in Session A would cause the transaction in Session B to
> abort with that error.
Thanks for the explanation and the fix!
I've made a simple isolation test (see attachment) to confirm this. And
it crashes without the fix as you said.
With your fix it works as expected (I was wondering how "plan->operation
!= CMD_SELECT" with RETURNING will work and I haven't seen any issues yet.)

(Besides that I've observed an infinite waiting for ShareLock with
step "s1i" { INSERT INTO pt VALUES (2000); }
This doesn't happen with a regular (not foreign) table.)

Best regards,
Alexander

--------------EBA650673BDF677814060D7D
Content-Type: text/x-patch; charset=UTF-8;
 name="test-for-bug-17355.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="test-for-bug-17355.patch"

diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index c1b0cad453f..02150ad8e2c 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -17,6 +17,7 @@ EXTENSION = postgres_fdw
 DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql
 
 REGRESS = postgres_fdw
+ISOLATION = bug-17355
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
diff --git a/contrib/postgres_fdw/expected/bug-17355.out b/contrib/postgres_fdw/expected/bug-17355.out
new file mode 100644
index 00000000000..1c829cd52c3
--- /dev/null
+++ b/contrib/postgres_fdw/expected/bug-17355.out
@@ -0,0 +1,27 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1i s1b s1u s2d s1c
+step s1i: INSERT INTO pt VALUES (3000);
+step s1b: BEGIN;
+step s1u: UPDATE pt SET a = a;
+step s2d: DELETE FROM pt RETURNING *; <waiting ...>
+step s1c: COMMIT;
+step s2d: <... completed>
+   a|b
+----+-
+3000| 
+(1 row)
+
+
+starting permutation: s1i s1b s1u s2u s1c
+step s1i: INSERT INTO pt VALUES (3000);
+step s1b: BEGIN;
+step s1u: UPDATE pt SET a = a;
+step s2u: UPDATE pt SET a = a RETURNING *; <waiting ...>
+step s1c: COMMIT;
+step s2u: <... completed>
+   a|b
+----+-
+3000| 
+(1 row)
+
diff --git a/contrib/postgres_fdw/specs/bug-17355.spec b/contrib/postgres_fdw/specs/bug-17355.spec
new file mode 100644
index 00000000000..b99aa50b052
--- /dev/null
+++ b/contrib/postgres_fdw/specs/bug-17355.spec
@@ -0,0 +1,42 @@
+setup
+{
+CREATE EXTENSION postgres_fdw;
+
+CREATE SERVER testserver1 FOREIGN DATA WRAPPER 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 CURRENT_USER SERVER loopback;
+CREATE TABLE pt (a int, b int) PARTITION BY RANGE (a);
+CREATE TABLE base_tbl2 (a int, b int);
+-- CREATE TABLE p2 PARTITION OF pt FOR VALUES FROM (2000) TO (3000);
+CREATE FOREIGN TABLE p2 PARTITION OF pt FOR VALUES FROM (2000) TO (3000)
+  SERVER loopback OPTIONS (table_name 'base_tbl2');
+CREATE TABLE p3 PARTITION OF pt FOR VALUES FROM (3000) TO
+(4000);
+}
+
+teardown
+{
+  DROP TABLE pt;
+  DROP TABLE base_tbl2;
+  DROP EXTENSION postgres_fdw CASCADE;
+}
+
+session "s1"
+step "s1i" { INSERT INTO pt VALUES (3000); }
+step "s1b" { BEGIN; }
+step "s1u" { UPDATE pt SET a = a; }
+step "s1c" { COMMIT; }
+
+session "s2"
+step "s2d"  { DELETE FROM pt RETURNING *; }
+step "s2u"  { UPDATE pt SET a = a RETURNING *; }
+
+permutation "s1i" "s1b" "s1u" "s2d" "s1c"
+permutation "s1i" "s1b" "s1u" "s2u" "s1c"

--------------EBA650673BDF677814060D7D--