pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
12+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2024-07-19 5c571a34d0 935fe79ea1 d97f2ee50e f39f3e0fbe postgres_fdw: Avoid "cursor can only scan forward" error.
* BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2023-04-06 18:42 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 12+ messages in thread
From: PG Bug reporting form @ 2023-04-06 18:42 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: eric.cyr@gmail.com
The following bug has been logged on the website:
Bug reference: 17889
Logged by: Eric Cyr
Email address: eric.cyr@gmail.com
PostgreSQL version: 15.2
Operating system: Ubuntu 22.04.2 LTS, macOS 13.3
Description:
Hello,
I stumbled upon a strange case that causes an error in version 15.
Hopefully the following will be helpful.
Thank you very much
Have a great day
--
The scenario is the following:
On DB 1 have
3 tables, a view using a where clause on table 1 and a view using a inner
join on table 1 and 2.
On DB 2 have
A fdw server(with use_remote_estimate set to true), 3 foreign tables, one
for each view and one for table 3 and a view on the foreign table of view of
table 1 (with a cte using a function and used in where clause)
On DB 2 execute a SELECT on view of foreign table 1 with a join on foreign
table 2 with a where clause using a subquery on foreign table 3.
If the SELECT would return an amount of rows equal or greater than the fetch
size of foreign table of view of table 1 the error will occur.
--
The same scenario was tested on Postgres 10 and 14, both worked without
error.
--
The error is the following:
ERROR: cursor can only scan forward Hint:
Declare it with SCROLL option to enable backward scan.
Where: remote SQL command: MOVE BACKWARD ALL IN c3
--
The move backward happens in postgres_fdw.c 1676:1680
else if (fsstate->fetch_ct_2 > 1)
{
snprintf(sql, sizeof(sql), "MOVE BACKWARD ALL IN c%u",
fsstate->cursor_number);
}
--
The EXPLAIN result is the following:
Nested Loop Left Join (cost=224.47..923.78 rows=1 width=68)
" Output: a.def, b.other_def, a.c_fk"
Join Filter: (b.a_fk = a.pk)
-> Foreign Scan on schema_test.test_a_table_view_foreign a
(cost=100.00..773.45 rows=1 width=40)
" Output: a.pk, a.c_fk, a.def, a.flag"
Filter: ((SubPlan 1) = 'cdef1'::text)
" Remote SQL: SELECT pk, c_fk, def FROM
schema_test.test_a_table_view"
SubPlan 1
-> Foreign Scan on schema_test.test_c_table_foreign c
(cost=100.16..108.20 rows=1 width=32)
Output: c.def
Remote SQL: SELECT def FROM schema_test.test_c_table WHERE
((pk = $1::integer))
-> Foreign Scan on schema_test.test_b_table_view_foreign b
(cost=124.47..150.19 rows=6 width=44)
" Output: NULL::integer, b.a_fk, NULL::integer, b.other_def"
" Remote SQL: SELECT a_fk, other_def FROM
schema_test.test_b_table_view WHERE ((other_id = $1::integer))"
InitPlan 2 (returns $1)
-> Result (cost=0.00..0.26 rows=1 width=4)
Output: schema_test.test_function()
--
Here is the SQL used to recreate the issue:
--
CREATE USER test_user PASSWORD '1234';
CREATE USER test_fdw_user PASSWORD '1234';
CREATE DATABASE db_test_1;
CREATE DATABASE db_test_2;
--
--
--
\c db_test_1
--
CREATE SCHEMA schema_test;
GRANT ALL ON SCHEMA schema_test TO public;
--
CREATE TABLE schema_test.test_c_table (
pk integer NOT NULL,
other_id integer,
def text NOT NULL,
flag integer NOT NULL
);
ALTER TABLE schema_test.test_c_table
ADD CONSTRAINT test_c_table_pk PRIMARY KEY (pk);
GRANT ALL ON TABLE schema_test.test_c_table TO public;
INSERT INTO schema_test.test_c_table(pk, other_id, def, flag) VALUES (1, 1,
'cdef1', 1);
--
CREATE TABLE schema_test.test_a_table (
pk integer NOT NULL,
c_fk integer NOT NULL,
def text,
flag integer NOT NULL
);
ALTER TABLE schema_test.test_a_table
ADD CONSTRAINT test_a_table_pk PRIMARY KEY (pk);
ALTER TABLE schema_test.test_a_table
ADD CONSTRAINT test_a_table_fk FOREIGN KEY (c_fk) REFERENCES
schema_test.test_c_table(pk);
GRANT ALL ON TABLE schema_test.test_a_table TO public;
INSERT INTO schema_test.test_a_table(pk, c_fk, def, flag) VALUES (101, 1,
'adef2_101', 1);
INSERT INTO schema_test.test_a_table(pk, c_fk, def, flag) VALUES (102, 1,
'adef2_102', 1);
--
CREATE VIEW schema_test.test_a_table_view
AS
SELECT
a.pk,
a.c_fk,
a.def,
a.flag
FROM schema_test.test_a_table a
WHERE a.flag = 1
;
GRANT ALL ON TABLE schema_test.test_a_table_view TO public;
--
CREATE TABLE schema_test.test_b_table (
pk integer NOT NULL,
a_fk integer NOT NULL,
other_id integer NOT NULL,
other_def text NOT NULL
);
ALTER TABLE schema_test.test_b_table
ADD CONSTRAINT test_b_table_pk PRIMARY KEY (pk);
ALTER TABLE schema_test.test_b_table
ADD CONSTRAINT test_b_table_fk FOREIGN KEY (a_fk) REFERENCES
schema_test.test_a_table(pk);
GRANT ALL ON TABLE schema_test.test_b_table TO public;
INSERT INTO schema_test.test_b_table(pk, a_fk, other_id, other_def) VALUES
(1011, 101, 1, 'bdef-101-1');
INSERT INTO schema_test.test_b_table(pk, a_fk, other_id, other_def) VALUES
(1012, 101, 2, 'bdef-101-2');
INSERT INTO schema_test.test_b_table(pk, a_fk, other_id, other_def) VALUES
(1021, 102, 1, 'bdef-102-1');
INSERT INTO schema_test.test_b_table(pk, a_fk, other_id, other_def) VALUES
(1022, 102, 2, 'bdef-102-2');
--
CREATE VIEW schema_test.test_b_table_view
AS
SELECT
b.pk,
b.a_fk,
b.other_id,
b.other_def
FROM schema_test.test_b_table b
INNER JOIN schema_test.test_a_table a
ON b.a_fk = a.pk
;
GRANT ALL ON TABLE schema_test.test_b_table_view TO public;
--
--
--
\c db_test_2
--
CREATE SCHEMA schema_test;
GRANT ALL ON SCHEMA schema_test TO public;
--
CREATE EXTENSION IF NOT EXISTS postgres_fdw;
CREATE SERVER db_test_1_fdw FOREIGN DATA WRAPPER postgres_fdw OPTIONS (
dbname 'db_test_1',
host 'localhost',
use_remote_estimate 'true'
);
CREATE USER MAPPING FOR public SERVER db_test_1_fdw OPTIONS (
password '1234',
"user" 'test_fdw_user'
);
--
CREATE FOREIGN TABLE schema_test.test_a_table_view_foreign (
pk integer,
c_fk integer,
def text,
flag integer
)
SERVER db_test_1_fdw
OPTIONS (
schema_name 'schema_test',
table_name 'test_a_table_view'
);
--
CREATE FOREIGN TABLE schema_test.test_b_table_view_foreign (
pk integer,
a_fk integer,
other_id integer,
other_def text
)
SERVER db_test_1_fdw
OPTIONS (
schema_name 'schema_test',
table_name 'test_b_table_view',
fetch_size '100'
);
--
CREATE FUNCTION schema_test.test_function() RETURNS integer
LANGUAGE plpgsql STABLE
AS $$
BEGIN
return 1;
END;
$$
;
--
CREATE VIEW schema_test.test_b_table_view
AS
WITH t_test AS (
SELECT schema_test.test_function() AS other_id
)
SELECT
b.pk,
b.a_fk,
b.other_id,
b.other_def
FROM schema_test.test_b_table_view_foreign b
WHERE b.other_id = ( SELECT t.other_id FROM t_test t)
;
GRANT ALL ON TABLE schema_test.test_b_table_view TO public;
--
CREATE FOREIGN TABLE schema_test.test_c_table_foreign (
pk integer,
other_id integer,
def text,
flag integer
)
SERVER db_test_1_fdw
OPTIONS (
schema_name 'schema_test',
table_name 'test_c_table'
);
--
--
--
\c db_test_2
-- when nb returned rows >= fetch_size
-- fails with error -> [55000] ERROR: cursor can only scan forward Hint:
Declare it with SCROLL option to enable backward scan. Where: remote SQL
command: MOVE BACKWARD ALL IN c3
ALTER FOREIGN TABLE schema_test.test_b_table_view_foreign OPTIONS ( SET
fetch_size '2' );
SELECT a.def AS adef,
b.other_def AS bdef,
a.c_fk
FROM schema_test.test_a_table_view_foreign a
LEFT JOIN schema_test.test_b_table_view b
ON b.a_fk = a.pk
WHERE (SELECT c.def
FROM schema_test.test_c_table_foreign c
WHERE c.pk = a.c_fk) = 'cdef1'
;
-- when nb returned rows < fetch_size
-- succeed
ALTER FOREIGN TABLE schema_test.test_b_table_view_foreign OPTIONS ( SET
fetch_size '100' );
SELECT a.def AS adef,
b.other_def AS bdef,
a.c_fk
FROM schema_test.test_a_table_view_foreign a
LEFT JOIN schema_test.test_b_table_view b
ON b.a_fk = a.pk
WHERE (SELECT c.def
FROM schema_test.test_c_table_foreign c
WHERE c.pk = a.c_fk) = 'cdef1'
;
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2023-04-07 09:16 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 12+ messages in thread
From: Etsuro Fujita @ 2023-04-07 09:16 UTC (permalink / raw)
To: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
On Fri, Apr 7, 2023 at 5:21 AM PG Bug reporting form
<noreply@postgresql.org> wrote:
> The scenario is the following:
> On DB 1 have
> 3 tables, a view using a where clause on table 1 and a view using a inner
> join on table 1 and 2.
>
> On DB 2 have
> A fdw server(with use_remote_estimate set to true), 3 foreign tables, one
> for each view and one for table 3 and a view on the foreign table of view of
> table 1 (with a cte using a function and used in where clause)
>
> On DB 2 execute a SELECT on view of foreign table 1 with a join on foreign
> table 2 with a where clause using a subquery on foreign table 3.
>
> If the SELECT would return an amount of rows equal or greater than the fetch
> size of foreign table of view of table 1 the error will occur.
> The same scenario was tested on Postgres 10 and 14, both worked without
> error.
> The error is the following:
>
> ERROR: cursor can only scan forward Hint:
> Declare it with SCROLL option to enable backward scan.
> Where: remote SQL command: MOVE BACKWARD ALL IN c3
Will look into this. Thanks for the report!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2023-04-29 11:36 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Etsuro Fujita @ 2023-04-29 11:36 UTC (permalink / raw)
To: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
I CCed Tom.
On Fri, Apr 7, 2023 at 6:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Fri, Apr 7, 2023 at 5:21 AM PG Bug reporting form
> <noreply@postgresql.org> wrote:
> > The scenario is the following:
> > On DB 1 have
> > 3 tables, a view using a where clause on table 1 and a view using a inner
> > join on table 1 and 2.
> >
> > On DB 2 have
> > A fdw server(with use_remote_estimate set to true), 3 foreign tables, one
> > for each view and one for table 3 and a view on the foreign table of view of
> > table 1 (with a cte using a function and used in where clause)
> >
> > On DB 2 execute a SELECT on view of foreign table 1 with a join on foreign
> > table 2 with a where clause using a subquery on foreign table 3.
> >
> > If the SELECT would return an amount of rows equal or greater than the fetch
> > size of foreign table of view of table 1 the error will occur.
>
> > The same scenario was tested on Postgres 10 and 14, both worked without
> > error.
>
> > The error is the following:
> >
> > ERROR: cursor can only scan forward Hint:
> > Declare it with SCROLL option to enable backward scan.
> > Where: remote SQL command: MOVE BACKWARD ALL IN c3
>
> Will look into this.
Here is a simple reproducer:
create server loopback foreign data wrapper postgres_fdw options
(dbname 'postgres');
create user mapping for current_user server loopback;
create table loc1 (a int, b text);
create table loc2 (a int, b text);
insert into loc1 select i, 'loc1' from generate_series(1, 1000) i;
insert into loc2 select i, 'loc2' from generate_series(1, 2) i;
create foreign table rem1 (a int, b text) server loopback options
(table_name 'loc1');
analyze rem1;
analyze loc2;
set enable_mergejoin to false;
set enable_hashjoin to false;
set enable_material to false;
explain verbose update rem1 set b = rem1.b || rem1.b from loc2 where
rem1.a = loc2.a and random() > 0.0;
QUERY PLAN
-----------------------------------------------------------------------------------
Update on public.rem1 (cost=100.00..308.02 rows=0 width=0)
Remote SQL: UPDATE public.loc1 SET b = $2 WHERE ctid = $1
-> Nested Loop (cost=100.00..308.02 rows=1 width=77)
Output: (rem1.b || rem1.b), rem1.ctid, rem1.*, loc2.ctid
Join Filter: ((rem1.a = loc2.a) AND (random() > '0'::double precision))
-> Seq Scan on public.loc2 (cost=0.00..1.02 rows=2 width=10)
Output: loc2.ctid, loc2.a
-> Foreign Scan on public.rem1 (cost=100.00..136.00
rows=1000 width=48)
Output: rem1.b, rem1.ctid, rem1.*, rem1.a
Remote SQL: SELECT a, b, ctid FROM public.loc1 FOR UPDATE
(10 rows)
update rem1 set b = rem1.b || rem1.b from loc2 where rem1.a = loc2.a
and random() > 0.0;
ERROR: cursor can only scan forward
HINT: Declare it with SCROLL option to enable backward scan.
CONTEXT: remote SQL command: MOVE BACKWARD ALL IN c1
I think that the root cause is in commit d844cd75a, which disallowed
rewinding and then re-fetching forwards in a NO SCROLL cursor. I am
not sure what to do about this issue, but I am wondering whether that
commit is too restrictive, because 1) these examples would work just
fine without that commit, and 2) we still allow
rewind-and-fetch-forwards in a SCROLL cursor even when the query
includes volatile functions.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2023-04-29 11:42 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Etsuro Fujita @ 2023-04-29 11:42 UTC (permalink / raw)
To: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org; +Cc: Tom Lane <tgl@sss.pgh.pa.us>
I resent this, because I forgot to CC it to Tom. :(
On Sat, Apr 29, 2023 at 8:36 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> I CCed Tom.
>
> On Fri, Apr 7, 2023 at 6:16 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > On Fri, Apr 7, 2023 at 5:21 AM PG Bug reporting form
> > <noreply@postgresql.org> wrote:
> > > The scenario is the following:
> > > On DB 1 have
> > > 3 tables, a view using a where clause on table 1 and a view using a inner
> > > join on table 1 and 2.
> > >
> > > On DB 2 have
> > > A fdw server(with use_remote_estimate set to true), 3 foreign tables, one
> > > for each view and one for table 3 and a view on the foreign table of view of
> > > table 1 (with a cte using a function and used in where clause)
> > >
> > > On DB 2 execute a SELECT on view of foreign table 1 with a join on foreign
> > > table 2 with a where clause using a subquery on foreign table 3.
> > >
> > > If the SELECT would return an amount of rows equal or greater than the fetch
> > > size of foreign table of view of table 1 the error will occur.
> >
> > > The same scenario was tested on Postgres 10 and 14, both worked without
> > > error.
> >
> > > The error is the following:
> > >
> > > ERROR: cursor can only scan forward Hint:
> > > Declare it with SCROLL option to enable backward scan.
> > > Where: remote SQL command: MOVE BACKWARD ALL IN c3
> >
> > Will look into this.
>
> Here is a simple reproducer:
>
> create server loopback foreign data wrapper postgres_fdw options
> (dbname 'postgres');
> create user mapping for current_user server loopback;
> create table loc1 (a int, b text);
> create table loc2 (a int, b text);
> insert into loc1 select i, 'loc1' from generate_series(1, 1000) i;
> insert into loc2 select i, 'loc2' from generate_series(1, 2) i;
> create foreign table rem1 (a int, b text) server loopback options
> (table_name 'loc1');
> analyze rem1;
> analyze loc2;
> set enable_mergejoin to false;
> set enable_hashjoin to false;
> set enable_material to false;
>
> explain verbose update rem1 set b = rem1.b || rem1.b from loc2 where
> rem1.a = loc2.a and random() > 0.0;
> QUERY PLAN
> -----------------------------------------------------------------------------------
> Update on public.rem1 (cost=100.00..308.02 rows=0 width=0)
> Remote SQL: UPDATE public.loc1 SET b = $2 WHERE ctid = $1
> -> Nested Loop (cost=100.00..308.02 rows=1 width=77)
> Output: (rem1.b || rem1.b), rem1.ctid, rem1.*, loc2.ctid
> Join Filter: ((rem1.a = loc2.a) AND (random() > '0'::double precision))
> -> Seq Scan on public.loc2 (cost=0.00..1.02 rows=2 width=10)
> Output: loc2.ctid, loc2.a
> -> Foreign Scan on public.rem1 (cost=100.00..136.00
> rows=1000 width=48)
> Output: rem1.b, rem1.ctid, rem1.*, rem1.a
> Remote SQL: SELECT a, b, ctid FROM public.loc1 FOR UPDATE
> (10 rows)
>
> update rem1 set b = rem1.b || rem1.b from loc2 where rem1.a = loc2.a
> and random() > 0.0;
> ERROR: cursor can only scan forward
> HINT: Declare it with SCROLL option to enable backward scan.
> CONTEXT: remote SQL command: MOVE BACKWARD ALL IN c1
>
> I think that the root cause is in commit d844cd75a, which disallowed
> rewinding and then re-fetching forwards in a NO SCROLL cursor. I am
> not sure what to do about this issue, but I am wondering whether that
> commit is too restrictive, because 1) these examples would work just
> fine without that commit, and 2) we still allow
> rewind-and-fetch-forwards in a SCROLL cursor even when the query
> includes volatile functions.
>
> Best regards,
> Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2023-04-29 22:50 Tom Lane <tgl@sss.pgh.pa.us>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2023-04-29 22:50 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
Etsuro Fujita <etsuro.fujita@gmail.com> writes:
>> I think that the root cause is in commit d844cd75a, which disallowed
>> rewinding and then re-fetching forwards in a NO SCROLL cursor. I am
>> not sure what to do about this issue, but I am wondering whether that
>> commit is too restrictive, because 1) these examples would work just
>> fine without that commit, and 2) we still allow
>> rewind-and-fetch-forwards in a SCROLL cursor even when the query
>> includes volatile functions.
Well, the short answer here is that postgres_fdw is depending on
something that has squishy semantics, for the reasons enumerated
in d844cd75a (see also [1]). Maybe we can carve out an exception
that's narrow enough that we can convince ourselves it's not squishy,
but I'm not very sure what the rules should be.
regards, tom lane
[1] https://www.postgresql.org/message-id/201144.1682350828%40sss.pgh.pa.us
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-05 12:49 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 12+ messages in thread
From: Etsuro Fujita @ 2024-07-05 12:49 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
On Sun, Apr 30, 2023 at 7:50 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> >> I think that the root cause is in commit d844cd75a, which disallowed
> >> rewinding and then re-fetching forwards in a NO SCROLL cursor. I am
> >> not sure what to do about this issue, but I am wondering whether that
> >> commit is too restrictive, because 1) these examples would work just
> >> fine without that commit, and 2) we still allow
> >> rewind-and-fetch-forwards in a SCROLL cursor even when the query
> >> includes volatile functions.
>
> Well, the short answer here is that postgres_fdw is depending on
> something that has squishy semantics, for the reasons enumerated
> in d844cd75a (see also [1]).
Understood.
> Maybe we can carve out an exception
> that's narrow enough that we can convince ourselves it's not squishy,
> but I'm not very sure what the rules should be.
I think it would be good if we could relax the restriction on NO
SCROLL cursors so that postgres_fdw can rewind them as before, but I
am not sure we can. I think problematic cases arise when a cursor
query contains FOR UPDATE: as discussed in [1], the cursor would be
idempotent if later commands did not update tuples locked by the
query; but if they did, it would not be idempotent anymore, leading to
reintroducing an anomaly removed by that commit.
To fix, how about instead modifying postgres_fdw so that if the remote
server is v15 or later, it just recreates a cursor when rewinding it
is needed, like the attached? If we had a way to check the
scrollability of a remote cursor created without the SCROLL option, we
could avoid doing so for scrollable cursors, but we do not have it, so
I modified it to recreate it in any case.
Best regards,
Etsuro Fujita
Attachments:
fix-rescan-in-postgres-fdw.patch (application/octet-stream, 5.3K) download
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 1f22309194..b56cdc4492 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -6849,6 +6849,49 @@ 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)
+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
-- ===================================================================
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 0bb9a5ae8f..fc65d81e21 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -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
{
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index b57f8cfda6..73524ed9e3 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1647,6 +1647,29 @@ 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;
+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
-- ===================================================================
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-15 10:51 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Etsuro Fujita @ 2024-07-15 10:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
On Fri, Jul 5, 2024 at 9:49 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> To fix, how about instead modifying postgres_fdw so that if the remote
> server is v15 or later, it just recreates a cursor when rewinding it
> is needed, like the attached? If we had a way to check the
> scrollability of a remote cursor created without the SCROLL option, we
> could avoid doing so for scrollable cursors, but we do not have it, so
> I modified it to recreate it in any case.
Barring objections, I will push this and back-patch to v15.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-15 13:55 Tom Lane <tgl@sss.pgh.pa.us>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2024-07-15 13:55 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> On Fri, Jul 5, 2024 at 9:49 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> To fix, how about instead modifying postgres_fdw so that if the remote
>> server is v15 or later, it just recreates a cursor when rewinding it
>> is needed, like the attached?
Uh ... does this ensure that the data hasn't changed?
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-15 19:45 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 12+ messages in thread
From: Etsuro Fujita @ 2024-07-15 19:45 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
On Mon, Jul 15, 2024 at 10:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> > On Fri, Jul 5, 2024 at 9:49 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> >> To fix, how about instead modifying postgres_fdw so that if the remote
> >> server is v15 or later, it just recreates a cursor when rewinding it
> >> is needed, like the attached?
>
> Uh ... does this ensure that the data hasn't changed?
postgres_fdw opens a remote transaction using REPEATABLE READ or
SERIALIZABLE, so the recreated cursor uses the same snapshot except
changes made by the remote transaction.
This causes eg, a join-UPDATE query where multiple rows join to the
same foreign target row to repeatedly update the target row, as shown
below, which would never happen if rewinding the cursor.
Slightly modified version of the test case in the proposed patch where
the foreign row (1001, ‘foo’) is repeatedly updated:
CREATE TABLE loct1 (c1 int);
CREATE TABLE loct2 (c1 int, c2 text);
INSERT INTO loct1 VALUES (1001);
INSERT INTO loct1 VALUES (1001);
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
1001 | foofoofoofoo
(2 rows)
Note that postgres_fdw already recreates a cursor when doing a rescan
with parameter changes, so we already have this issue. IMO I think we
should avoid writing a query like this.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-15 20:01 Tom Lane <tgl@sss.pgh.pa.us>
parent: Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 2 replies; 12+ messages in thread
From: Tom Lane @ 2024-07-15 20:01 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> On Mon, Jul 15, 2024 at 10:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Uh ... does this ensure that the data hasn't changed?
> postgres_fdw opens a remote transaction using REPEATABLE READ or
> SERIALIZABLE, so the recreated cursor uses the same snapshot except
> changes made by the remote transaction.
Ah, right.
> This causes eg, a join-UPDATE query where multiple rows join to the
> same foreign target row to repeatedly update the target row, as shown
> below, which would never happen if rewinding the cursor.
> ...
> Note that postgres_fdw already recreates a cursor when doing a rescan
> with parameter changes, so we already have this issue. IMO I think we
> should avoid writing a query like this.
Hmm. In principle, since postgres_fdw controls all the SQL sent to
the remote side, we could avoid building problematic queries. But
I'm not sure how to make that work in practice, or how we'd avoid
somebody carelessly breaking it in future. It seems like the
property you propose requiring is a second-order effect that would
be hard to ensure.
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-16 10:29 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 12+ messages in thread
From: Etsuro Fujita @ 2024-07-16 10:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
On Tue, Jul 16, 2024 at 5:01 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> > This causes eg, a join-UPDATE query where multiple rows join to the
> > same foreign target row to repeatedly update the target row, as shown
> > below, which would never happen if rewinding the cursor.
> > ...
> > Note that postgres_fdw already recreates a cursor when doing a rescan
> > with parameter changes, so we already have this issue. IMO I think we
> > should avoid writing a query like this.
>
> Hmm. In principle, since postgres_fdw controls all the SQL sent to
> the remote side, we could avoid building problematic queries. But
> I'm not sure how to make that work in practice, or how we'd avoid
> somebody carelessly breaking it in future. It seems like the
> property you propose requiring is a second-order effect that would
> be hard to ensure.
Agreed. To be honest I am not sure if we can fix this issue, but if
so, I think that that would be going to require invasive changes to
the core and probably would not be back-patchable, so I will leave
this for future work.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)
@ 2024-07-19 04:45 Etsuro Fujita <etsuro.fujita@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 12+ messages in thread
From: Etsuro Fujita @ 2024-07-19 04:45 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: eric.cyr@gmail.com; pgsql-bugs@lists.postgresql.org
On Tue, Jul 16, 2024 at 5:01 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> > On Mon, Jul 15, 2024 at 10:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Uh ... does this ensure that the data hasn't changed?
>
> > postgres_fdw opens a remote transaction using REPEATABLE READ or
> > SERIALIZABLE, so the recreated cursor uses the same snapshot except
> > changes made by the remote transaction.
>
> Ah, right.
Pushed and back-patched to v15 after adding a comment for the test
case. Thanks for reviewing!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2024-07-19 04:45 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-06 18:42 BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX) PG Bug reporting form <noreply@postgresql.org>
2023-04-07 09:16 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2023-04-29 11:36 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2023-04-29 11:42 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2023-04-29 22:50 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-07-05 12:49 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2024-07-15 10:51 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2024-07-15 13:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-07-15 19:45 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2024-07-15 20:01 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-07-16 10:29 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2024-07-19 04:45 ` Etsuro Fujita <etsuro.fujita@gmail.com>
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