Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)

On Sun, Feb 7, 2016 at 7:28 PM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> The new callbacks of T_ExtensibleNode will replace the necessity to
> form and deform process of private values, like as:
> https://github.com/pg-strom/devel/blob/master/src/gpujoin.c#L114

Yeah.

> It transforms a bunch of internal data of CustomScan (similar to the
> extended fields in T_ExtensibleNode) to/from the node functions
> understandable forms for copy, input and output support.
> I think it implies you proposition is workable.
>
> I'd like to follow this proposition basically.
> On the other hands, I have two points I want to pay attention.
>
> 1. At this moment, it is allowed to define a larger structure that
> embeds CustomPath and CustomScanState by extension. How do we treat
> this coding manner in this case? Especially, CustomScanState has no
> private pointer dereference because it assumes an extension define
> a larger structure. Of course, we don't need to care about node
> operations on Path and PlanState nodes, but right now.

I see no real advantage in letting a CustomPath be larger. If
custom_private can include extension-defined node types, that seems
good enough. On the other hand, if CustomScanState can be larger,
that seems fine. We don't really need any special support for that,
do we?

> 2. I intended to replace LibraryName and SymbolName fields from the
> CustomScanMethods structure by integration of extensible node type.
> We had to give a pair of these identifiers because custom scan provider
> has no registration points at this moment. A little concern is extension
> has to assume a particular filename of itself.
> But, probably, it shall be a separated discussion. My preference is
> preliminary registration of custom scan provider by its name, as well
> as extensible node.

Seems like we could just leave the CustomScan stuff alone and worry
about this as a separate facility.

> Towards the last question; whether *_private shall be void * or List *,
> I want to keep fdw_private and custom_private as List * pointer, because
> a new node type definition is a bit overdone job if this FDW or CSP will
> take only a few private fields with primitive data types.
> It is a preferable features when extension defines ten or more private
> fields.

Well, I suggested Node *, not void *. A Node can be a List, but not
every Node is a List.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company