Re: [HACKERS] Odd system-column handling in postgres_fdw join pushdown patch

[Etsuro Fujita]

On 2016/04/14 4:57, Robert Haas wrote:

1. For a regular FDW scan, zero the xmin, xmax, and cid of the tuple
before returning it from postgres_fdw, so that we don't expose the
datum-tuple fields.   I can't see any reason this isn't safe, but I
might be missing something.

I noticed odd behavior of this in EvalPlanQual.  Consider:

-- session 1
postgres=# create extension postgres_fdw;
CREATE EXTENSION

CREATE SERVER
postgres=# create user mapping for public server fs;
CREATE USER MAPPING
postgres=# create table t1 (a int, b int);
CREATE TABLE
postgres=# create table t2 (a int, b int);
CREATE TABLE
postgres=# insert into t1 values (1, 1);
INSERT 0 1
postgres=# insert into t2 values (1, 1);
INSERT 0 1

CREATE FOREIGN TABLE
postgres=# select xmin, xmax, cmin, * from ft1;
  xmin | xmax | cmin | a | b
------+------+------+---+---
     0 |    0 |    0 | 1 | 1
(1 row)

-- session 2
postgres=# begin;
BEGIN
postgres=# update t2 set a = a;
UPDATE 1

-- session 1


-- session 2
postgres=# commit;
COMMIT

-- session 1 (will show the following)
  xmin |    xmax    | cmin  | a | b
------+------------+-------+---+---
   128 | 4294967295 | 16398 | 1 | 1
(1 row)





Best regards,
Etsuro Fujita

postgres=# create server fs foreign data wrapper postgres_fdw options (dbname 'postgres'); postgres=# create foreign table ft1 (a int, b int) server fs options (table_name 't1'); postgres=# select ft1.xmin, ft1.xmax, ft1.cmin, ft1.* from ft1, t2 for update; The values of xmin, xmax, and cmin are not 0! The reason for that is that we don't zero these values in a test tuple stored by EvalPlanQualFetchRowMarks for EvalPlanQual re-evaluations. That cleanup applies to the file_fdw foreign table case as well, so I think xmin, xmax, and cid in tuples from such tables should be set to 0, too. And ISTM that it's better that the core (ie, ForeignNext) supports doing that, rather than each FDW does that work. That would also minimize the overhead because ForeignNext does that if needed. Please find attached a patch.

postgres-fdw-syscol-cleanup.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers