Re: CustomScan support on readfuncs.c - Mailing list pgsql-hackers

> > On Tue, Sep 29, 2015 at 6:19 PM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> > >> On Mon, Sep 28, 2015 at 8:31 PM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> > >> >> Instead of doing this:
> > >> >>
> > >> >> +    /* Dump library and symbol name instead of raw pointer */
> > >> >>      appendStringInfoString(str, " :methods ");
> > >> >> -    _outToken(str, node->methods->CustomName);
> > >> >> +    _outToken(str, node->methods->methods_library_name);
> > >> >> +    appendStringInfoChar(str, ' ');
> > >> >> +    _outToken(str, node->methods->methods_symbol_name);
> > >> >>
> > >> >> Suppose we just make library_name and symbol_name fields in the node
> > >> >> itself, that are dumped and loaded like any others.
> > >> >>
> > >> >> Would that be better?
> > >> >>
> > >> > I have no preference here.
> > >> >
> > >> > Even if we dump library_name/symbol_name as if field in CustomScan,
> > >> > not CustomScanMethods, in a similar way, we cannot use WRITE_STRING_FIELD
> > >> > here, because its 'fldname' assumes these members are direct field of
> > >> > CustomScan.
> > >> >
> > >> >   /* Write a character-string (possibly NULL) field */
> > >> >   #define WRITE_STRING_FIELD(fldname) \
> > >> >       (appendStringInfo(str, " :" CppAsString(fldname) " "), \
> > >> >        _outToken(str, node->fldname))
> > >>
> > >> Well that's exactly what I was suggesting: making them a direct field
> > >> of CustomScan.
> > >>
> > > Let me confirm. Are you suggesting to have library_name/symbol_name
> > > in CustomScan, not CustomScanMethods?
> > > I prefer these fields are in CustomScanMethods because we don't need
> > > to setup them again once PG_init set up these symbols. CustomScan has
> > > to be created every time when it is chosen by planner.
> >
> > True.  But that doesn't cost much.  I'm not sure it's better, so if
> > you don't like it, don't worry about it.
> >
> Yep, I like library_name and symbol_name are in CustomScanMethods
> because the table of callbacks and its identifiers are not separable
> 
> > >> > One other question I have. Do we have a portable way to lookup
> > >> > a pair of library and symbol by address?
> > >> > Glibc has dladdr() functions that returns these information,
> > >> > however, manpage warned it is not defined by POSIX.
> > >> > If we would be able to have any portable way, it may make the
> > >> > interface simpler.
> > >>
> > >> Yes: load_external_function.
> > >>
> > > It looks up an address by a pair of library and symbol name....
> > > What I'm looking for is a portable function that looks up a pair
> > > of library and symbol name by an address, like dladdr() of glibc.
> > > I don't know whether other *nix or windows have these infrastructure.
> >
> > No, that doesn't exist, and the chances of us trying add that
> > infrastructure for this feature are nil.
> >
> OK, probably, it is the only way to expect extension put correct
> values on the pair of library and symbol names.
> 
> I try to check whether the current patch workable with this direction
> using my extension. Please wait for a few days.
>
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.

In addition, I felt the following functions are useful for extensions.
 * _outToken()
 * _outBitmapset()
 * _readBitmapset()

Do we have none-static version of above functions?
If nothing, extension has to implement them by itself, more or less.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>