pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedTRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
42+ messages / 4 participants
[nested] [flat]
* TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-05 18:00 Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 2 replies; 42+ messages in thread
From: Masahiko Sawada @ 2025-08-05 18:00 UTC (permalink / raw)
To: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; +Cc: kristianlejao@gmail.com
Hi all,
Kristian Lejao (colleague, in CC) has found the following assertion
failure in postgres_fdw.c when rechecking the result tuple via
EvalPlanQual():
TRAP: failed Assert("outerPlan != NULL"), File: "postgres_fdw.c",
Line: 2366, PID: 2043518
Here is the reproducible steps that I've simplified from the one
Kristian originally created:
1. setup local node:
create extension postgres_fdw;
create server srv foreign data wrapper postgres_fdw options (host
'localhost', port '5433', dbname 'postgres');
create user mapping for public server srv;
create table a (i int primary key);
create foreign table b (i int) server srv;
create foreign table c (i int) server srv;
insert into a values (1);
2. setup remote node:
create table b (i int);
create table c (i int);
insert into b values (1);
insert into c values (1);
3. attach to the backend process started on the local node (say conn1)
using gdb and set breakpoint at table_tuple_lock().
4. run the following query on conn1 (which stops before locking the
result tuple):
select a.i,
(select 1 from b, c where a.i = b.i and b.i = c.i)
from a
for update;
5. on another session, update the tuple concurrently:
update a set i = i + 1; -- update 1 tuple
6. continue the query on conn1, the server crashes due to the assertion failure.
The plan of the FOR UPDATE query lead to this issue is:
QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------
LockRows (cost=0.00..615886.00 rows=2550 width=14)
Output: a.i, ((SubPlan 1)), a.ctid
-> Seq Scan on public.a (cost=0.00..615860.50 rows=2550 width=14)
Output: a.i, (SubPlan 1), a.ctid
SubPlan 1
-> Foreign Scan (cost=100.00..241.50 rows=225 width=4)
Output: 1
Relations: (public.b) INNER JOIN (public.c)
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)
The point is that in the subquery in the target list we pushed the
inner join to the foreign server. In postgresGetForeignJoinPaths(), we
prepare the join path for EvalPlanQual() check (and used in
postgresRecheckForeignScan()) if the query is DELETE, UPDATE, or FOR
UPDATE/SHARE (as shown below) but we skip it since the subquery itself
is parsed as a normal SELECT query without rowMarks, leaving
fdw_outerpath of the ForeignScan node NULL:
/*
* If there is a possibility that EvalPlanQual will be executed, we need
* to be able to reconstruct the row using scans of the base relations.
* GetExistingLocalJoinPath will find a suitable path for this purpose in
* the path list of the joinrel, if one exists. We must be careful to
* call it before adding any ForeignPath, since the ForeignPath might
* dominate the only suitable local path available. We also do it before
* calling foreign_join_ok(), since that function updates fpinfo and marks
* it as pushable if the join is found to be pushable.
*/
if (root->parse->commandType == CMD_DELETE ||
root->parse->commandType == CMD_UPDATE ||
root->rowMarks)
{
epq_path = GetExistingLocalJoinPath(joinrel);
Therefore, if the tuple is concurrently updated before taking a lock,
we recheck the traversed tuple via EvalPlanQual() but we end up with
the assertion failure since we didn't prepare the join plan for that.
The attached patch includes the draft fix and regression tests (using
injection points).
I don't have enough experience with the planner and FDW code area to
evaluate whether the patch fixes the issue in the right approach.
Feedback is very welcome. I've confirmed this assertion could happen
with the same scenario on all supported branches.
In addition to that, I realized that none of the regression tests
execute postgresRecheckForeignScan()[1]. I think we need to add
regression tests to cover that function.
Regards,
[1] https://coverage.postgresql.org/contrib/postgres_fdw/postgres_fdw.c.gcov.html#2354()
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/x-patch] 0001-Fix-assertion-failure-in-postgresGetForeignJoinPaths.patch (5.6K, ../../CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com/2-0001-Fix-assertion-failure-in-postgresGetForeignJoinPaths.patch)
download | inline diff:
From fc40c46eb78f5659e77ed37aa0bc70aef6cfd48e Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 4 Aug 2025 22:32:05 +0000
Subject: [PATCH] Fix assertion failure in postgresGetForeignJoinPaths().
---
contrib/postgres_fdw/Makefile | 4 +
contrib/postgres_fdw/postgres_fdw.c | 16 +++-
contrib/postgres_fdw/t/002_foreign_recheck.pl | 94 +++++++++++++++++++
src/backend/access/heap/heapam_handler.c | 3 +
4 files changed, 114 insertions(+), 3 deletions(-)
create mode 100644 contrib/postgres_fdw/t/002_foreign_recheck.pl
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index adfbd2ef758..8b88e42bb76 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -19,6 +19,10 @@ DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.1--1.2.s
REGRESS = postgres_fdw query_cancel
TAP_TESTS = 1
+EXTRA_INSTALL=src/test/modules/injection_points
+
+export enable_injection_points
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 456b267f70b..23c1b956048 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6276,6 +6276,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
Cost total_cost;
Path *epq_path; /* Path to create plan to be executed when
* EvalPlanQual gets triggered. */
+ bool need_epq = false;
/*
* Skip if this join combination has been considered already.
@@ -6313,9 +6314,18 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
* calling foreign_join_ok(), since that function updates fpinfo and marks
* it as pushable if the join is found to be pushable.
*/
- if (root->parse->commandType == CMD_DELETE ||
- root->parse->commandType == CMD_UPDATE ||
- root->rowMarks)
+ for (PlannerInfo *proot = root; proot != NULL; proot = proot->parent_root)
+ {
+ if (proot->parse->commandType == CMD_DELETE ||
+ proot->parse->commandType == CMD_UPDATE ||
+ proot->rowMarks)
+ {
+ need_epq = true;
+ break;
+ }
+ }
+
+ if (need_epq)
{
epq_path = GetExistingLocalJoinPath(joinrel);
if (!epq_path)
diff --git a/contrib/postgres_fdw/t/002_foreign_recheck.pl b/contrib/postgres_fdw/t/002_foreign_recheck.pl
new file mode 100644
index 00000000000..943c65b3bca
--- /dev/null
+++ b/contrib/postgres_fdw/t/002_foreign_recheck.pl
@@ -0,0 +1,94 @@
+# Copyright (c) 2024-2025, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+my $node1 = PostgreSQL::Test::Cluster->new('node1');
+my $node2 = PostgreSQL::Test::Cluster->new('node2');
+$node1->init;
+$node2->init;
+$node1->start;
+$node2->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node1->check_extension('injection_points'))
+{
+ plan skip_all => 'Extension injection_points not installed';
+}
+
+$node1->safe_psql('postgres',
+ qq[
+create extension postgres_fdw;
+create extension injection_points;
+]);
+
+my $host = $node2->host;
+my $port = $node2->port;
+$node1->safe_psql('postgres',
+ qq[
+create server node2 foreign data wrapper postgres_fdw options (
+ host '$host', port '$port', dbname 'postgres');
+create user mapping for public server node2;
+
+create table a (i int primary key);
+create foreign table b (i int) server node2;
+create foreign table c (i int) server node2;
+]);
+
+$node2->safe_psql('postgres',
+ qq[
+create table b (i int);
+create table c (i int);
+]);
+
+$node1->safe_psql('postgres',
+ qq[
+insert into a values (1);
+insert into b values (1);
+insert into c values (1);
+]);
+
+my $psql_session = $node1->background_psql('postgres');
+
+$psql_session->query_safe(qq[
+ select injection_points_set_local();
+ select injection_points_attach('heapam_lock_tuple-before-lock', 'wait');
+]);
+
+# Execute the query and stops before locking the result tuple.
+$psql_session->query_until(
+ qr/lock_tuple/, q(
+\echo lock_tuple
+select
+ (select a.i from b, c where a.i = b.i and b.i = c.i)
+from a
+for update;
+));
+$node1->wait_for_event('client backend', 'heapam_lock_tuple-before-lock');
+
+# Update the tupel concurrently
+$node1->safe_psql('postgres',
+ qq[update a set i = i + 1]);
+
+# Wake up the FOR UPDATE query. The query detects the concurrent update and
+# performs recheck using EvalPlanQual().
+$node1->safe_psql('postgres',
+ qq[select injection_points_wakeup('heapam_lock_tuple-before-lock')]);
+
+ok($psql_session->quit);
+
+$node1->stop;
+$node2->stop;
+done_testing();
+
+
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index cb4bc35c93e..45b1cafc01d 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -44,6 +44,7 @@
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/rel.h"
static void reform_and_rewrite_tuple(HeapTuple tuple,
@@ -375,6 +376,8 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
Assert(TTS_IS_BUFFERTUPLE(slot));
+ INJECTION_POINT("heapam_lock_tuple-before-lock", NULL);
+
tuple_lock_retry:
tuple->t_self = *tid;
result = heap_lock_tuple(relation, tuple, cid, mode, wait_policy,
--
2.47.3
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-06 11:30 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-08-06 11:30 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
Sawada-san,
On Wed, Aug 6, 2025 at 3:01 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Kristian Lejao (colleague, in CC) has found the following assertion
> failure in postgres_fdw.c when rechecking the result tuple via
> EvalPlanQual():
>
> TRAP: failed Assert("outerPlan != NULL"), File: "postgres_fdw.c",
> Line: 2366, PID: 2043518
>
> Here is the reproducible steps that I've simplified from the one
> Kristian originally created:
[...]
> The point is that in the subquery in the target list we pushed the
> inner join to the foreign server. In postgresGetForeignJoinPaths(), we
> prepare the join path for EvalPlanQual() check (and used in
> postgresRecheckForeignScan()) if the query is DELETE, UPDATE, or FOR
> UPDATE/SHARE (as shown below) but we skip it since the subquery itself
> is parsed as a normal SELECT query without rowMarks, leaving
> fdw_outerpath of the ForeignScan node NULL:
>
> /*
> * If there is a possibility that EvalPlanQual will be executed, we need
> * to be able to reconstruct the row using scans of the base relations.
> * GetExistingLocalJoinPath will find a suitable path for this purpose in
> * the path list of the joinrel, if one exists. We must be careful to
> * call it before adding any ForeignPath, since the ForeignPath might
> * dominate the only suitable local path available. We also do it before
> * calling foreign_join_ok(), since that function updates fpinfo and marks
> * it as pushable if the join is found to be pushable.
> */
> if (root->parse->commandType == CMD_DELETE ||
> root->parse->commandType == CMD_UPDATE ||
> root->rowMarks)
> {
> epq_path = GetExistingLocalJoinPath(joinrel);
>
> Therefore, if the tuple is concurrently updated before taking a lock,
> we recheck the traversed tuple via EvalPlanQual() but we end up with
> the assertion failure since we didn't prepare the join plan for that.
>
> The attached patch includes the draft fix and regression tests (using
> injection points).
>
> I don't have enough experience with the planner and FDW code area to
> evaluate whether the patch fixes the issue in the right approach.
> Feedback is very welcome. I've confirmed this assertion could happen
> with the same scenario on all supported branches.
Will review. Thank you for the report and patch!
> In addition to that, I realized that none of the regression tests
> execute postgresRecheckForeignScan()[1]. I think we need to add
> regression tests to cover that function.
Yeah, I think so too.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-07 03:38 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-08-07 03:38 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Tue, Aug 05, 2025 at 11:00:54AM -0700, Masahiko Sawada wrote:
> The attached patch includes the draft fix and regression tests (using
> injection points).
+$psql_session->query_safe(qq[
+ select injection_points_set_local();
+ select injection_points_attach('heapam_lock_tuple-before-lock', 'wait');
+]);
It seems to me that an isolation test would be a better fit here. TAP
tests are usually a good fit for injection points if you need to do
direct node manipulations, like restarts or stops. The whole test
posted only does SQL-ish things, and you could rely on a loopback
server as we do in the SQL tests of postgres_fdw?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aJQfz3G-ZipUNTRx@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-07 17:29 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-08-07 17:29 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Aug 6, 2025 at 8:39 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Aug 05, 2025 at 11:00:54AM -0700, Masahiko Sawada wrote:
> > The attached patch includes the draft fix and regression tests (using
> > injection points).
>
> +$psql_session->query_safe(qq[
> + select injection_points_set_local();
> + select injection_points_attach('heapam_lock_tuple-before-lock', 'wait');
> +]);
>
> It seems to me that an isolation test would be a better fit here. TAP
> tests are usually a good fit for injection points if you need to do
> direct node manipulations, like restarts or stops. The whole test
> posted only does SQL-ish things, and you could rely on a loopback
> server as we do in the SQL tests of postgres_fdw?
Yes, it's definitely possible to create the test using isolation. I
wasn't sure how to invoke isolation tests only when injection points
are enabled.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-07 23:39 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-08-07 23:39 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Thu, Aug 07, 2025 at 10:29:41AM -0700, Masahiko Sawada wrote:
> Yes, it's definitely possible to create the test using isolation. I
> wasn't sure how to invoke isolation tests only when injection points
> are enabled.
For meson, I think that I would tweak the isolation test list based on
get_option('injection_points'), and do a similar thing for Makefile.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aJU5LMlFXWoix7Ip@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-08 07:15 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-08-08 07:15 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Thu, Aug 7, 2025 at 4:39 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Aug 07, 2025 at 10:29:41AM -0700, Masahiko Sawada wrote:
> > Yes, it's definitely possible to create the test using isolation. I
> > wasn't sure how to invoke isolation tests only when injection points
> > are enabled.
>
> For meson, I think that I would tweak the isolation test list based on
> get_option('injection_points'), and do a similar thing for Makefile.
Thank you for the advice! I've changed the regression tests to use
isolation tests.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v2-0001-Fix-assertion-failure-in-postgresGetForeignJoinPa.patch (6.7K, ../../CAD21AoCm1XYSD_R1capjuvLZ_3fKXnCu0C751Vk3hVOBDSwaCQ@mail.gmail.com/2-v2-0001-Fix-assertion-failure-in-postgresGetForeignJoinPa.patch)
download | inline diff:
From c2367bfb3e2b6e3b592b2bda08d5825cf2b95bd8 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Thu, 7 Aug 2025 23:37:31 -0700
Subject: [PATCH v2] Fix assertion failure in postgresGetForeignJoinPaths().
---
contrib/postgres_fdw/Makefile | 8 +++
.../postgres_fdw/expected/foreign_recheck.out | 27 ++++++++++
contrib/postgres_fdw/meson.build | 14 +++++-
contrib/postgres_fdw/postgres_fdw.c | 16 ++++--
.../postgres_fdw/specs/foreign_recheck.spec | 50 +++++++++++++++++++
src/backend/access/heap/heapam_handler.c | 3 ++
6 files changed, 114 insertions(+), 4 deletions(-)
create mode 100644 contrib/postgres_fdw/expected/foreign_recheck.out
create mode 100644 contrib/postgres_fdw/specs/foreign_recheck.spec
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index adfbd2ef758..fd1133f00dd 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -28,5 +28,13 @@ SHLIB_PREREQS = submake-libpq
subdir = contrib/postgres_fdw
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
+
+# We have the sole isolation test case that requires injection points,
+# so invoke the isolation test if injection points are enabled.
+ifeq ($(enable_injection_points),yes)
+EXTRA_INSTALL=src/test/modules/injection_points
+ISOLATION = foreign_recheck
+endif
+
include $(top_srcdir)/contrib/contrib-global.mk
endif
diff --git a/contrib/postgres_fdw/expected/foreign_recheck.out b/contrib/postgres_fdw/expected/foreign_recheck.out
new file mode 100644
index 00000000000..bab3b61c0c2
--- /dev/null
+++ b/contrib/postgres_fdw/expected/foreign_recheck.out
@@ -0,0 +1,27 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s0_lock s1_update s1_wakeup
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s0_lock:
+ SELECT
+ (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;
+ <waiting ...>
+step s1_update: UPDATE sch1.a SET i = i + 100;
+step s1_wakeup: SELECT injection_points_wakeup('heapam_lock_tuple-before-lock');
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s0_lock: <... completed>
+?column?
+--------
+
+(1 row)
+
diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build
index 5c11bc6496f..6dc40d7f1af 100644
--- a/contrib/postgres_fdw/meson.build
+++ b/contrib/postgres_fdw/meson.build
@@ -30,7 +30,7 @@ install_data(
kwargs: contrib_data_args,
)
-tests += {
+postgres_fdw_test = {
'name': 'postgres_fdw',
'sd': meson.current_source_dir(),
'bd': meson.current_build_dir(),
@@ -47,3 +47,15 @@ tests += {
],
},
}
+
+if get_option('injection_points')
+ postgres_fdw_test += {
+ 'isolation': {
+ 'specs': [
+ 'foreign_recheck',
+ ]
+ }
+ }
+endif
+
+tests += postgres_fdw_test
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 456b267f70b..23c1b956048 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6276,6 +6276,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
Cost total_cost;
Path *epq_path; /* Path to create plan to be executed when
* EvalPlanQual gets triggered. */
+ bool need_epq = false;
/*
* Skip if this join combination has been considered already.
@@ -6313,9 +6314,18 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
* calling foreign_join_ok(), since that function updates fpinfo and marks
* it as pushable if the join is found to be pushable.
*/
- if (root->parse->commandType == CMD_DELETE ||
- root->parse->commandType == CMD_UPDATE ||
- root->rowMarks)
+ for (PlannerInfo *proot = root; proot != NULL; proot = proot->parent_root)
+ {
+ if (proot->parse->commandType == CMD_DELETE ||
+ proot->parse->commandType == CMD_UPDATE ||
+ proot->rowMarks)
+ {
+ need_epq = true;
+ break;
+ }
+ }
+
+ if (need_epq)
{
epq_path = GetExistingLocalJoinPath(joinrel);
if (!epq_path)
diff --git a/contrib/postgres_fdw/specs/foreign_recheck.spec b/contrib/postgres_fdw/specs/foreign_recheck.spec
new file mode 100644
index 00000000000..9cc48e6a48c
--- /dev/null
+++ b/contrib/postgres_fdw/specs/foreign_recheck.spec
@@ -0,0 +1,50 @@
+setup
+{
+ CREATE EXTENSION injection_points;
+ 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);
+}
+
+session "s0"
+setup {
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('heapam_lock_tuple-before-lock', 'wait');
+}
+step "s0_lock" {
+ SELECT
+ (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;
+}
+
+session "s1"
+step "s1_update" { UPDATE sch1.a SET i = i + 100; }
+step "s1_wakeup" { SELECT injection_points_wakeup('heapam_lock_tuple-before-lock'); }
+
+# "s0_lock" execute a FOR UPDATE query but it stops before locking the result
+# tuple because of the injection point. "s1_update" updates the same tuple
+# concurrently and we wake up the session "s0", resulting that the FOR UPDATE
+# query rechecks the tuple via EPQ. Verify that postgresRecheckForeignScan()
+# function can check the tuple properly.
+permutation "s0_lock" "s1_update" "s1_wakeup"
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index cb4bc35c93e..45b1cafc01d 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -44,6 +44,7 @@
#include "storage/procarray.h"
#include "storage/smgr.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/rel.h"
static void reform_and_rewrite_tuple(HeapTuple tuple,
@@ -375,6 +376,8 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
Assert(TTS_IS_BUFFERTUPLE(slot));
+ INJECTION_POINT("heapam_lock_tuple-before-lock", NULL);
+
tuple_lock_retry:
tuple->t_self = *tid;
result = heap_lock_tuple(relation, tuple, cid, mode, wait_policy,
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-18 06:58 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 2 replies; 42+ messages in thread
From: Michael Paquier @ 2025-08-18 06:58 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Fri, Aug 08, 2025 at 12:15:19AM -0700, Masahiko Sawada wrote:
> On Thu, Aug 7, 2025 at 4:39 PM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Thu, Aug 07, 2025 at 10:29:41AM -0700, Masahiko Sawada wrote:
> > > Yes, it's definitely possible to create the test using isolation. I
> > > wasn't sure how to invoke isolation tests only when injection points
> > > are enabled.
> >
> > For meson, I think that I would tweak the isolation test list based on
> > get_option('injection_points'), and do a similar thing for Makefile.
>
> Thank you for the advice! I've changed the regression tests to use
> isolation tests.
That's enough to reproduce the MULL pointer dereference, thanks!
I would suggest to add a description at the top of foreign_recheck,
documenting the purpose of the test.
+# "s0_lock" execute a FOR UPDATE query but it stops before locking the result
s/execute/executes/, with an 's'.
Fujita-san, are you planning to look at the proposal?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aKLPJet54cgC3JNA@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-18 07:02 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-08-18 07:02 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Mon, Aug 18, 2025 at 03:58:45PM +0900, Michael Paquier wrote:
> That's enough to reproduce the MULL pointer dereference, thanks!
>
> I would suggest to add a description at the top of foreign_recheck,
> documenting the purpose of the test.
>
> +# "s0_lock" execute a FOR UPDATE query but it stops before locking the result
> s/execute/executes/, with an 's'.
Another thing that I've noticed has been forgotten: postgres_fdw's
.gitignore needs entries for output_iso/ and tmp_check_iso/. Based on
git-prompt, GIT_PS1_SHOWUNTRACKEDFILES reports these paths after a
test run.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aKLQDb71OPsehsIX@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-18 08:42 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-08-18 08:42 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
Hi,
On Mon, Aug 18, 2025 at 3:58 PM Michael Paquier <michael@paquier.xyz> wrote:
> Fujita-san, are you planning to look at the proposal?
I have just started reviewing the patch. I will provide my first
comments by this Friday at the very latest.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-19 00:08 Michael Paquier <michael@paquier.xyz>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2025-08-19 00:08 UTC (permalink / raw)
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
On Mon, Aug 18, 2025 at 05:42:58PM +0900, Etsuro Fujita wrote:
> I have just started reviewing the patch. I will provide my first
> comments by this Friday at the very latest.
Great, thanks a lot!
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aKPAfUMRYy_JNU8S@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-19 18:04 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 42+ messages in thread
From: Masahiko Sawada @ 2025-08-19 18:04 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Mon, Aug 18, 2025 at 12:02 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Aug 18, 2025 at 03:58:45PM +0900, Michael Paquier wrote:
> > That's enough to reproduce the MULL pointer dereference, thanks!
> >
> > I would suggest to add a description at the top of foreign_recheck,
> > documenting the purpose of the test.
> >
> > +# "s0_lock" execute a FOR UPDATE query but it stops before locking the result
> > s/execute/executes/, with an 's'.
>
> Another thing that I've noticed has been forgotten: postgres_fdw's
> .gitignore needs entries for output_iso/ and tmp_check_iso/. Based on
> git-prompt, GIT_PS1_SHOWUNTRACKEDFILES reports these paths after a
> test run.
Thank you for reviewing the patch! I'll incorporate these comments in
the next version patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-22 10:59 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-08-22 10:59 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Aug 6, 2025 at 8:30 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Wed, Aug 6, 2025 at 3:01 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > The attached patch includes the draft fix and regression tests (using
> > injection points).
> >
> > I don't have enough experience with the planner and FDW code area to
> > evaluate whether the patch fixes the issue in the right approach.
> > Feedback is very welcome. I've confirmed this assertion could happen
> > with the same scenario on all supported branches.
>
> Will review. Thank you for the report and patch!
First, my apologies for the delay.
I reviewed the postgres_fdw.c part of the fix:
@@ -6313,9 +6314,18 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
* calling foreign_join_ok(), since that function updates fpinfo and marks
* it as pushable if the join is found to be pushable.
*/
- if (root->parse->commandType == CMD_DELETE ||
- root->parse->commandType == CMD_UPDATE ||
- root->rowMarks)
+ for (PlannerInfo *proot = root; proot != NULL; proot = proot->parent_root)
+ {
+ if (proot->parse->commandType == CMD_DELETE ||
+ proot->parse->commandType == CMD_UPDATE ||
+ proot->rowMarks)
+ {
+ need_epq = true;
+ break;
+ }
+ }
+
+ if (need_epq)
{
epq_path = GetExistingLocalJoinPath(joinrel);
if (!epq_path)
I think this successfully avoids the assertion failure and produces
the correct result, but sorry, I don't think it's the right way to go.
I think the root cause of this issue is in the EPQ handling of
foreign/custom joins in ExecScanFetch() in execScan.h: it runs the
recheck method function for a given foreign/custom join whenever it is
called for EPQ rechecking, but that is not 100% correct. I think the
correct handling is: run the recheck method function for the join if
it is called for EPQ rechecking and the join is a *descendant* join in
the EPQ plan tree; otherwise run the access method function for the
join even if it is called for EPQ rechecking, like the attached (where
I used the epqParam of the given EPQState to determine whether the
join is a descendant join or not, which localizes the fix pretty
well). For the SELECT FOR UPDATE query shown upthread, when doing an
EPQ recheck, the fix evaluates the sub-select expression in the target
list by doing a remote join, not a local join, so it would work more
efficiently than the fix you proposed.
Sorry, I have not yet reviewed the isolation test part. Will do next.
Best regards,
Etsuro Fujita
Attachments:
[application/octet-stream] Fix-EPQ-handling-of-foreign-joins-in-ExecScanFetch.patch (1.3K, ../../CAPmGK16e1JgMN=MYsz1=kkmL29sa92_LOTOQNwL9fG2+QH-=Gw@mail.gmail.com/2-Fix-EPQ-handling-of-foreign-joins-in-ExecScanFetch.patch)
download | inline diff:
diff --git a/src/include/executor/execScan.h b/src/include/executor/execScan.h
index 837ea7785bb..2003cbc7ed5 100644
--- a/src/include/executor/execScan.h
+++ b/src/include/executor/execScan.h
@@ -49,16 +49,24 @@ 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])
{
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-08-22 20:53 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-08-22 20:53 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Fri, Aug 22, 2025 at 3:59 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Wed, Aug 6, 2025 at 8:30 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Wed, Aug 6, 2025 at 3:01 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > The attached patch includes the draft fix and regression tests (using
> > > injection points).
> > >
> > > I don't have enough experience with the planner and FDW code area to
> > > evaluate whether the patch fixes the issue in the right approach.
> > > Feedback is very welcome. I've confirmed this assertion could happen
> > > with the same scenario on all supported branches.
> >
> > Will review. Thank you for the report and patch!
>
> First, my apologies for the delay.
>
> I reviewed the postgres_fdw.c part of the fix:
>
> @@ -6313,9 +6314,18 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
> * calling foreign_join_ok(), since that function updates fpinfo and marks
> * it as pushable if the join is found to be pushable.
> */
> - if (root->parse->commandType == CMD_DELETE ||
> - root->parse->commandType == CMD_UPDATE ||
> - root->rowMarks)
> + for (PlannerInfo *proot = root; proot != NULL; proot = proot->parent_root)
> + {
> + if (proot->parse->commandType == CMD_DELETE ||
> + proot->parse->commandType == CMD_UPDATE ||
> + proot->rowMarks)
> + {
> + need_epq = true;
> + break;
> + }
> + }
> +
> + if (need_epq)
> {
> epq_path = GetExistingLocalJoinPath(joinrel);
> if (!epq_path)
>
> I think this successfully avoids the assertion failure and produces
> the correct result, but sorry, I don't think it's the right way to go.
> I think the root cause of this issue is in the EPQ handling of
> foreign/custom joins in ExecScanFetch() in execScan.h: it runs the
> recheck method function for a given foreign/custom join whenever it is
> called for EPQ rechecking, but that is not 100% correct. I think the
> correct handling is: run the recheck method function for the join if
> it is called for EPQ rechecking and the join is a *descendant* join in
> the EPQ plan tree; otherwise run the access method function for the
> join even if it is called for EPQ rechecking, like the attached (where
> I used the epqParam of the given EPQState to determine whether the
> join is a descendant join or not, which localizes the fix pretty
> well). For the SELECT FOR UPDATE query shown upthread, when doing an
> EPQ recheck, the fix evaluates the sub-select expression in the target
> list by doing a remote join, not a local join, so it would work more
> efficiently than the fix you proposed.
Thank you for reviewing the patch! I've confirmed that your patch
fixes the issue too.
If I understand your proposed fix correctly, the reported problem is
fixed by not rechecking the test tuple by ForeignRecheck() (performing
a local join), but instead we simply call ForeignNext() and get the
next tuple. I think while this fix would have to have a regression
test like I've proposed, it's probably a good time to add some
regression tests to cover postgresRecheckForeignScan() too possibly as
a separate patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-17 10:42 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-09-17 10:42 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Sat, Aug 23, 2025 at 5:54 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Fri, Aug 22, 2025 at 3:59 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > I reviewed the postgres_fdw.c part of the fix:
> >
> > @@ -6313,9 +6314,18 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
> > * calling foreign_join_ok(), since that function updates fpinfo and marks
> > * it as pushable if the join is found to be pushable.
> > */
> > - if (root->parse->commandType == CMD_DELETE ||
> > - root->parse->commandType == CMD_UPDATE ||
> > - root->rowMarks)
> > + for (PlannerInfo *proot = root; proot != NULL; proot = proot->parent_root)
> > + {
> > + if (proot->parse->commandType == CMD_DELETE ||
> > + proot->parse->commandType == CMD_UPDATE ||
> > + proot->rowMarks)
> > + {
> > + need_epq = true;
> > + break;
> > + }
> > + }
> > +
> > + if (need_epq)
> > {
> > epq_path = GetExistingLocalJoinPath(joinrel);
> > if (!epq_path)
> >
> > I think this successfully avoids the assertion failure and produces
> > the correct result, but sorry, I don't think it's the right way to go.
> > I think the root cause of this issue is in the EPQ handling of
> > foreign/custom joins in ExecScanFetch() in execScan.h: it runs the
> > recheck method function for a given foreign/custom join whenever it is
> > called for EPQ rechecking, but that is not 100% correct. I think the
> > correct handling is: run the recheck method function for the join if
> > it is called for EPQ rechecking and the join is a *descendant* join in
> > the EPQ plan tree; otherwise run the access method function for the
> > join even if it is called for EPQ rechecking, like the attached (where
> > I used the epqParam of the given EPQState to determine whether the
> > join is a descendant join or not, which localizes the fix pretty
> > well). For the SELECT FOR UPDATE query shown upthread, when doing an
> > EPQ recheck, the fix evaluates the sub-select expression in the target
> > list by doing a remote join, not a local join, so it would work more
> > efficiently than the fix you proposed.
>
> Thank you for reviewing the patch! I've confirmed that your patch
> fixes the issue too.
Thanks for testing!
> If I understand your proposed fix correctly, the reported problem is
> fixed by not rechecking the test tuple by ForeignRecheck() (performing
> a local join), but instead we simply call ForeignNext() and get the
> next tuple.
That's right.
> I think while this fix would have to have a regression
> test like I've proposed, it's probably a good time to add some
> regression tests to cover postgresRecheckForeignScan() too possibly as
> a separate patch.
Agreed. Actually, we have fixed many EvalPlanQual issues with
postgres_fdw so far, but have always left adding test cases for future
work, so let's do that now!
The test case you showed upthread and added to the patch is useful, so
I think we should add it as well, but my question about it is: is it
really a good idea to use injection points? Why don't you just use
BEGIN/COMMIT statements like this:
-- session 1
begin isolation level read committed;
update a set i = i + 1;
-- session 2
begin isolation level read committed;
select a.i,
(select 1 from b, c where a.i = b.i and b.i = c.i)
from a
for update; -- waits for the transaction in session 1 to complete
-- session 1
commit;
-- session 2
select a.i,
(select 1 from b, c where a.i = b.i and b.i = c.i)
from a
for update; -- produces results after doing an EvalPlanQual recheck
i | ?column?
---+----------
2 |
(1 row)
Again, my apologies for the late response.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-18 00:24 Michael Paquier <michael@paquier.xyz>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-09-18 00:24 UTC (permalink / raw)
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
On Wed, Sep 17, 2025 at 07:42:36PM +0900, Etsuro Fujita wrote:
> The test case you showed upthread and added to the patch is useful, so
> I think we should add it as well, but my question about it is: is it
> really a good idea to use injection points?
> Why don't you just use BEGIN/COMMIT statements like this:
>
> -- session 1
> begin isolation level read committed;
> update a set i = i + 1;
>
> -- session 2
> begin isolation level read committed;
> select a.i,
> (select 1 from b, c where a.i = b.i and b.i = c.i)
> from a
> for update; -- waits for the transaction in session 1 to complete
>
> -- session 1
> commit;
>
> -- session 2
> select a.i,
> (select 1 from b, c where a.i = b.i and b.i = c.i)
> from a
> for update; -- produces results after doing an EvalPlanQual recheck
> i | ?column?
> ---+----------
> 2 |
> (1 row)
>
> Again, my apologies for the late response.
As far as I can see, this causes the SELECT FOR UPDATE of session 2
that's waiting for the commit of session 1 to crash, if we don't have
the fix, of course. Removing the dependency with injection points is
nice if we don't require it, so we can just tweak the isolation test
proposed upthread to use the same schema, but the queries you are
suggesting.
As a whole, +1 for your suggestion.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aMtRUQqnowiYFIha@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-23 21:41 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 3 replies; 42+ messages in thread
From: Masahiko Sawada @ 2025-09-23 21:41 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Sep 17, 2025 at 5:25 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Sep 17, 2025 at 07:42:36PM +0900, Etsuro Fujita wrote:
> > The test case you showed upthread and added to the patch is useful, so
> > I think we should add it as well, but my question about it is: is it
> > really a good idea to use injection points?
>
> > Why don't you just use BEGIN/COMMIT statements like this:
> >
> > -- session 1
> > begin isolation level read committed;
> > update a set i = i + 1;
> >
> > -- session 2
> > begin isolation level read committed;
> > select a.i,
> > (select 1 from b, c where a.i = b.i and b.i = c.i)
> > from a
> > for update; -- waits for the transaction in session 1 to complete
> >
> > -- session 1
> > commit;
> >
> > -- session 2
> > select a.i,
> > (select 1 from b, c where a.i = b.i and b.i = c.i)
> > from a
> > for update; -- produces results after doing an EvalPlanQual recheck
> > i | ?column?
> > ---+----------
> > 2 |
> > (1 row)
> >
> > Again, my apologies for the late response.
>
> As far as I can see, this causes the SELECT FOR UPDATE of session 2
> that's waiting for the commit of session 1 to crash, if we don't have
> the fix, of course. Removing the dependency with injection points is
> nice if we don't require it, so we can just tweak the isolation test
> proposed upthread to use the same schema, but the queries you are
> suggesting.
+1
I changed the regression tests and used the fix proposed by
Fujita-san. Please review the attached new version patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v3-0001-Fix-assertion-failure-in-postgresGetForeignJoinPa.patch (5.6K, ../../CAD21AoCD=Fg6cx9HQGY1JUy+ZVd5_k_zTqPn7HjPPL2CjncbHg@mail.gmail.com/2-v3-0001-Fix-assertion-failure-in-postgresGetForeignJoinPa.patch)
download | inline diff:
From c4d87c2acf0a6175b910e528874c6644d743069a Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
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;
+ <waiting ...>
+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
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-23 23:39 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
2 siblings, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2025-09-23 23:39 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Tue, Sep 23, 2025 at 02:41:45PM -0700, Masahiko Sawada wrote:
> I changed the regression tests and used the fix proposed by
> Fujita-san. Please review the attached new version patch.
Thanks for the updated patch. Confirmed that I can still blow up the
backend as expected when not using the fix, only the test.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aNMvl1-eGc-7Ghyf@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-24 10:28 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
2 siblings, 2 replies; 42+ messages in thread
From: Etsuro Fujita @ 2025-09-24 10:28 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Sep 24, 2025 at 6:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I changed the regression tests and used the fix proposed by
> Fujita-san. Please review the attached new version patch.
Thanks for updating the patch!
I took a quick glance at the patch. My initial comment is: it only
includes the test case discussed here, but I think it's a good idea to
add more cases in it (that exercise code paths in
postgresRecheckForeignScan), as I mentioned upthread. What do you
think about that? If no objections, I'd like to update it to include
such cases.
Anyway I'll review it in more detail.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-24 16:45 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
1 sibling, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-09-24 16:45 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Sep 24, 2025 at 3:28 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Wed, Sep 24, 2025 at 6:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > I changed the regression tests and used the fix proposed by
> > Fujita-san. Please review the attached new version patch.
>
> Thanks for updating the patch!
>
> I took a quick glance at the patch. My initial comment is: it only
> includes the test case discussed here, but I think it's a good idea to
> add more cases in it (that exercise code paths in
> postgresRecheckForeignScan), as I mentioned upthread. What do you
> think about that? If no objections, I'd like to update it to include
> such cases.
+1 to add more test cases for the code paths in postgresRecheckForeignScan.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-24 23:14 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-09-24 23:14 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Sep 24, 2025 at 09:45:26AM -0700, Masahiko Sawada wrote:
> On Wed, Sep 24, 2025 at 3:28 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> I took a quick glance at the patch. My initial comment is: it only
>> includes the test case discussed here, but I think it's a good idea to
>> add more cases in it (that exercise code paths in
>> postgresRecheckForeignScan), as I mentioned upthread. What do you
>> think about that? If no objections, I'd like to update it to include
>> such cases.
>
> +1 to add more test cases for the code paths in postgresRecheckForeignScan.
+1. Would these be included in the same isolation test? That may be
cleaner, especially as it seems like the EPQ re-evaluations should be
able to happen with the same schema as the one set up by the test.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aNR7Zi_1-4gtYeI8@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-25 10:55 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 42+ messages in thread
From: Etsuro Fujita @ 2025-09-25 10:55 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Thu, Sep 25, 2025 at 8:15 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Wed, Sep 24, 2025 at 09:45:26AM -0700, Masahiko Sawada wrote:
> > On Wed, Sep 24, 2025 at 3:28 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> >> I took a quick glance at the patch. My initial comment is: it only
> >> includes the test case discussed here, but I think it's a good idea to
> >> add more cases in it (that exercise code paths in
> >> postgresRecheckForeignScan), as I mentioned upthread. What do you
> >> think about that? If no objections, I'd like to update it to include
> >> such cases.
> >
> > +1 to add more test cases for the code paths in postgresRecheckForeignScan.
>
> +1. Would these be included in the same isolation test? That may be
> cleaner, especially as it seems like the EPQ re-evaluations should be
> able to happen with the same schema as the one set up by the test.
Yes, I will add those cases to the same isolation test, as you mentioned.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-26 13:34 Álvaro Herrera <alvherre@kurilemu.de>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
1 sibling, 1 reply; 42+ messages in thread
From: Álvaro Herrera @ 2025-09-26 13:34 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On 2025-Sep-24, Etsuro Fujita wrote:
> I took a quick glance at the patch. My initial comment is: it only
> includes the test case discussed here, but I think it's a good idea to
> add more cases in it (that exercise code paths in
> postgresRecheckForeignScan), as I mentioned upthread. What do you
> think about that? If no objections, I'd like to update it to include
> such cases.
I would like to suggest a different approach: because this patch is
known to fix a server crash in a known case, it would be better to get
the fix (and its corresponding test) pushed as is without further delay.
Additional tests are an excellent idea, but I think we shouldn't make
this patch wait for them. If further fixes are deemed necessary for
those other (thus far hypothetical) tests, we can still make them
afterwards.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-09-28 08:55 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 0 replies; 42+ messages in thread
From: Etsuro Fujita @ 2025-09-28 08:55 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Fri, Sep 26, 2025 at 10:34 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
> On 2025-Sep-24, Etsuro Fujita wrote:
>
> > I took a quick glance at the patch. My initial comment is: it only
> > includes the test case discussed here, but I think it's a good idea to
> > add more cases in it (that exercise code paths in
> > postgresRecheckForeignScan), as I mentioned upthread. What do you
> > think about that? If no objections, I'd like to update it to include
> > such cases.
>
> I would like to suggest a different approach: because this patch is
> known to fix a server crash in a known case, it would be better to get
> the fix (and its corresponding test) pushed as is without further delay.
> Additional tests are an excellent idea, but I think we shouldn't make
> this patch wait for them. If further fixes are deemed necessary for
> those other (thus far hypothetical) tests, we can still make them
> afterwards.
The 2-step approach sounds reasonable, so I'll review the proposed
isolation test and report the results first.
Thanks!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-01 10:53 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
2 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-01 10:53 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Sep 24, 2025 at 6:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I changed the regression tests and used the fix proposed by
> Fujita-san. Please review the attached new version patch.
I reviewed the test part. Here are my comments about it.
You renamed the spec file to concurrent_update.spec. It's a broader
name to cover the test case discussed here, but seems a bit vague to
me. How about eval_plan_qual.spec like
src/test/isolation/specs/eval-plan-qual.spec, instead? Which I think
is more specific.
In the setup block in the test, you created schemas:
+ 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);
But actually, we don't need these schemas. As I'm planning to add
more permutations using the same setup, and setup is executed once per
permutation, I'd like to propose to save cycles by creating all these
tables in the current schema like this:
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');
+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;
+}
I think the important point in the test case discussed here is that
the sub-select has a foreign-join plan (without an alternative local
join plan). So I'd like to propose to add an EXPLAIN command to this
step, to confirm that it has such a plan.
Also, how about s/s1_lock/s1_tuplock/? This is just my taste, though.
+# "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"
Let's add to the permutation s1_commit as well, which I think is a
good practice.
Considering the permutation is a typical sequence to exercise an
EvalPlanQual recheck, I don't feel the need for the first part of the
comments, sorry. Also, IMO I think the second part is a bit too
detailed. How about simplifying the comments to something like this
(I used a comment for a similar test in
src/test/isolation/specs/eval-plan-qual.spec.):
"This test exercises EvalPlanQual with a SubLink sub-select (which
should be unaffected by any EPQ recheck behavior in the outer query)"
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-02 11:01 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-02 11:01 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Oct 1, 2025 at 7:53 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Wed, Sep 24, 2025 at 6:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > I changed the regression tests and used the fix proposed by
> > Fujita-san. Please review the attached new version patch.
>
> I reviewed the test part. Here are my comments about it.
>
> You renamed the spec file to concurrent_update.spec. It's a broader
> name to cover the test case discussed here, but seems a bit vague to
> me. How about eval_plan_qual.spec like
> src/test/isolation/specs/eval-plan-qual.spec, instead? Which I think
> is more specific.
>
> In the setup block in the test, you created schemas:
>
> + 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);
>
> But actually, we don't need these schemas. As I'm planning to add
> more permutations using the same setup, and setup is executed once per
> permutation, I'd like to propose to save cycles by creating all these
> tables in the current schema like this:
>
> 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');
>
> +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;
> +}
>
> I think the important point in the test case discussed here is that
> the sub-select has a foreign-join plan (without an alternative local
> join plan). So I'd like to propose to add an EXPLAIN command to this
> step, to confirm that it has such a plan.
>
> Also, how about s/s1_lock/s1_tuplock/? This is just my taste, though.
>
> +# "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"
>
> Let's add to the permutation s1_commit as well, which I think is a
> good practice.
>
> Considering the permutation is a typical sequence to exercise an
> EvalPlanQual recheck, I don't feel the need for the first part of the
> comments, sorry. Also, IMO I think the second part is a bit too
> detailed. How about simplifying the comments to something like this
> (I used a comment for a similar test in
> src/test/isolation/specs/eval-plan-qual.spec.):
>
> "This test exercises EvalPlanQual with a SubLink sub-select (which
> should be unaffected by any EPQ recheck behavior in the outer query)"
Some more comments:
+teardown
+{
+}
I think it's good to add cleanup statements here; otherwise let's
remove this for now, to suppress a "teardown failed:" error.
--- 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
+
I'd like to propose to add "ISOLATION_OPTS =
--load-extension=postgres_fdw", by which we don't need to do CREATE
EXTENSION in the setup block.
That's it.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-02 22:20 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 2 replies; 42+ messages in thread
From: Masahiko Sawada @ 2025-10-02 22:20 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Thu, Oct 2, 2025 at 4:01 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Wed, Oct 1, 2025 at 7:53 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Wed, Sep 24, 2025 at 6:42 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > I changed the regression tests and used the fix proposed by
> > > Fujita-san. Please review the attached new version patch.
> >
> > I reviewed the test part. Here are my comments about it.
> >
> > You renamed the spec file to concurrent_update.spec. It's a broader
> > name to cover the test case discussed here, but seems a bit vague to
> > me. How about eval_plan_qual.spec like
> > src/test/isolation/specs/eval-plan-qual.spec, instead? Which I think
> > is more specific.
> >
> > In the setup block in the test, you created schemas:
> >
> > + 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);
> >
> > But actually, we don't need these schemas. As I'm planning to add
> > more permutations using the same setup, and setup is executed once per
> > permutation, I'd like to propose to save cycles by creating all these
> > tables in the current schema like this:
> >
> > 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');
> >
> > +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;
> > +}
> >
> > I think the important point in the test case discussed here is that
> > the sub-select has a foreign-join plan (without an alternative local
> > join plan). So I'd like to propose to add an EXPLAIN command to this
> > step, to confirm that it has such a plan.
> >
> > Also, how about s/s1_lock/s1_tuplock/? This is just my taste, though.
> >
> > +# "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"
> >
> > Let's add to the permutation s1_commit as well, which I think is a
> > good practice.
> >
> > Considering the permutation is a typical sequence to exercise an
> > EvalPlanQual recheck, I don't feel the need for the first part of the
> > comments, sorry. Also, IMO I think the second part is a bit too
> > detailed. How about simplifying the comments to something like this
> > (I used a comment for a similar test in
> > src/test/isolation/specs/eval-plan-qual.spec.):
> >
> > "This test exercises EvalPlanQual with a SubLink sub-select (which
> > should be unaffected by any EPQ recheck behavior in the outer query)"
>
> Some more comments:
>
> +teardown
> +{
> +}
>
> I think it's good to add cleanup statements here; otherwise let's
> remove this for now, to suppress a "teardown failed:" error.
>
> --- 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
> +
>
> I'd like to propose to add "ISOLATION_OPTS =
> --load-extension=postgres_fdw", by which we don't need to do CREATE
> EXTENSION in the setup block.
>
> That's it.
>
Thank you for reviewing the patch!
I've updated the patch based on your comments. Please find the attached patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v4-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch (6.2K, ../../CAD21AoALtwy43TYR7tXoOPHKEPkutagYYY1TmbNzoR_6+fzA1g@mail.gmail.com/2-v4-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch)
download | inline diff:
From e3c0a37bbe457a37f074ca3062b6e53415db0695 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 23 Sep 2025 12:23:16 -0700
Subject: [PATCH v4] postgres_fdw: Fix assertion failure when EvalPlanQual with
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=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com
---
contrib/postgres_fdw/Makefile | 3 ++
.../postgres_fdw/expected/eval_plan_qual.out | 25 +++++++++
contrib/postgres_fdw/meson.build | 6 +++
.../postgres_fdw/specs/eval_plan_qual.spec | 53 +++++++++++++++++++
src/include/executor/execScan.h | 23 +++++---
5 files changed, 102 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/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index adfbd2ef758..81df6ca9085 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 00000000000..385b1a03981
--- /dev/null
+++ b/contrib/postgres_fdw/expected/eval_plan_qual.out
@@ -0,0 +1,25 @@
+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:
+ EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+ SELECT a.i,
+ (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
+ FROM a as a
+ FOR UPDATE;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock: <... completed>
+QUERY PLAN
+-----------------------------------------------------
+LockRows (actual rows=1.00 loops=1)
+ -> Seq Scan on a (actual rows=1.00 loops=1)
+ SubPlan 1
+ -> Foreign Scan (actual rows=1.00 loops=1)
+ Relations: (fb) INNER JOIN (fc)
+(5 rows)
+
+step s1_commit: COMMIT;
diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build
index 5c11bc6496f..aac89ffdde8 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 00000000000..2a2399d5848
--- /dev/null
+++ b/contrib/postgres_fdw/specs/eval_plan_qual.spec
@@ -0,0 +1,53 @@
+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" {
+ EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+ SELECT a.i,
+ (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
+ FROM a as a
+ FOR UPDATE;
+}
+step "s1_commit" { COMMIT; }
+
+# This test exerices 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"
+
+
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
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-03 00:57 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2025-10-03 00:57 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Thu, Oct 02, 2025 at 03:20:32PM -0700, Masahiko Sawada wrote:
> I've updated the patch based on your comments. Please find the attached patch.
Looks sensible here in shape, a few style nits noticed while reading.
x
+# This test exerices EvalPlanQual with a SubLink sub-select (which should
+# be unaffected by any EPQ recheck behavior in the outer query)
The setup phase has a mix of tabs (used in the DO block) and
spaces used, that may render weirdly depending on one's setup when
showing the diffs. It looks to me that your intention is to use
spaces.
s/exerices/exercises/ And I would add a short explanation about the
reason why this script exists at the top of eval_plan_qual.spec. If
you don't feel strongly about that, feel free to ignore me.
One other bit I have noticed on the way, due to the extra whitespaces
at the bottom of the file:
$ git diff master --check
contrib/postgres_fdw/specs/eval_plan_qual.spec:52: new blank line at EOF.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aN8fYwIw0p7ZMYGA@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-03 11:00 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
1 sibling, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-03 11:00 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Fri, Oct 3, 2025 at 7:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I've updated the patch based on your comments. Please find the attached patch.
Thanks for updating the patch!
+step "s1_tuplock" {
+ EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+ SELECT a.i,
+ (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
+ FROM a as a
+ FOR UPDATE;
+}
Maybe my comment about this step was not enough, but I'm wondering we
should run EXPLAIN and then SELECT here like below, rather than
running EXPLAIN ANALYZE, as that seems more usual to me:
step "s1_tuplock" {
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;
}
I added the VERBOSE option to show the remote query, and removed the
alias for a.
BTW: you added quotation marks around a name for each session or step
like "s1_tuplock". Do we really need them? This is nitpicking,
though.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-03 21:21 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-10-03 21:21 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Fri, Oct 3, 2025 at 4:00 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Fri, Oct 3, 2025 at 7:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > I've updated the patch based on your comments. Please find the attached patch.
>
> Thanks for updating the patch!
>
> +step "s1_tuplock" {
> + EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
> + SELECT a.i,
> + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
> + FROM a as a
> + FOR UPDATE;
> +}
>
> Maybe my comment about this step was not enough, but I'm wondering we
> should run EXPLAIN and then SELECT here like below, rather than
> running EXPLAIN ANALYZE, as that seems more usual to me:
>
> step "s1_tuplock" {
> 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;
> }
>
> I added the VERBOSE option to show the remote query, and removed the
> alias for a.
Done.
> BTW: you added quotation marks around a name for each session or step
> like "s1_tuplock". Do we really need them? This is nitpicking,
> though.
No, I brought a convention from other spec files. I've unquoted all
sessions and steps.
I've attached the updated patch that dealt with all comments including
ones Michael-san made[1].
Regards,
[1] https://www.postgresql.org/message-id/aN8fYwIw0p7ZMYGA%40paquier.xyz
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v5-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch (6.4K, ../../CAD21AoDbY_nYAas_cfR1PQ-ZrN+mBCkYWHAoe6p1Q3E_7=PJfg@mail.gmail.com/2-v5-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch)
download | inline diff:
From 99f332fc9a5ad546a0660cd6a7a4cdb76369c79f Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 23 Sep 2025 12:23:16 -0700
Subject: [PATCH v5] postgres_fdw: Fix assertion failure when EvalPlanQual with
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=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com
---
contrib/postgres_fdw/Makefile | 3 +
.../postgres_fdw/expected/eval_plan_qual.out | 25 ++++++++
contrib/postgres_fdw/meson.build | 6 ++
.../postgres_fdw/specs/eval_plan_qual.spec | 58 +++++++++++++++++++
src/include/executor/execScan.h | 23 +++++---
5 files changed, 107 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/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index adfbd2ef758..81df6ca9085 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 00000000000..385b1a03981
--- /dev/null
+++ b/contrib/postgres_fdw/expected/eval_plan_qual.out
@@ -0,0 +1,25 @@
+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:
+ EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
+ SELECT a.i,
+ (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
+ FROM a as a
+ FOR UPDATE;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock: <... completed>
+QUERY PLAN
+-----------------------------------------------------
+LockRows (actual rows=1.00 loops=1)
+ -> Seq Scan on a (actual rows=1.00 loops=1)
+ SubPlan 1
+ -> Foreign Scan (actual rows=1.00 loops=1)
+ Relations: (fb) INNER JOIN (fc)
+(5 rows)
+
+step s1_commit: COMMIT;
diff --git a/contrib/postgres_fdw/meson.build b/contrib/postgres_fdw/meson.build
index 5c11bc6496f..aac89ffdde8 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 00000000000..6552bd605f4
--- /dev/null
+++ b/contrib/postgres_fdw/specs/eval_plan_qual.spec
@@ -0,0 +1,58 @@
+# 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 as a
+ FOR UPDATE;
+ SELECT a.i,
+ (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
+ FROM a as 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
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
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-05 17:15 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-05 17:15 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Sat, Oct 4, 2025 at 6:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Fri, Oct 3, 2025 at 4:00 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Fri, Oct 3, 2025 at 7:21 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > I've updated the patch based on your comments. Please find the attached patch.
> >
> > Thanks for updating the patch!
> >
> > +step "s1_tuplock" {
> > + EXPLAIN (COSTS OFF, ANALYZE ON, TIMING OFF, SUMMARY OFF, BUFFERS OFF)
> > + SELECT a.i,
> > + (SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
> > + FROM a as a
> > + FOR UPDATE;
> > +}
> >
> > Maybe my comment about this step was not enough, but I'm wondering we
> > should run EXPLAIN and then SELECT here like below, rather than
> > running EXPLAIN ANALYZE, as that seems more usual to me:
> >
> > step "s1_tuplock" {
> > 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;
> > }
> >
> > I added the VERBOSE option to show the remote query, and removed the
> > alias for a.
>
> Done.
>
> > BTW: you added quotation marks around a name for each session or step
> > like "s1_tuplock". Do we really need them? This is nitpicking,
> > though.
>
> No, I brought a convention from other spec files. I've unquoted all
> sessions and steps.
>
> I've attached the updated patch that dealt with all comments including
> ones Michael-san made[1].
Thanks for updating the patch!
The isolation test fails. I think you failed to update the expected
file. Also, you left the useless alias for table a alone...
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-06 03:37 Michael Paquier <michael@paquier.xyz>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-10-06 03:37 UTC (permalink / raw)
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
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
Attachments:
[text/x-diff] v6-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch (7.9K, ../../aOM5cXaj24EE-ryO@paquier.xyz/2-v6-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch)
download | inline diff:
From bb403a2867bd8e33cfee10902d073a95f4431bbe Mon Sep 17 00:00:00 2001
From: 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 with
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=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;
+ <waiting ...>
+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
[application/pgp-signature] signature.asc (832B, ../../aOM5cXaj24EE-ryO@paquier.xyz/3-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-07 10:59 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-07 10:59 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Mon, Oct 6, 2025 at 12:37 PM Michael Paquier <michael@paquier.xyz> wrote:
> 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.
LGTM. Thanks for updating the patch including the .gitignore file!
Sawada-san, the patch uses the fix for ExecScanFetch that I proposed,
so if you wouldn't mind, I'd like to take care of this.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-07 17:25 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-10-07 17:25 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Tue, Oct 7, 2025 at 3:59 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Mon, Oct 6, 2025 at 12:37 PM Michael Paquier <michael@paquier.xyz> wrote:
> > 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.
>
> LGTM. Thanks for updating the patch including the .gitignore file!
+1
> Sawada-san, the patch uses the fix for ExecScanFetch that I proposed,
> so if you wouldn't mind, I'd like to take care of this.
Thank you. That would be great. Please feel free to edit the commit
message, subjects, and credits. I should have credited Fujita-san as
an author or co-author with me since most changes are based on
suggestions by Fujita-san.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-08 00:05 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-10-08 00:05 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Tue, Oct 07, 2025 at 10:25:29AM -0700, Masahiko Sawada wrote:
> Thank you. That would be great. Please feel free to edit the commit
> message, subjects, and credits. I should have credited Fujita-san as
> an author or co-author with me since most changes are based on
> suggestions by Fujita-san.
Yes, please feel free to edit that as you see fit. I was not in an
imaginative mood when posting the last patch, reusing what Sawada-san
has posted upthread as commit message.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aOWq5EYgbvl1eWeO@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-08 06:09 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-08 06:09 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Oct 8, 2025 at 9:06 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Tue, Oct 07, 2025 at 10:25:29AM -0700, Masahiko Sawada wrote:
> > Thank you. That would be great. Please feel free to edit the commit
> > message, subjects, and credits. I should have credited Fujita-san as
> > an author or co-author with me since most changes are based on
> > suggestions by Fujita-san.
>
> Yes, please feel free to edit that as you see fit. I was not in an
> imaginative mood when posting the last patch, reusing what Sawada-san
> has posted upthread as commit message.
Ok, will do.
Thanks!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-09 02:16 Michael Paquier <michael@paquier.xyz>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-10-09 02:16 UTC (permalink / raw)
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
On Wed, Oct 08, 2025 at 03:09:52PM +0900, Etsuro Fujita wrote:
> Ok, will do.
Cool. Thanks, Fujita-san.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aOca75WwQgnNWBWa@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-12 18:05 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-12 18:05 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Thu, Oct 9, 2025 at 11:16 AM Michael Paquier <michael@paquier.xyz> wrote:
> On Wed, Oct 08, 2025 at 03:09:52PM +0900, Etsuro Fujita wrote:
> > Ok, will do.
>
> Cool. Thanks, Fujita-san.
Here is an updated version of the patch, labelled v7, in which I
modified .gitignore a bit further to include tmp_check_iso as well,
did a bit of editorialization (eg, indentation in the spec file),
rebased the patch against HEAD, and updated the commit message.
If there are no objections, I'll push this version and back-patch it
to all supported branches.
Best regards,
Etsuro Fujita
Attachments:
[application/octet-stream] v7-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch (9.1K, ../../CAPmGK15=ebVst8L-uJuFuTOB0Cww-7Kmry4qAxBa1etNvb7HNA@mail.gmail.com/2-v7-0001-postgres_fdw-Fix-assertion-failure-when-EvalPlanQ.patch)
download | inline diff:
From a8e7888d472b31d4a5e6f93a6d1475c4922afa44 Mon Sep 17 00:00:00 2001
From: Etsuro Fujita <etsuro.fujita@gmail.com>
Date: Mon, 13 Oct 2025 02:51:55 +0900
Subject: [PATCH] Fix EvalPlanQual handling of foreign/custom joins in
ExecScanFetch.
If inside an EPQ recheck, ExecScanFetch would run the recheck method
function for foreign/custom joins even if they aren't descendant nodes
in the EPQ recheck plan tree, which is problematic at least in the
foreign-join case, because such foreign joins aren't guaranteed to have
an alternative local-join plan required for running the recheck method
function; in the postgres_fdw case this could lead to a segmentation
fault or an assert failure in an assert-enabled build when running the
recheck method function.
Even if inside an EPQ recheck, any scan nodes that aren't descendant
ones in the EPQ recheck plan tree should be normally processed by using
the access method function; fix by modifying ExecScanFetch so that if
inside an EPQ recheck, it runs the recheck method function for
foreign/custom joins that are descendant nodes in the EPQ recheck plan
tree as before and runs the access method function for foreign/custom
joins that aren't.
This fix also adds to postgres_fdw an isolation test for an EPQ recheck
that caused issues stated above.
Oversight in commit 385f337c9.
Reported-by: Kristian Lejao <kristianlejao@gmail.com>
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Co-authored-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com>
Discussion: https://postgr.es/m/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com
Backpatch-through: 13
---
contrib/postgres_fdw/.gitignore | 2 +
contrib/postgres_fdw/Makefile | 2 +
.../postgres_fdw/expected/eval_plan_qual.out | 37 +++++++++++++
contrib/postgres_fdw/meson.build | 6 ++
.../postgres_fdw/specs/eval_plan_qual.spec | 55 +++++++++++++++++++
src/include/executor/execScan.h | 22 +++++---
6 files changed, 117 insertions(+), 7 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/contrib/postgres_fdw/.gitignore b/contrib/postgres_fdw/.gitignore
index 5dcb3ff9723..b4903eba657 100644
--- a/contrib/postgres_fdw/.gitignore
+++ b/contrib/postgres_fdw/.gitignore
@@ -1,4 +1,6 @@
# Generated subdirectories
/log/
/results/
+/output_iso/
/tmp_check/
+/tmp_check_iso/
diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile
index adfbd2ef758..8eaf4d263b6 100644
--- a/contrib/postgres_fdw/Makefile
+++ b/contrib/postgres_fdw/Makefile
@@ -17,6 +17,8 @@ EXTENSION = postgres_fdw
DATA = postgres_fdw--1.0.sql postgres_fdw--1.0--1.1.sql postgres_fdw--1.1--1.2.sql
REGRESS = postgres_fdw query_cancel
+ISOLATION = eval_plan_qual
+ISOLATION_OPTS = --load-extension=postgres_fdw
TAP_TESTS = 1
ifdef USE_PGXS
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 00000000000..f3e3a22b336
--- /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 plan
+ 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;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock: <... completed>
+QUERY PLAN
+----------------------------------------------------------------------------------------------------------------------------------------
+LockRows
+ Output: a.i, ((SubPlan expr_1)), a.ctid
+ -> Seq Scan on public.a
+ Output: a.i, (SubPlan expr_1), a.ctid
+ SubPlan expr_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 5c11bc6496f..aac89ffdde8 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 00000000000..30a83e04058
--- /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_update { UPDATE a SET i = i + 1; }
+step s0_commit { COMMIT; }
+
+session s1
+step s1_begin { BEGIN ISOLATION LEVEL READ COMMITTED; }
+step s1_tuplock {
+ -- Verify if the sub-select has a foreign-join plan
+ 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
diff --git a/src/include/executor/execScan.h b/src/include/executor/execScan.h
index 837ea7785bb..2003cbc7ed5 100644
--- a/src/include/executor/execScan.h
+++ b/src/include/executor/execScan.h
@@ -49,16 +49,24 @@ 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.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-13 20:18 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-10-13 20:18 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Sun, Oct 12, 2025 at 11:06 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Thu, Oct 9, 2025 at 11:16 AM Michael Paquier <michael@paquier.xyz> wrote:
> > On Wed, Oct 08, 2025 at 03:09:52PM +0900, Etsuro Fujita wrote:
> > > Ok, will do.
> >
> > Cool. Thanks, Fujita-san.
>
> Here is an updated version of the patch, labelled v7, in which I
> modified .gitignore a bit further to include tmp_check_iso as well,
> did a bit of editorialization (eg, indentation in the spec file),
> rebased the patch against HEAD, and updated the commit message.
>
> If there are no objections, I'll push this version and back-patch it
> to all supported branches.
Thank you for updating the patch! LGTM.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-14 04:28 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Michael Paquier @ 2025-10-14 04:28 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Mon, Oct 13, 2025 at 01:18:34PM -0700, Masahiko Sawada wrote:
> On Sun, Oct 12, 2025 at 11:06 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> If there are no objections, I'll push this version and back-patch it
>> to all supported branches.
>
> Thank you for updating the patch! LGTM.
That works for me as well, for what it's worth.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aO3RV6ZcpGZmYRI_@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-15 08:22 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 42+ messages in thread
From: Etsuro Fujita @ 2025-10-15 08:22 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Masahiko Sawada <sawada.mshk@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Tue, Oct 14, 2025 at 1:28 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Oct 13, 2025 at 01:18:34PM -0700, Masahiko Sawada wrote:
> > On Sun, Oct 12, 2025 at 11:06 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> >> If there are no objections, I'll push this version and back-patch it
> >> to all supported branches.
> >
> > Thank you for updating the patch! LGTM.
>
> That works for me as well, for what it's worth.
Done.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-15 18:35 Masahiko Sawada <sawada.mshk@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 42+ messages in thread
From: Masahiko Sawada @ 2025-10-15 18:35 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Oct 15, 2025 at 1:23 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Tue, Oct 14, 2025 at 1:28 PM Michael Paquier <michael@paquier.xyz> wrote:
> > On Mon, Oct 13, 2025 at 01:18:34PM -0700, Masahiko Sawada wrote:
> > > On Sun, Oct 12, 2025 at 11:06 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > >> If there are no objections, I'll push this version and back-patch it
> > >> to all supported branches.
> > >
> > > Thank you for updating the patch! LGTM.
> >
> > That works for me as well, for what it's worth.
>
> Done.
Thank you!
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 42+ messages in thread
* Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c
@ 2025-10-16 01:53 Michael Paquier <michael@paquier.xyz>
parent: Masahiko Sawada <sawada.mshk@gmail.com>
0 siblings, 0 replies; 42+ messages in thread
From: Michael Paquier @ 2025-10-16 01:53 UTC (permalink / raw)
To: Masahiko Sawada <sawada.mshk@gmail.com>; +Cc: Etsuro Fujita <etsuro.fujita@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; kristianlejao@gmail.com
On Wed, Oct 15, 2025 at 11:35:07AM -0700, Masahiko Sawada wrote:
> On Wed, Oct 15, 2025 at 1:23 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> Done.
>
> Thank you!
+1.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aPBQA-1BImfpX1GH@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 42+ messages in thread
end of thread, other threads:[~2025-10-16 01:53 UTC | newest]
Thread overview: 42+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-08-05 18:00 TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c Masahiko Sawada <sawada.mshk@gmail.com>
2025-08-06 11:30 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-08-22 10:59 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-08-22 20:53 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-09-17 10:42 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-09-18 00:24 ` Michael Paquier <michael@paquier.xyz>
2025-09-23 21:41 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-09-23 23:39 ` Michael Paquier <michael@paquier.xyz>
2025-09-24 10:28 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-09-24 16:45 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-09-24 23:14 ` Michael Paquier <michael@paquier.xyz>
2025-09-25 10:55 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-09-26 13:34 ` Álvaro Herrera <alvherre@kurilemu.de>
2025-09-28 08:55 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-01 10:53 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-02 11:01 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-02 22:20 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-10-03 00:57 ` Michael Paquier <michael@paquier.xyz>
2025-10-03 11:00 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-03 21:21 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-10-05 17:15 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-06 03:37 ` Michael Paquier <michael@paquier.xyz>
2025-10-07 10:59 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-07 17:25 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-10-08 00:05 ` Michael Paquier <michael@paquier.xyz>
2025-10-08 06:09 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-09 02:16 ` Michael Paquier <michael@paquier.xyz>
2025-10-12 18:05 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-13 20:18 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-10-14 04:28 ` Michael Paquier <michael@paquier.xyz>
2025-10-15 08:22 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-10-15 18:35 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-10-16 01:53 ` Michael Paquier <michael@paquier.xyz>
2025-08-07 03:38 ` Michael Paquier <michael@paquier.xyz>
2025-08-07 17:29 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-08-07 23:39 ` Michael Paquier <michael@paquier.xyz>
2025-08-08 07:15 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-08-18 06:58 ` Michael Paquier <michael@paquier.xyz>
2025-08-18 07:02 ` Michael Paquier <michael@paquier.xyz>
2025-08-19 18:04 ` Masahiko Sawada <sawada.mshk@gmail.com>
2025-08-18 08:42 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2025-08-19 00:08 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox