Re: SQL/MED - core functionality

On 25.11.2010 16:16, Itagaki Takahiro wrote:
> On Thu, Nov 25, 2010 at 22:03, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> I propose the attached API instead. This has a clear separation between plan
>> and execution.
>
> The APIs seem to be cleaner. The previous ones might be too straight
> implementation of the SQL standard.
>
> But I have some questions about the new APIs:
> 1. Doesn't FdwPlan need to inherit Plan struct?
> 2. Doesn't FdwPlan need to support copyObject()?

No. You'll need a ForeignScan object in the planner that supports
copyObject(), just like in your patch. ForeignScan points to the
FdwPlan, but the FDW doesn't need to know anything about that stuff.

I left out some details on what exactly FdwPlan should contain and what
it's lifecycle should be. I'm thinking that it should be allocated in
the CurrentMemoryContext that's active when the FDW Plan routine is
called, which would be the same context where we store all the Plan
objects. It should not be modified after creation, so that it doesn't
need to be copied when the ForeignScan is copied with copyObject(). It
should not contain transient state information like connection objects,
or references to a remotely prepared cursor etc. It must be possible to
call BeginScan multiple times with the same FdwPlan object, so that it
can be stored in a prepared plan that is executed multiple times.

For a typical case like the PostgreSQL FDW, it would contain the foreign
server's OID, and the constructed SQL query that will be sent to the
remote server on execution. For the file FDW, it will probably contain
the filename, and the format options in some pre-parsed format.

> 3. If "Datum *values, bool *isnulls" is the better interface,
> why do we use TupleTableSlot?

I'm not wedded to that part, but in general, the less the FDW needs to
know about PostgreSQL internals the better. There's performance gain
from passing a TupleTableSlot to the FDW, but the ForeignScan node will
certainly store the datums/isnulls array to a TupleTableSlot to pass on
the tuple.

> We might have the similar issue
> in the index-only scan; it also handles virtual tuples.

Index-only scans are a very different story, that's going to be tightly
internal to the planner and executor, there's no externally-visible API
there.

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