postgres_fdw: Add more test coverage for EvalPlanQual testing
Started by Etsuro Fujita2 months ago2 messages
Hi,
In [1]/messages/by-id/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com it was pointed out that there is no test coverage for
postgresRecheckForeignScan. So I would like to propose to add test
cases to cover that function (and related core functions like
ForeignRecheck), as promised in that thread. Please find attached a
patch. (Note that commit 12609fbac, which fixes an EPQ issue reported
in [1]/messages/by-id/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com, added a test case for ExecScanFetch(), but didn't add any test
cases for that function.)
In the patch I modified all permutations, including existing one, to
use per-session setup doing "BEGIN ISOLATION LEVEL READ COMMITTED",
for simplicity, and modified existing step names/comments a little bit
to match new ones, for consistency.
Like commit 12609fbac, I would also like to propose to back-patch this
to all supported versions.
Best regards,
Etsuro Fujita
[1]: /messages/by-id/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com
Attachments:
postgres_fdw-Add-more-EPQ-test-coverage.patchapplication/octet-stream; name=postgres_fdw-Add-more-EPQ-test-coverage.patchDownload
diff --git a/contrib/postgres_fdw/expected/eval_plan_qual.out b/contrib/postgres_fdw/expected/eval_plan_qual.out
index f3e3a22b336..5361fe6f329 100644
--- a/contrib/postgres_fdw/expected/eval_plan_qual.out
+++ b/contrib/postgres_fdw/expected/eval_plan_qual.out
@@ -1,11 +1,105 @@
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
+starting permutation: s0_update_l s1_tuplock_l_0 s0_commit s1_commit
+step s0_update_l: UPDATE l SET i = i + 1;
+step s1_tuplock_l_0:
+ EXPLAIN (VERBOSE, COSTS OFF)
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.i = 123 FOR UPDATE OF l;
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.i = 123 FOR UPDATE OF l;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock_l_0: <... completed>
+QUERY PLAN
+---------------------------------------------------------------------
+LockRows
+ Output: l.i, l.v, l.ctid, ft.*
+ -> Nested Loop
+ Output: l.i, l.v, l.ctid, ft.*
+ -> Seq Scan on public.l
+ Output: l.i, l.v, l.ctid
+ Filter: (l.i = 123)
+ -> Foreign Scan on public.ft
+ Output: ft.*, ft.i
+ Remote SQL: SELECT i, v FROM public.t WHERE ((i = 123))
+(10 rows)
+
+i|v
+-+-
+(0 rows)
+
+step s1_commit: COMMIT;
+
+starting permutation: s0_update_l s1_tuplock_l_1 s0_commit s1_commit
+step s0_update_l: UPDATE l SET i = i + 1;
+step s1_tuplock_l_1:
+ EXPLAIN (VERBOSE, COSTS OFF)
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.v = 'foo' FOR UPDATE OF l;
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.v = 'foo' FOR UPDATE OF l;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock_l_1: <... completed>
+QUERY PLAN
+-----------------------------------------------------------------------------
+LockRows
+ Output: l.i, l.v, l.ctid, ft.*
+ -> Nested Loop
+ Output: l.i, l.v, l.ctid, ft.*
+ -> Seq Scan on public.l
+ Output: l.i, l.v, l.ctid
+ Filter: (l.v = 'foo'::text)
+ -> Foreign Scan on public.ft
+ Output: ft.*, ft.i
+ Remote SQL: SELECT i, v FROM public.t WHERE ((i = $1::integer))
+(10 rows)
+
+i|v
+-+-
+(0 rows)
+
+step s1_commit: COMMIT;
+
+starting permutation: s0_update_a s1_tuplock_a_0 s0_commit s1_commit
+step s0_update_a: UPDATE a SET i = i + 1;
+step s1_tuplock_a_0:
+ EXPLAIN (VERBOSE, COSTS OFF)
+ SELECT a.i FROM a, fb, fc WHERE a.i = fb.i AND fb.i = fc.i FOR UPDATE OF a;
+ SELECT a.i FROM a, fb, fc WHERE a.i = fb.i AND fb.i = fc.i FOR UPDATE OF a;
+ <waiting ...>
+step s0_commit: COMMIT;
+step s1_tuplock_a_0: <... completed>
+QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+LockRows
+ Output: a.i, a.ctid, fb.*, fc.*
+ -> Nested Loop
+ Output: a.i, a.ctid, fb.*, fc.*
+ Join Filter: (fb.i = a.i)
+ -> Foreign Scan
+ Output: fb.*, fb.i, fc.*, fc.i
+ Relations: (public.fb) INNER JOIN (public.fc)
+ Remote SQL: SELECT CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2.i) END, r2.i, CASE WHEN (r3.*)::text IS NOT NULL THEN ROW(r3.i) END, r3.i FROM (public.b r2 INNER JOIN public.c r3 ON (((r2.i = r3.i))))
+ -> Nested Loop
+ Output: fb.*, fb.i, fc.*, fc.i
+ Join Filter: (fb.i = fc.i)
+ -> Foreign Scan on public.fb
+ Output: fb.*, fb.i
+ Remote SQL: SELECT i FROM public.b ORDER BY i ASC NULLS LAST
+ -> Foreign Scan on public.fc
+ Output: fc.*, fc.i
+ Remote SQL: SELECT i FROM public.c
+ -> Seq Scan on public.a
+ Output: a.i, a.ctid
+(20 rows)
+
+i
+-
+(0 rows)
+
+step s1_commit: COMMIT;
+
+starting permutation: s0_update_a s1_tuplock_a_1 s0_commit s1_commit
+step s0_update_a: UPDATE a SET i = i + 1;
+step s1_tuplock_a_1:
EXPLAIN (VERBOSE, COSTS OFF)
SELECT a.i,
(SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
@@ -15,7 +109,7 @@ step s1_tuplock:
FROM a FOR UPDATE;
<waiting ...>
step s0_commit: COMMIT;
-step s1_tuplock: <... completed>
+step s1_tuplock_a_1: <... completed>
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------
LockRows
diff --git a/contrib/postgres_fdw/specs/eval_plan_qual.spec b/contrib/postgres_fdw/specs/eval_plan_qual.spec
index 30a83e04058..07f15270a19 100644
--- a/contrib/postgres_fdw/specs/eval_plan_qual.spec
+++ b/contrib/postgres_fdw/specs/eval_plan_qual.spec
@@ -6,12 +6,22 @@ setup
BEGIN
EXECUTE $$CREATE SERVER loopback FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (dbname '$$||current_database()||$$',
- port '$$||current_setting('port')||$$'
+ port '$$||current_setting('port')||$$',
+ use_remote_estimate 'true'
)$$;
END;
$d$;
CREATE USER MAPPING FOR PUBLIC SERVER loopback;
+ CREATE TABLE l (i int, v text);
+ CREATE TABLE t (i int, v text);
+ CREATE FOREIGN TABLE ft (i int, v text) SERVER loopback OPTIONS (table_name 't');
+
+ INSERT INTO l VALUES (123, 'foo'), (456, 'bar'), (789, 'baz');
+ INSERT INTO t SELECT i, to_char(i, 'FM0000') FROM generate_series(0, 999) i;
+ CREATE INDEX t_idx ON t (i);
+ ANALYZE l, t;
+
CREATE TABLE a (i int);
CREATE TABLE b (i int);
CREATE TABLE c (i int);
@@ -21,10 +31,13 @@ setup
INSERT INTO a VALUES (1);
INSERT INTO b VALUES (1);
INSERT INTO c VALUES (1);
+ ANALYZE a, b, c;
}
teardown
{
+ DROP TABLE l;
+ DROP TABLE t;
DROP TABLE a;
DROP TABLE b;
DROP TABLE c;
@@ -32,14 +45,38 @@ teardown
}
session s0
-step s0_begin { BEGIN ISOLATION LEVEL READ COMMITTED; }
-step s0_update { UPDATE a SET i = i + 1; }
+setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
+step s0_update_l { UPDATE l SET i = i + 1; }
+step s0_update_a { 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
+setup { BEGIN ISOLATION LEVEL READ COMMITTED; }
+
+# Test for EPQ with a foreign scan pushing down a qual
+step s1_tuplock_l_0 {
+ EXPLAIN (VERBOSE, COSTS OFF)
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.i = 123 FOR UPDATE OF l;
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.i = 123 FOR UPDATE OF l;
+}
+
+# Same test, except that the qual is parameterized
+step s1_tuplock_l_1 {
+ EXPLAIN (VERBOSE, COSTS OFF)
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.v = 'foo' FOR UPDATE OF l;
+ SELECT l.* FROM l, ft WHERE l.i = ft.i AND l.v = 'foo' FOR UPDATE OF l;
+}
+
+# Test for EPQ with a foreign scan pushing down a join
+step s1_tuplock_a_0 {
+ EXPLAIN (VERBOSE, COSTS OFF)
+ SELECT a.i FROM a, fb, fc WHERE a.i = fb.i AND fb.i = fc.i FOR UPDATE OF a;
+ SELECT a.i FROM a, fb, fc WHERE a.i = fb.i AND fb.i = fc.i FOR UPDATE OF a;
+}
+
+# Same test, except that the join is contained in a SubLink sub-select, not
+# in the main query
+step s1_tuplock_a_1 {
EXPLAIN (VERBOSE, COSTS OFF)
SELECT a.i,
(SELECT 1 FROM fb, fc WHERE a.i = fb.i AND fb.i = fc.i)
@@ -48,8 +85,18 @@ step s1_tuplock {
(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 checks properly rechecking a pushed-down qual.
+permutation s0_update_l s1_tuplock_l_0 s0_commit s1_commit
+
+# This test checks properly rechecking a pushed-down parameterized qual.
+permutation s0_update_l s1_tuplock_l_1 s0_commit s1_commit
+
+# This test checks properly rechecking a pushed-down join.
+permutation s0_update_a s1_tuplock_a_0 s0_commit s1_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
+permutation s0_update_a s1_tuplock_a_1 s0_commit s1_commit
Re: postgres_fdw: Add more test coverage for EvalPlanQual testing
On Sat, Nov 1, 2025 at 8:18 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
In [1] it was pointed out that there is no test coverage for
postgresRecheckForeignScan. So I would like to propose to add test
cases to cover that function (and related core functions like
ForeignRecheck), as promised in that thread. Please find attached a
patch. (Note that commit 12609fbac, which fixes an EPQ issue reported
in [1], added a test case for ExecScanFetch(), but didn't add any test
cases for that function.)In the patch I modified all permutations, including existing one, to
use per-session setup doing "BEGIN ISOLATION LEVEL READ COMMITTED",
for simplicity, and modified existing step names/comments a little bit
to match new ones, for consistency.Like commit 12609fbac, I would also like to propose to back-patch this
to all supported versions.
There seemed to be no objections, so pushed and back-patched.
Best regards,
Etsuro Fujita