On Wed, Nov 4, 2015 at 10:13 AM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> Sorry for my late.
>
> I confirmed that CustomScan support of readfuncs.c works fine using the
> attached patch for the PG-Strom tree. It worked as expected - duplication,
> serialization and deserialization by:
> plan_dup = stringToNode(nodeToString(copyObject(plan_orig)))
>
> So, the custom-scan-on-readfuncs.v1.path itself is good for me.
I don't think this is is what we want. In particular, I like this:
Plan trees must be able to be duplicated using <function>copyObject</>,
- so all the data stored within the <quote>custom</> fields must consist of
- nodes that that function can handle. Furthermore, custom scan providers
- cannot substitute a larger structure that embeds
- a <structname>CustomScan</> for the structure itself, as would be possible
- for a <structname>CustomPath</> or <structname>CustomScanState</>.
+ serialized using <function>nodeToString</> and deserialized using
+ <function>stringToNode</>, so so all the data stored within
+ the <quote>custom</> fields must consist of nodes that that function
+ can handle.
+ Furthermore, custom scan providers have to implement <quote>optional</>
+ callbacks if it defines substitute a larger structure that embeds
+ a <structname>CustomScan</> for the structure itself.
+ </para>
The purposes of this patch is supposedly to add readfuncs.c support to
custom scans - I agree with that goal. This documentation change is
talking about something else, namely using a CustomScan as the fist
field in a larger structure. I'm not sure I agree with that goal, and
in any case it seems like it ought to be a separate patch. This patch
would be a lot smaller if it weren't trying to make that change too.
I also don't think that this patch ought to introduce
get_current_library_filename(). There are other cases - e.g. for
launching background workers - where such a thing could be used, but
we haven't introduced it up until now. It isn't a requirement to get
readfuncs support working.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
