fdw_private and (List*) handling in FDW API

Hi,

I've been exploring the new FDW API in the past few days, and I'm slightly
confused by the fdw_private fields. A few comments:

1) Generally all the API functions pass data using fields in the nodes
(e.g. GetForeignRelSize uses baserel->fdw_private etc.), but
PlanForeignModify simply returns the data, and BeginForeignModify accepts
that as a regular parameter. Is there any particular reason not to adapt
the same approach in all cases, i.e. either return the private data in all
cases (and pass as parameters), or passing them inside node/plan/...?

2) Is there any particular reason why PlanForeignModify/BeginForeignModify
require the fdw_private to be a List*, and not a generic pointer? I mean,
RelOptInfo declares fdw_private as a (void*) but the other structures
(e.g. ForeignScan) switches to (List*) for some reason. But all the
optimizer does with this data is this in createplan.c

fdw_private_list = NIL;
i = 0;
foreach(lc, resultRelations)
{
...
fdw_private = fdwroutine->PlanForeignModify(root, node, rti, i);
fdw_private_list = lappend(fdw_private_list, fdw_private);
i++;
}

node->fdwPrivLists = fdw_private_list;
return node;

If I read that correctly, it just accumulates all the lists into a single
list (and then unpacks that into individual lists in nodeModifyTable.c).
What is the reason for using (List*) here? I'd rather use a structure
here, not generic lists, YMMV. Or is there something I missed (e.g. future
plans)?

regards
Tomas

"Tomas Vondra" <tv(at)fuzzy(dot)cz> writes:
> 2) Is there any particular reason why PlanForeignModify/BeginForeignModify
> require the fdw_private to be a List*, and not a generic pointer?

That data has to be copiable by copyObject(), which a generic void* is
not. We could perhaps have made it Node* instead, but that would only
work conveniently if there were infrastructure for plugins to create new
first-class Node types; which there isn't. A List is often the easiest
way to transport a few random values from plan time to execution time,
so it seemed best to declare fdw_private that way.

regards, tom lane

On 18 Říjen 2013, 17:52, Tom Lane wrote:
> "Tomas Vondra" <tv(at)fuzzy(dot)cz> writes:
>> 2) Is there any particular reason why
>> PlanForeignModify/BeginForeignModify
>> require the fdw_private to be a List*, and not a generic pointer?
>
> That data has to be copiable by copyObject(), which a generic void* is
> not. We could perhaps have made it Node* instead, but that would only
> work conveniently if there were infrastructure for plugins to create new
> first-class Node types; which there isn't. A List is often the easiest
> way to transport a few random values from plan time to execution time,
> so it seemed best to declare fdw_private that way.

Oh, I see. Thanks for explanation.

Tomas