Re: Custom Scans and private data

From: Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Custom Scans and private data
Date: 2015-08-26 00:55:48
Message-ID: 9A28C8860F777E439AA12E8AEA7694F80113805D@BPXM15GP.gisp.nec.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 2015-08-25 14:42:32 -0400, Tom Lane wrote:
> > Andres Freund <andres(at)anarazel(dot)de> writes:
> > > Since we already have CustomScan->methods, it seems to be rather
> > > reasonable to have a CopyCustomScan callback and let it do the copying
> > > of the private data if present? Or possibly of the whole node, which'd
> > > allow to embed it into a bigger node?
> >
> > Weren't there rumblings of requiring plan trees to be deserializable/
> > reserializable, not just copiable, so that they could be passed to
> > background workers?
>
> Yes, I do recall that as well. that's going to require a good bit of
> independent stuff - currently there's callbacks as pointers in nodes;
> that's obviously not going to fly well across processes. Additionally
> custom scan already has a TextOutCustomScan callback, even if it's
> currently probably intended for debugging.
>
> I rather doubt that adding out/readfuncs without the ability to do
> something about what exactly is read in is going to work well. But I
> admit I'm not too sure about it.
>
> > In any case, since this convention already exists for FDWs I'm not
> > sure why we should make it different for CustomScan.
>
> I think it was a noticeable mistake in the fdw case, but we already
> released with that. We shouldn't make the same mistake twice. Looking at
> postgres_fdw and the pg-strom example linked upthread imo pretty clearly
> shows how ugly this is. There's also the rather noticeable difference
> that we already have callbacks in the node for custom scans (used by
> outfuncs), making this rather trivial to add.
>
As Tom pointed out, the primary reason why CustomScan required provider
to save its private data on custom_exprs/custom_private were awareness
of copyObject().
In addition, custom_exprs are expected to link expression node to be
processed in setrefs.c and subselect.c because core implementation
cannot know which type of data is managed in private.

Do you concern about custom_private only? Even if we have extra
callbacks like CopyObjectCustomScan() and TextReadCustomScan(),
how do we care about the situation when core implementation needs to
know the location of expression nodes? Is custom_exprs retained as is?

In the earlier version of CustomScan interface had individual
callbacks on setrefs.c and subselect.c, however, its structure
depended on the current implementation too much, then we moved
to the implementation to have two individual private fields
rather than callbacks on outfuncs.c.

On the other hands, I'm inclined to think TextOutCustomScan() might
be a misdesign to support serialize/deserialize via readfuncs.c.
http://www.postgresql.org/message-id/9A28C8860F777E439AA12E8AEA7694F80111D04F@BPXM15GP.gisp.nec.co.jp
I think it shall be deprecated rather then enhancement.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joe Conway 2015-08-26 01:03:52 Re: One question about security label command
Previous Message Tom Lane 2015-08-26 00:36:52 Re: Idea: closing the loop for "pg_ctl reload"