Re: review: FDW API

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>

From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Postgres - Hackers <pgsql-hackers@postgresql.org>, Shigeru HANADA <hanada@metrosystems.co.jp>, Robert Haas <robertmhaas@gmail.com>, Jan Urbański <wulczer@wulczer.org>
Date: 2011-02-18T20:30:08Z
Lists: pgsql-hackers
On 18.02.2011 22:16, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> Another version, rebased against master branch and with a bunch of small
>> cosmetic fixes.
>
>> I guess this is as good as this is going to get for 9.1.
>
> This is *badly* in need of another cleanup pass; it's full of typos,
> contradictory comments, #ifdef NOT_USED stuff, etc etc.  And the
> documentation is really inadequate.  If you're out of energy to go
> over it, I guess I should step up.

If you have the energy, by all means, thanks.

> Question after first look: what is the motivation for passing
> estate->es_param_list_info to BeginScan?  AFAICS, even if there is a
> reason for that function to need that, it isn't receiving any info that
> would be sufficient to let it know what's in there.

The idea is that when the query is planned, the FDW can choose to push 
down a qual that contains a parameter marker, like "WHERE remotecol = 
$1". At execution time, it needs the value of the parameter to send it 
to the remote server. The PostgreSQL FDW does that, although I didn't 
test it so it might well be broken.

>  What seems more
> likely to be useful is to pass in the EState pointer, as for example
> being able to look at es_query_cxt seems like a good idea.

By "look at", you mean allocate stuff in it?

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com