Re: CustomScan and readfuncs.c

> 2. Reproduce method table on background worker
> ----------------------------------------------
> The method field of CustomPath/Scan/ScanState is expected to be
> a reference to a static structure. Thus, copyObject() does not
> copy the entire table, but only pointers.
> However, we have no way to guarantee the callback functions have
> same entrypoint addressed on background workers. So, we may need
> an infrastructure to reproduce same CustomScan node with same
> callback function tables, probably, identified by name.
> We may have a few ways to solve the problem.
>
> * Add system catalog, function returns pointer
> The simplest way, like FDW. System catalog has a name and function
> to return callback pointers. It also needs SQL statement support,
> even a little down side.
>
I tried to design a DDL statement and relevant system catalog as follows.

#define CustomPlanRelationId 3999

CATALOG(pg_custom_plan,3999)
{
NameData custom_name;
regproc custom_handler;
} FormData_pg_custom_plan;

This simple catalog saves a pair of name and handler function of custom
plan provider. Like FDW, this handler function returns pointers to the
entrypoint to be called by set_(rel|join)_pathlist_hook and relevant
CustomXXXMethods table.

User can register a custom plan provider using the following statement:
CREATE CUSTOM PLAN <name> HANDLER <function_name>;

And unregister:
DROP CUSTOM PLAN <name>;

This enhancement allows background workers to reproduce CustomScan node
that was serialized by nodeToString(), as long as provider is specified
by the name.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

> -----Original Message-----
> From: Kaigai Kouhei(海外 浩平)
> Sent: Monday, July 27, 2015 8:42 AM
> To: 'Tom Lane'
> Cc: pgsql-hackers(at)postgresql(dot)org
> Subject: RE: [HACKERS] CustomScan and readfuncs.c
>
> > Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> writes:
> > > Under the investigation of ParallelAppend, I noticed here is a few
> > > problems in CustomScan, that prevents to reproduce an equivalent
> > > plan node on the background worker from serialized string.
> >
> > > 1. CustomScanMethods->TextOutCustomScan callback
> > > ------------------------------------------------
> > > This callback allows to output custom information on nodeToString.
> > > Originally, we intend to use this callback for debug only, because
> > > CustomScan must be copyObject() safe, thus, all the private data
> > > also must be stored in custom_exprs or custom_private.
> > > However, it will lead another problem when we try to reproduce
> > > CustomScan node from the string form generated by outfuncs.c.
> > > If TextOutCustomScan prints something, upcoming _readCustomScan
> > > has to deal with unexpected number of tokens in unexpected format.
> >
> > Um ... wait a second. There is no support in readfuncs for any
> > plan node type, and never has been, and I seriously doubt that there
> > ever should be. I do not think it makes sense to ship plans around
> > in the way you seem to have in mind. (Also, I don't think the
> > problems you mention are exactly unique to CustomScan. There's no
> > reason to assume that FDW plans could survive this treatment either,
> > since we do not know what's in the fdw_private stuff; certainly no
> > one has ever suggested that it should not contain pointers to static
> > data.)
> >
> Yep, no Plan node types are supported at this moment, however, will
> appear soon by the Funnel + PartialSeqScan nodes.
> It serializes a partial plan subtree using nodeToString() then gives
> the flatten PlannedStmt to background workers.
> I'm now investigating to apply same structure to Append not to kick
> child nodes in parallel.
> Once various plan node types appear in readfuncs.c, we have to care
> about this problem, don't it? I'm working for the patch submission
> of ParallelAppend on the next commit-fest, so like to make a consensus
> how to treat this matter.
>
> > > I'd like to propose to omit this callback prior to v9.5 release,
> > > for least compatibility issues.
> >
> > I regard our commitment to cross-version compatibility for the
> > custom scan APIs as being essentially zero, for reasons previously
> > discussed. So if this goes away in 9.6 it will not matter, but we
> > might as well leave it in for now for debug support.
> >
> I don't argue this point strongly. If TextOutCustomScan shall be
> obsoleted on v9.6, it is just kindness for developers not to use
> this callback.
>
> Thanks,
> --
> NEC Business Creation Division / PG-Strom Project
> KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>