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

On Thu, Nov 26, 2015 at 5:27 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> I'm now implementing. The above design perfectly works on ForeignScan.
> On the other hands, I'd like to have deeper consideration for CustomScan.
>
> My recent patch adds LibraryName and SymbolName on CustomScanMethods
> to lookup the method table even if library is not loaded yet.
> However, this ExtensibleNodeMethods relies custom scan provider shall
> be loaded, by parallel infrastructure, prior to the deserialization.
> It means extension has a chance to register itself as well.
>
> My idea is, redefine CustomScanMethod as follows:
>
> typedef struct ExtensibleNodeMethods
> {
> const char *extnodename;
> Size node_size;
> Node *(*nodeCopy)(const Node *from);
> bool (*nodeEqual)(const Node *a, const Node *b);
> void (*nodeOut)(struct StringInfoData *str, const Node *node);
> void (*nodeRead)(Node *node);
> } ExtensibleNodeMethods;
>
> typedef struct CustomScanMethods
> {
> union {
> const char *CustomName;
> ExtensibleNodeMethods xnode;
> };
> /* Create execution state (CustomScanState) from a CustomScan plan node */
> Node *(*CreateCustomScanState) (struct CustomScan *cscan);
> } CustomScanMethods;
>
> It allows to pull CustomScanMethods using GetExtensibleNodeMethods
> by CustomName; that is alias of extnodename in ExtensibleNodeMethods.
> Thus, we don't need to care about LibraryName and SymbolName.
>
> This kind of trick is not needed for ForeignScan because all the pointer
> stuff are reproducible by catalog, however, method table of CustomScan
> is not.
>
> How about your opinion?

Anonymous unions are not C89, so this is a no-go.

I think we should just drop CustomName and replace it with
ExtensibleNodeMethods. That will break things for third-party code
that is using the existing CustomScan stuff, but there's a good chance
that nobody other than you has written any such code. And even if
someone has, updating it for this change will not be very difficult.

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