Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c) - Mailing list pgsql-hackers

From Robert Haas
Subject Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)
Date
Msg-id CA+TgmoZx4rYgkp62FseY7y-bx+BeBK1BUxSn=VXxJj8CGTyF9g@mail.gmail.com
Whole thread Raw
In response to CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)  (Kouhei Kaigai <kaigai@ak.jp.nec.com>)
Responses Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
On Mon, Nov 9, 2015 at 10:26 PM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> It is a relevant topic of readfuncs support for custom-scan.
>
> Unlike CustomPath and CustomScanState, we don't allow custom-scan
> provider to define own and larger structure that embeds CustomScan
> at head of the structure, to support copyObject().
> Thus, custom-scan provider needs to have own logic to pack and
> unpack private fields, like:
>   https://github.com/pg-strom/devel/blob/master/src/gpujoin.c#L112
>
> At present, only create_customscan_plan() and _copyCustomScan()
> can produce CustomScan node.
> The create_customscan_plan() invokes PlanCustomPath callback,
> thus, CSP can return a pointer of CustomScan field within
> a larger structure. So, we don't adjust this interface.
> On the other hands, _copyCustomScan() allocates a new node using
> makeNode(CustomScan), thus, here is no space for the larger
> structure if CSP wants to use to keep private values without
> packing and unpacking.
> Only CSP knows exact size of the larger structure, so all we can
> do is ask CSP to allocate a new node and copy its private fields.
> This patch newly adds NodeCopyCustomScan callback to support
> copyObject.
>
> Also, v9.6 will support nodeToString/stringToNode on plan nodes.
> So, we need to re-define the role of TextOutCustomScan callback
> that is also defined at v9.5.
> If CSP extends the CustomScan to save private values on the extra
> area, only CSP knows what values and which data types are stored,
> thus, all we can do is ask CSP to serialize and deserialize its
> private fields without inconsistency.
> Even though TextOutCustomScan callback shall be once ripped out
> to support readfuncs.c, a pair of TextOut and TextRead callback
> will re-define its responsibility again; when CSP defines
> a larger structure that embeds CustomScan, a pair of these
> callbacks are used to serialize/deserialize the entire custom
> defined objects.

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.  There
was talk previously of improving it, and maybe some day somebody will,
but I can't get excited about it.  Writing serialization and
deserialization code is annoying but completely insufficient, in my
mind, to justify the hassle of creating a system for this that will be
used by very, very little code.

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.

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



pgsql-hackers by date:

Previous
From: Thom Brown
Date:
Subject: Re: Parallel Seq Scan
Next
From: Andres Freund
Date:
Subject: Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)