Re: Department of Redundancy Department: makeNode(FuncCall) division - Mailing list 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@fetter.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@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers