On 2016/09/21 0:40, Robert Haas wrote:
> On Fri, Jul 1, 2016 at 3:10 AM, Etsuro Fujita
> <fujita.etsuro@lab.ntt.co.jp> wrote:
>> 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.
> If you want this to be considered, you'll need to rebase it and submit
> it to the next CommitFest.
Will do.
Best regards,
Etsuro Fujita