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

From Kouhei Kaigai
Subject Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)
Date
Msg-id 9A28C8860F777E439AA12E8AEA7694F80116EE94@BPXM15GP.gisp.nec.co.jp
Whole thread Raw
In response to Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Wed, Nov 11, 2015 at 3:29 PM, Andres Freund <andres@anarazel.de> wrote:
> >> 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'm with Robert here.  The fact is that postgres_fdw and other FDWs are
> living just fine with the restrictions we put in to allow copyfuncs.c to
> copy ForeignScan, and there's been no sufficient justification offered
> as to why CustomScan can't play by those rules.
>
So, I'd like to propose copy/out/read callbacks for both of ForeignScan
and CustomScan.

> Yes, it would be nice to be able to use a more-tailored struct definition,
> but it's not *necessary*.  We should not contort the core system
> enormously in order to provide a bit more notational cleanliness to
> extensions.
>
> I'd be fine with fixing this if a nice solution were to present itself,
> but so far nothing's been offered that wasn't horrid.
>
> BTW, the "inefficiency" argument is junk, unless you provide some hard
> evidence of a case where it hurts a lot.  If we were forcing people to
> use serialized representations at execution time, it would be meaningful,
> but we're not; you can convert as needed at executor setup time.
>
Indeed, it is a "nice to have" feature. We can survive without luxury
goods but more expensive tax.

Once an extension tries to implement own join logic, it shall have
much larger number of private fields than usual scan logic.
Andres got a shock when he looked at GpuJoin code of PG-Strom before
because of its pack/unpack routine at: https://github.com/pg-strom/devel/blob/master/src/gpujoin.c#L112
The custom_private/fdw_private have to be safe to copyObject(), thus,
we have to pack/unpack private variables to/from a List structure.
When we design serialization/deserialization of Plan nodes, it means
ForeignScan and CustomScan has to pay double cost for this.

I never say it is a "necessary" feature, but "very nice to have".

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




pgsql-hackers by date:

Previous
From: Kyotaro HORIGUCHI
Date:
Subject: Re: Making tab-complete.c easier to maintain
Next
From: Kouhei Kaigai
Date:
Subject: Re: [DESIGN] ParallelAppend