Re: review: FDW API

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Postgres - Hackers <pgsql-hackers(at)postgresql(dot)org>, Shigeru HANADA <hanada(at)metrosystems(dot)co(dot)jp>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jan Urbański <wulczer(at)wulczer(dot)org>
Subject: Re: review: FDW API
Date: 2011-02-18 20:30:08
Message-ID: 4D5ED6D0.1080906@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 18.02.2011 22:16, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-02-18 20:37:34 Re: WIP - Add ability to constrain backend temporary file space
Previous Message Dimitri Fontaine 2011-02-18 20:23:10 Re: contrib loose ends: 9.0 to 9.1 incompatibilities