Re: Department of Redundancy Department: makeNode(FuncCall) division

From: David Fetter <david(at)fetter(dot)org>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Department of Redundancy Department: makeNode(FuncCall) division
Date: 2013-06-17 05:43:04
Message-ID: 20130617054304.GC19441@fetter.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 11, 2013 at 10:48:38AM -0800, David Fetter wrote:
> On Sun, Feb 10, 2013 at 10:09:19AM -0500, Tom Lane wrote:
> > David Fetter <david(at)fetter(dot)org> writes:
> > > Per suggestions and lots of help from Andrew Gierth, please find
> > > attached a patch to clean up the call sites for FuncCall nodes, which
> > > I'd like to expand centrally rather than in each of the 37 (or 38, but
> > > I only redid 37) places where it's called. The remaining one is in
> > > src/backend/nodes/copyfuncs.c, which has to be modified for any
> > > changes in the that struct anyhow.
> >
> > TBH, I don't think this is an improvement.
> >
> > The problem with adding a new field to any struct is that you have to
> > run around and examine (and, usually, modify) every place that
> > manufactures that type of struct. With a makeFuncCall defined like
> > this, you'd still have to do that; it would just become a lot easier
> > to forget to do so.
> >
> > If the subroutine were defined like most other makeXXX subroutines,
> > ie you have to supply *all* the fields, that argument would go away,
> > but the notational advantage is then dubious.
> >
> > The bigger-picture point is that you're proposing to make the coding
> > conventions for building FuncCalls different from what they are for
> > any other grammar node. I don't think that's a great idea; it will
> > mostly foster confusion.
>
> The major difference between FuncCalls and others is that `while most
> raw-parsetree nodes are constructed only in their own syntax
> productions, FuncCall is constructed in many places unrelated to
> actual function call syntax.
>
> This really will make things a good bit easier on our successors.

Here's a rebased patch with comments illustrating how best to employ.

In my previous message, I characterized the difference between
FuncCalls and other raw-parsetree nodes. Is there some flaw in that
logic? If there isn't, is there some reason not to treat them in a
less redundant, more uniform manner as this patch does?

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Attachment Content-Type Size
makeFuncArgs_002.diff text/plain 27.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2013-06-17 05:47:53 Re: pluggable compression support
Previous Message David Fetter 2013-06-17 05:36:55 Re: FILTER for aggregates [was Re: Department of Redundancy Department: makeNode(FuncCall) division]