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.
So you're saying that I've insufficiently put the choke point there?
It seems to me that needing to go back to each and every jot and
tittle of the code to add a new default parameter in each place is a
recipe for mishaps, where in this one, the new correct defaults will
just appear in all the places they need to.
> 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.
I find this a good bit less confusing than the litany of repeated
parameter settings, the vast majority of which in most cases are along
the lines of NULL/NIL/etc.
This way, anything set that's not the default stands out.
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
