Re: EvalPlanQual behaves oddly for FDW queries involving system columns

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
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-11 15:34:19
Message-ID: 27322.1431358459@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> writes:
>> On 2015/05/11 8:50, Tom Lane wrote:
>>> I wonder if we should instead add a "ScanState*" field and expect the
>>> core code to set that up (ExecOpenScanRelation could do it with minor
>>> API changes...).

>> Sorry, I don't understand clearly what you mean, but that (the idea of
>> expecting the core to set it up) sounds inconsistent with your comment
>> on the earlier version of the API "BeginForeignFetch" [1].

> Well, the other way that it could work would be for the FDW's
> BeginForeignScan routine to search for a relevant ExecRowMark entry
> (which, per that previous discussion, it'd likely need to do anyway) and
> then plug a back-link to the ForeignScanState into the ExecRowMark.
> But I don't see any very good reason to make every FDW that's concerned
> with this do that, rather than doing it once in the core code. I'm also
> thinking that having a link to the source scan node there might be useful
> someday for regular tables as well as FDWs.

And on the third hand ... in view of the custom/foreign join pushdown
stuff that just went in, it would be a mistake to assume that there
necessarily is a distinct source scan node associated with each
ExecRowMark. The FDW can presumably find all the ExecRowMarks associated
with the rels that a join ForeignScan is scanning, but we can't assume
that ExecOpenScanRelation will be able to set up those links, and the FDW
might not want a simple link to the join scan node anyway. So it's
probably best to leave it as an unspecified void* instead of trying to
nail down the meaning more precisely.

I still don't much like calling it "fdw_state" though, because I don't
think it should be documented as only for the use of FDWs. (Custom scans
might have a use for a pointer field here too, for example.) Maybe just
call it "void *extra" and document it as being for the use of whatever
plan node is sourcing the relation's tuples.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2015-05-11 16:00:13 Re: Streaming replication and WAL archive interactions
Previous Message Tom Lane 2015-05-11 15:05:56 Re: EvalPlanQual behaves oddly for FDW queries involving system columns