Thread: Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)

> On Wed, Nov 11, 2015 at 3:29 PM, Andres Freund <andres@anarazel.de> wrote:
> > On 2015-11-11 14:59:33 -0500, Robert Haas wrote:
> >> I don't see this as being a particularly good idea.  The same issue
> >> exists for FDWs, and we're just living with it in that case.
> >
> > It's absolutely horrible there. I don't see why that's a justification
> > for much.  To deal with the lack of extensible copy/out/readfuncs I've
> > just had to copy the entirety of readfuncs.c into an extension. Or you
> > build replacements for those (as e.g. postgres_fdw essentially has
> > done).
> >
> >> If we do want to improve it, I'm not sure this is the way to go,
> >> either.  I think there could be other designs where we focus on making
> >> the serialization and deserialization options better, rather than
> >> letting people tack stuff onto the struct.
> >
> > Just better serialization doesn't actually help all that much. Being
> > able to conveniently access data directly, i.e. as fields in a struct,
> > makes code rather more readable. And in many cases more
> > efficient. Having to serialize internal datastructures unconditionally,
> > just so copyfuncs.c works if actually used, makes for a fair amount of
> > inefficiency (forced deserialization, even when not copying) and uglier
> > code.
> 
> Fair enough, but I'm still not very interested in having something for
> CustomScan only.
>
I agree with we have no reason why only custom-scan is allowed to have
serialize/deserialize capability. I can implement an equivalent stuff
for foreign-scan also, and it is helpful for extension authors, especially,
who tries to implement remote join feature because FDW driver has to
keep larger number of private fields than scan.

Also, what is the reason why we allow extensions to define a larger
structure which contains CustomPath or CustomScanState? It seems to
me that these types are not (fully) supported by the current copyfuncs,
outfuncs and readfuncs, aren't it?
(Although outfuncs.c supports path-nodes, thus CustomPathMethods has
TextOut callback but no copy/read handler at this moment.)

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

On Wed, Nov 11, 2015 at 11:13 PM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> I agree with we have no reason why only custom-scan is allowed to have
> serialize/deserialize capability. I can implement an equivalent stuff
> for foreign-scan also, and it is helpful for extension authors, especially,
> who tries to implement remote join feature because FDW driver has to
> keep larger number of private fields than scan.
>
> Also, what is the reason why we allow extensions to define a larger
> structure which contains CustomPath or CustomScanState? It seems to
> me that these types are not (fully) supported by the current copyfuncs,
> outfuncs and readfuncs, aren't it?
> (Although outfuncs.c supports path-nodes, thus CustomPathMethods has
> TextOut callback but no copy/read handler at this moment.)

I feel like we're sort of plunging blindly down a path of trying to
make certain parts of the system extensible and pluggable without
really having a clear vision of where we want to end up.  Somehow I
feel like we ought to start by thinking about a framework for *node*
extensibility; then, within that, we can talk about what that means
for particular types of nodes.

For example, suppose do something like this:

typedef struct
{  NodeTag tag;  char *extnodename;
} ExtensibleNode;

typedef stuct
{   char *extnodename;   char *library_name;   char *function_name;   Size node_size;   ExtensibleNode
*(*nodeCopy)(ExtensibleNode*);   bool (*nodeEqual)(ExtensibleNode *, ExtensibleNode *);   void (*nodeOut)(StringInfo
str,ExtensibleNode *);   ExtensibleNode (*nodeRead)(void);
 
} ExtensibleNodeMethods;

extern void RegisterExtensibleNodeMethods(ExtensibleNodeMethods *);
extern ExtensibleNodeMethods *GetExtensibleNodeMethods(char *extnodename);

This provides a generic infrastructure so that we don't have to keep
inventing the same stuff over and over, rather than coming up with one
way to do it for ForeignScans and another way for CustomScan and
another way for CustomScanState.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company