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.)

> 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.

regards, tom lane