postgres_fdw: Avoid "cursor can only scan forward" error.
authorEtsuro Fujita <[email protected]>
Fri, 19 Jul 2024 04:15:00 +0000 (13:15 +0900)
committerEtsuro Fujita <[email protected]>
Fri, 19 Jul 2024 04:15:00 +0000 (13:15 +0900)
Commit d844cd75a 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

contrib/postgres_fdw/expected/postgres_fdw.out
contrib/postgres_fdw/postgres_fdw.c
contrib/postgres_fdw/sql/postgres_fdw.sql

index 0cc77190dc41df887e2d1424d1cc5795b9b41300..8f0886f2ff66af87a0e409ef8cae160d134a262f 100644 (file)
@@ -6853,6 +6853,51 @@ SELECT * FROM ft1 ORDER BY c6 ASC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
    40 |  42 | 00040_trig_update | Tue Feb 10 00:00:00 1970 PST | Tue Feb 10 00:00:00 1970 | 0    | 0          | foo
 (10 rows)
 
+-- Test ReScan code path that recreates the cursor even when no parameters
+-- change (bug #17889)
+CREATE TABLE loct1 (c1 int);
+CREATE TABLE loct2 (c1 int, c2 text);
+INSERT INTO loct1 VALUES (1001);
+INSERT INTO loct1 VALUES (1002);
+INSERT INTO loct2 SELECT id, to_char(id, 'FM0000') FROM generate_series(1, 1000) id;
+INSERT INTO loct2 VALUES (1001, 'foo');
+INSERT INTO loct2 VALUES (1002, 'bar');
+CREATE FOREIGN TABLE remt2 (c1 int, c2 text) SERVER loopback OPTIONS (table_name 'loct2');
+ANALYZE loct1;
+ANALYZE remt2;
+SET enable_mergejoin TO false;
+SET enable_hashjoin TO false;
+SET enable_material TO false;
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
+                                   QUERY PLAN                                   
+--------------------------------------------------------------------------------
+ Update on public.remt2
+   Output: remt2.c1, remt2.c2
+   Remote SQL: UPDATE public.loct2 SET c2 = $2 WHERE ctid = $1 RETURNING c1, c2
+   ->  Nested Loop
+         Output: (remt2.c2 || remt2.c2), remt2.ctid, remt2.*, loct1.ctid
+         Join Filter: (remt2.c1 = loct1.c1)
+         ->  Seq Scan on public.loct1
+               Output: loct1.ctid, loct1.c1
+         ->  Foreign Scan on public.remt2
+               Output: remt2.c2, remt2.ctid, remt2.*, remt2.c1
+               Remote SQL: SELECT c1, c2, ctid FROM public.loct2 FOR UPDATE
+(11 rows)
+
+UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
+  c1  |   c2   
+------+--------
+ 1001 | foofoo
+ 1002 | barbar
+(2 rows)
+
+RESET enable_mergejoin;
+RESET enable_hashjoin;
+RESET enable_material;
+DROP FOREIGN TABLE remt2;
+DROP TABLE loct1;
+DROP TABLE loct2;
 -- ===================================================================
 -- test check constraints
 -- ===================================================================
index 0bb9a5ae8f682af44607174789dcf1133f06fd7f..fc65d81e2177ada3671a1fd1296280cd64a5e046 100644 (file)
@@ -1662,9 +1662,12 @@ postgresReScanForeignScan(ForeignScanState *node)
 
    /*
     * If any internal parameters affecting this node have changed, we'd
-    * better destroy and recreate the cursor.  Otherwise, rewinding it should
-    * be good enough.  If we've only fetched zero or one batch, we needn't
-    * even rewind the cursor, just rescan what we have.
+    * better destroy and recreate the cursor.  Otherwise, if the remote
+    * server is v14 or older, rewinding it should be good enough; if not,
+    * rewind is only allowed for scrollable cursors, but we don't have a way
+    * to check the scrollability of it, so destroy and recreate it in any
+    * case.  If we've only fetched zero or one batch, we needn't even rewind
+    * the cursor, just rescan what we have.
     */
    if (node->ss.ps.chgParam != NULL)
    {
@@ -1674,8 +1677,15 @@ postgresReScanForeignScan(ForeignScanState *node)
    }
    else if (fsstate->fetch_ct_2 > 1)
    {
-       snprintf(sql, sizeof(sql), "MOVE BACKWARD ALL IN c%u",
-                fsstate->cursor_number);
+       if (PQserverVersion(fsstate->conn) < 150000)
+           snprintf(sql, sizeof(sql), "MOVE BACKWARD ALL IN c%u",
+                    fsstate->cursor_number);
+       else
+       {
+           fsstate->cursor_exists = false;
+           snprintf(sql, sizeof(sql), "CLOSE c%u",
+                    fsstate->cursor_number);
+       }
    }
    else
    {
index b57f8cfda68e2b5589abade588c6d7bc4b3cd48d..733c10377121d012bb24db4fec6a2b7a176f164f 100644 (file)
@@ -1647,6 +1647,31 @@ SELECT * FROM ft1 ORDER BY c6 DESC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
 EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 ORDER BY c6 ASC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
 SELECT * FROM ft1 ORDER BY c6 ASC NULLS FIRST, c1 OFFSET 15 LIMIT 10;
 
+-- Test ReScan code path that recreates the cursor even when no parameters
+-- change (bug #17889)
+CREATE TABLE loct1 (c1 int);
+CREATE TABLE loct2 (c1 int, c2 text);
+INSERT INTO loct1 VALUES (1001);
+INSERT INTO loct1 VALUES (1002);
+INSERT INTO loct2 SELECT id, to_char(id, 'FM0000') FROM generate_series(1, 1000) id;
+INSERT INTO loct2 VALUES (1001, 'foo');
+INSERT INTO loct2 VALUES (1002, 'bar');
+CREATE FOREIGN TABLE remt2 (c1 int, c2 text) SERVER loopback OPTIONS (table_name 'loct2');
+ANALYZE loct1;
+ANALYZE remt2;
+SET enable_mergejoin TO false;
+SET enable_hashjoin TO false;
+SET enable_material TO false;
+EXPLAIN (VERBOSE, COSTS OFF)
+UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
+UPDATE remt2 SET c2 = remt2.c2 || remt2.c2 FROM loct1 WHERE loct1.c1 = remt2.c1 RETURNING remt2.*;
+RESET enable_mergejoin;
+RESET enable_hashjoin;
+RESET enable_material;
+DROP FOREIGN TABLE remt2;
+DROP TABLE loct1;
+DROP TABLE loct2;
+
 -- ===================================================================
 -- test check constraints
 -- ===================================================================