On 2015/03/11 17:37, Ashutosh Bapat wrote:
Now I can reproduce the problem.
Sanity
--------
Patch compiles cleanly and make check passes. The tests in file_fdw and
postgres_fdw contrib modules pass.
The patch works as expected in the test case reported.
Thanks for the testing!
I have only one doubt.
In EvalPlanQualFetchRowMarks(). tuple->t_self is assigned from
td->t_ctid. CTID or even t_self may be valid for foreign tables based on
postgres_fdw but may not be valid for other FDWs. So, this assignment
might put some garbage in t_self, rather we should set it to invalid as
done prior to the patch. I might have missed some previous thread, we
decided to go this route, so ignore the comment, in that case.
Good point. As the following code and comment I added to ForeignNext, I
think that FDW authors should initialize the tup->t_data->t_ctid of each
scan tuple with its own TID. If the authors do that, the t_self is
guaranteed to be assigned the right TID from the whole-row Var (ie,
td->t_ctid) in EvalPlanQualFetchRowMarks.
/* We assume that t_ctid is initialized with its own TID */
tup->t_self = tup->t_data->t_ctid;
IMHO, I'm not sure it's worth complicating the code as you mentioned.
(I don't know whether there are any discussions about this before.)
Note that file_fdw needs no treatment. In that case, in ForeignNext,
the tup->t_data->t_ctid of each scan tuple is initialized with (0,0) (if
necessary), and then the t_self will be correctly assigned (0,0) throguh
the whole-row Var in EvalPlanQualFetchRowMarks. So, no inconsistency!
Apart from this, I do not have any comments here.
Thanks again.
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
