On 2015/03/12 13:35, Tom Lane wrote:
> I don't like the execMain.c changes much at all. They look somewhat
> like they're intended to allow foreign tables to adopt a different
> locking strategy,
I didn't intend such a thing. My intention is, foreign tables have
markType = ROW_MARK_COPY as ever, but I might not have correctly
understood what you pointed out. Could you elaborate on that?
> The changes are not all consistent
> either, eg this:
>
> ! if (erm->relation &&
> ! erm->relation->rd_rel->relkind != RELKIND_FOREIGN_TABLE)
>
> is not necessary if this Assert is accurate:
>
> ! if (erm->relation)
> ! {
> ! Assert(erm->relation->rd_rel->relkind ==
> RELKIND_FOREIGN_TABLE);
I modified InitPlan so that relations get opened for foreign tables
as well as local tables. So, I think the change would be necessary.
> I don't see the need for the change in nodeForeignscan.c. If the FDW has
> returned a physical tuple, it can fix that for itself, while if it has
> returned a virtual tuple, the ctid will be garbage in any case.
If you leave nodeForeignscan.c unchanged, file_fdw would cause the
problem that I pointed out upthread. Here is an example.
[Create a test environment]
postgres=# create foreign table foo (a int) server file_server options
(filename '/home/pgsql/foo.csv', format 'csv');
CREATE FOREIGN TABLE
postgres=# select tableoid, ctid, * from foo;
tableoid | ctid | a
----------+----------------+---
16459 | (4294967295,0) | 1
(1 row)
postgres=# create table tab (a int, b int);
CREATE TABLE
postgres=# insert into tab values (1, 1);
INSERT 0 1
[Run concurrent transactions]
In terminal1:
postgres=# begin;
BEGIN
postgres=# update tab set b = b * 2;
UPDATE 1
In terminal2:
postgres=# select foo.tableoid, foo.ctid, foo.* from foo, tab where
foo.a = tab.a for update;
In terminal1:
postgres=# commit;
COMMIT
In terminal2:(When committing the UPDATE query in terminal1, psql in
terminal2 will display the result for the SELECT-FOR-UPDATE query as
shown below.)
tableoid | ctid | a
----------+-------+---
16459 | (0,0) | 1
(1 row)
Note the value of the ctid has changed!
Rather than changing nodeForeignscan.c, it might be better to change
heap_form_tuple to set the t_ctid of a formed tuple to be invalid.
Thanks for the review!
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
