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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Odd system-column handling in postgres_fdw join pushdown patch
Date: 2016-07-01 07:10:38
Message-ID: 84d281d5-53c2-292e-edec-1ced89bd9270@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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
postgres=# create server fs foreign data wrapper postgres_fdw options
(dbname 'postgres');
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
postgres=# create foreign table ft1 (a int, b int) server fs options
(table_name 't1');
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
postgres=# select ft1.xmin, ft1.xmax, ft1.cmin, ft1.* from ft1, t2 for
update;

-- session 2
postgres=# commit;
COMMIT

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

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.

Best regards,
Etsuro Fujita

Attachment Content-Type Size
postgres-fdw-syscol-cleanup.patch binary/octet-stream 3.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2016-07-01 07:24:20 Re: Is a UDF binary portable across different minor releases and PostgreSQL distributions?
Previous Message Andres Freund 2016-07-01 06:48:26 Re: pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <