Re: Reworks of CustomScan serialization/deserialization

From: Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Reworks of CustomScan serialization/deserialization
Date: 2016-03-29 01:36:23
Message-ID: 9A28C8860F777E439AA12E8AEA7694F8011D7866@BPXM15GP.gisp.nec.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> -----Original Message-----
> From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Robert Haas
> Sent: Friday, March 25, 2016 12:27 AM
> To: Petr Jelinek
> Cc: Kaigai Kouhei(海外 浩平); pgsql-hackers(at)postgresql(dot)org
> Subject: Re: [HACKERS] Reworks of CustomScan serialization/deserialization
>
> On Wed, Mar 23, 2016 at 1:36 PM, Petr Jelinek <petr(at)2ndquadrant(dot)com> wrote:
> > Ok, I am happy with it, marked it as ready for committer (it was marked as
> > committed although it wasn't committed).
>
> Thanks for fixing the status. I had forgotten about this thread.
>
> I can't really endorse the naming conventions here. I mean, we've got
> the main extensible nodes stuff in extensible.h, and then we've got
> this stuff in custom_node.h (BTW, there is a leftover reference to
> custom-node.h). There's no hint in the naming that this relates to
> scans, and why is it extensible in one place and custom in another?
>
> I'm not quite sure how to clean this up. At a minimum, I think we
> should standardize on "custom_scan.h" instead of "custom_node.h". I
> think that would be clearer. But I'm wondering if we should bite the
> bullet and rename everything from "custom" to "extensible" and declare
> it all in "extensible.h".
>
I don't have a strong reason to keep these stuff in separate files.
Both stuffs covers similar features and amount of code are enough small.
So, the attached v4 just merged custom-node.[ch] stuff into extensible.

Once we put similar routines closely, it may be better to consolidate
these routines.
As long as EXTNODENAME_MAX_LEN == CUSTOM_NAME_MAX_LEN, both features
have identical structure layout, so it is easy to call an internal
common function to register or find out a table of callbacks according
to the function actually called by other modules.

I'm inclined to think to replace EXTNODENAME_MAX_LEN and
CUSTOM_NAME_MAX_LEN by NAMEDATALEN again, to fit structure layout.

> src/backend/nodes/custom_node.c:45: indent with spaces.
> + }
>
Oops, thanks,

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

Attachment Content-Type Size
pgsql-v9.6-custom-scan-serialization-reworks.4.patch application/octet-stream 14.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-03-29 01:52:05 Re: Re: [COMMITTERS] pgsql: Only try to push down foreign joins if the user mapping OIDs mat
Previous Message Stephen Frost 2016-03-29 01:36:15 Re: extend pgbench expressions with functions