Re: 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>