Skip to content

Commit f39f3e0

Browse files
author
Etsuro Fujita
committed
postgres_fdw: Avoid "cursor can only scan forward" error.
Commit d844cd7 disallowed rewind in a non-scrollable cursor to resolve anomalies arising from such a cursor operation. However, this failed to take into account the assumption in postgres_fdw that when rescanning a foreign relation, it can rewind the cursor created for scanning the foreign relation without specifying the SCROLL option, regardless of its scrollability, causing this error when it tried to do such a rewind in a non-scrollable cursor. Fix by modifying postgres_fdw to instead recreate the cursor, regardless of its scrollability, when rescanning the foreign relation. (If we had a way to check its scrollability, we could improve this by rewinding it if it is scrollable and recreating it if not, but we do not have it, so this commit modifies it to recreate it in any case.) Per bug #17889 from Eric Cyr. Devrim Gunduz also reported this problem. Back-patch to v15 where that commit enforced the prohibition. Reviewed by Tom Lane. Discussion: https://postgr.es/m/17889-e8c39a251d258dda%40postgresql.org Discussion: https://postgr.es/m/b415ac3255f8352d1ea921cf3b7ba39e0587768a.camel%40gunduz.org
1 parent bc9d706 commit f39f3e0

File tree

3 files changed

+85
-5
lines changed

3 files changed

+85
-5
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6510,6 +6510,51 @@ SELECT * FROM ft1 ORDER BY c6 ASC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
65106510
40 | 42 | 00040_trig_update | Tue Feb 10 00:00:00 1970 PST | Tue Feb 10 00:00:00 1970 | 0 | 0 | foo
65116511
(10 rows)
65126512

6513+
-- Test ReScan code path that recreates the cursor even when no parameters
6514+
-- change (bug #17889)
6515+
CREATE TABLE loct1 (c1 int);
6516+
CREATE TABLE loct2 (c1 int, c2 text);
6517+
INSERT INTO loct1 VALUES (1001);
6518+
INSERT INTO loct1 VALUES (1002);
6519+
INSERT INTO loct2 SELECT id, to_char(id, 'FM0000') FROM generate_series(1, 1000) id;
6520+
INSERT INTO loct2 VALUES (1001, 'foo');
6521+
INSERT INTO loct2 VALUES (1002, 'bar');
6522+
CREATE FOREIGN TABLE remt2 (c1 int, c2 text) SERVER loopback OPTIONS (table_name 'loct2');
6523+
ANALYZE loct1;
6524+
ANALYZE remt2;
6525+
SET enable_mergejoin TO false;
6526+
SET enable_hashjoin TO false;
6527+
SET enable_material TO false;
6528+
EXPLAIN (VERBOSE, COSTS OFF)
6529+
UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
6530+
QUERY PLAN
6531+
--------------------------------------------------------------------------------
6532+
Update on public.remt2
6533+
Output: remt2.c1, remt2.c2
6534+
Remote SQL: UPDATE public.loct2 SET c2 = $2 WHERE ctid = $1 RETURNING c1, c2
6535+
-> Nested Loop
6536+
Output: (remt2.c2 || remt2.c2), remt2.ctid, remt2.*, loct1.ctid
6537+
Join Filter: (remt2.c1 = loct1.c1)
6538+
-> Seq Scan on public.loct1
6539+
Output: loct1.ctid, loct1.c1
6540+
-> Foreign Scan on public.remt2
6541+
Output: remt2.c2, remt2.ctid, remt2.*, remt2.c1
6542+
Remote SQL: SELECT c1, c2, ctid FROM public.loct2 FOR UPDATE
6543+
(11 rows)
6544+
6545+
UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
6546+
c1 | c2
6547+
------+--------
6548+
1001 | foofoo
6549+
1002 | barbar
6550+
(2 rows)
6551+
6552+
RESET enable_mergejoin;
6553+
RESET enable_hashjoin;
6554+
RESET enable_material;
6555+
DROP FOREIGN TABLE remt2;
6556+
DROP TABLE loct1;
6557+
DROP TABLE loct2;
65136558
-- ===================================================================
65146559
-- test check constraints
65156560
-- ===================================================================

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1663,9 +1663,12 @@ postgresReScanForeignScan(ForeignScanState *node)
16631663

16641664
/*
16651665
* If any internal parameters affecting this node have changed, we'd
1666-
* better destroy and recreate the cursor. Otherwise, rewinding it should
1667-
* be good enough. If we've only fetched zero or one batch, we needn't
1668-
* even rewind the cursor, just rescan what we have.
1666+
* better destroy and recreate the cursor. Otherwise, if the remote
1667+
* server is v14 or older, rewinding it should be good enough; if not,
1668+
* rewind is only allowed for scrollable cursors, but we don't have a way
1669+
* to check the scrollability of it, so destroy and recreate it in any
1670+
* case. If we've only fetched zero or one batch, we needn't even rewind
1671+
* the cursor, just rescan what we have.
16691672
*/
16701673
if (node->ss.ps.chgParam != NULL)
16711674
{
@@ -1675,8 +1678,15 @@ postgresReScanForeignScan(ForeignScanState *node)
16751678
}
16761679
else if (fsstate->fetch_ct_2 > 1)
16771680
{
1678-
snprintf(sql, sizeof(sql), "MOVE BACKWARD ALL IN c%u",
1679-
fsstate->cursor_number);
1681+
if (PQserverVersion(fsstate->conn) < 150000)
1682+
snprintf(sql, sizeof(sql), "MOVE BACKWARD ALL IN c%u",
1683+
fsstate->cursor_number);
1684+
else
1685+
{
1686+
fsstate->cursor_exists = false;
1687+
snprintf(sql, sizeof(sql), "CLOSE c%u",
1688+
fsstate->cursor_number);
1689+
}
16801690
}
16811691
else
16821692
{

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,31 @@ SELECT * FROM ft1 ORDER BY c6 DESC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
14731473
EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 ORDER BY c6 ASC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
14741474
SELECT * FROM ft1 ORDER BY c6 ASC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
14751475

1476+
-- Test ReScan code path that recreates the cursor even when no parameters
1477+
-- change (bug #17889)
1478+
CREATE TABLE loct1 (c1 int);
1479+
CREATE TABLE loct2 (c1 int, c2 text);
1480+
INSERT INTO loct1 VALUES (1001);
1481+
INSERT INTO loct1 VALUES (1002);
1482+
INSERT INTO loct2 SELECT id, to_char(id, 'FM0000') FROM generate_series(1, 1000) id;
1483+
INSERT INTO loct2 VALUES (1001, 'foo');
1484+
INSERT INTO loct2 VALUES (1002, 'bar');
1485+
CREATE FOREIGN TABLE remt2 (c1 int, c2 text) SERVER loopback OPTIONS (table_name 'loct2');
1486+
ANALYZE loct1;
1487+
ANALYZE remt2;
1488+
SET enable_mergejoin TO false;
1489+
SET enable_hashjoin TO false;
1490+
SET enable_material TO false;
1491+
EXPLAIN (VERBOSE, COSTS OFF)
1492+
UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
1493+
UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
1494+
RESET enable_mergejoin;
1495+
RESET enable_hashjoin;
1496+
RESET enable_material;
1497+
DROP FOREIGN TABLE remt2;
1498+
DROP TABLE loct1;
1499+
DROP TABLE loct2;
1500+
14761501
-- ===================================================================
14771502
-- test check constraints
14781503
-- ===================================================================

0 commit comments

Comments
 (0)