From: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
---|---|
To: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: EvalPlanQual behaves oddly for FDW queries involving system columns |
Date: | 2015-03-12 03:51:50 |
Message-ID: | CAFjFpRfKOk7TkB40rJxmbznMnJLwjVDDV5qVyDpcDr2ArKBYwg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Mar 11, 2015 at 5:10 PM, Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
wrote:
> 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!
>
>
I will leave this issue for the committer to judge. Changed the status to
"ready for committer".
> Apart from this, I do not have any comments here.
>>
>
> Thanks again.
>
> Best regards,
> Etsuro Fujita
>
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2015-03-12 04:35:07 | Re: EvalPlanQual behaves oddly for FDW queries involving system columns |
Previous Message | Andreas Karlsson | 2015-03-12 02:50:41 | Re: patch : Allow toast tables to be moved to a different tablespace |