Horton

Thread

Commits

  1. postgres_fdw: Avoid "cursor can only scan forward" error.

  2. Fix some anomalies with NO SCROLL cursors.

  1. BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    The Post Office <noreply@postgresql.org> — 2023-04-06T18:42:38Z

    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'
;

  2. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2023-04-07T09:16:03Z

    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

  3. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2023-04-29T11:36:55Z

    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

  4. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2023-04-29T11:42:02Z

    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

  5. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-04-29T22:50:24Z

    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

  6. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2024-07-05T12:49:27Z

    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

  7. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2024-07-15T10:51:03Z

    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

  8. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-07-15T13:55:28Z

    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

  9. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2024-07-15T19:45:48Z

    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

  10. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-07-15T20:01:02Z

    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

  11. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2024-07-16T10:29:16Z

    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

  12. Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2024-07-19T04:45:27Z

    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