Re: EvalPlanQual behaves oddly for FDW queries involving system columns

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, ashutosh(dot)bapat(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: EvalPlanQual behaves oddly for FDW queries involving system columns
Date: 2015-05-13 08:25:16
Message-ID: 55530A6C.1010105@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2015/05/13 6:22, Tom Lane wrote:
> I wrote:
>> I did a very basic update of your postgres_fdw patch to test this with,
>> and attach that so that you don't have to repeat the effort. I'm not sure
>> whether we want to try to convert that into something committable. I'm
>> afraid that the extra round trips involved in doing row locking this way
>> will be so expensive that no one really wants it for postgres_fdw. It's
>> more credible that FDWs operating against local storage would have use
>> for it.
>
> Of course, if we don't do that then we still have your original gripe
> about ctid not being correct in EvalPlanQual results. I poked at this
> a bit and it seems like we could arrange to pass ctid through even in
> the ROW_MARK_COPY case, if we define the t_ctid field of a composite
> Datum as being the thing to use. The attached WIP, mostly-comment-free
> patch seems to fix the original test case. It would likely result in
> ctid coming out as (0,0) not (4294967295,0) for FDWs that don't take
> any special steps to return a valid ctid, as a result of the fact that
> heap_form_tuple just leaves that field zeroed.

+1

I did the same thing when creating the first version of the patch [1].
In addition, I made another change to ForeignNext so that the FDWs to
get ctid coming out as the same value (0, 0) instead of (4294967295,0)
before and after an EvalPlanQual recheck. But IIRC, I think both of
them were rejected by you ...

> We could fix that by
> adding an ItemPointerSetInvalid(&(td->t_ctid)) call to heap_form_tuple,
> but I dunno if we want to add even a small number of cycles for this
> purpose to such a core function.

I thought so too when creating the first version.

Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/54B7691B.5080702@lab.ntt.co.jp

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2015-05-13 08:31:59 Re: multivariate statistics / patch v6
Previous Message Etsuro Fujita 2015-05-13 07:25:52 Re: EvalPlanQual behaves oddly for FDW queries involving system columns