Re: Patch - Reference Function Parameters by Name

George Gensure <werkt0(at)gmail(dot)com> writes:
> Attached is a patch to perform parameter reference lookups by name in
> the body of functions. I'm hesitant to put it in for the commitfest
> as is, without a couple of questions posed to the group:

I looked through this very quickly. I'm not in favor of the approach
you have chosen of hacking all the upper layers in order to pass
parameter names through them; and putting the function name into those
APIs too is right out. What I did in plpgsql avoided that by
establishing a callback protocol and keeping all the knowledge of names
within the callback function. SQL functions have a different call path
to the parser, so we might need to adjust things in that path; but you
definitely should not be needing to mess with plancache.c any further.

> 1. palloc needs no free? I suppose this is a general knowledge
> question, but it seemed to be the case after trying to look for
> deallocation

Depends. If you're creating something that is meant to live about
as long as the current statement anyway, you can just leave it to be
garbage-collected when the current memory context is destroyed.
There are cases where you need to be more aggressive about pfree'ing
things to avoid large cumulative memory usage, but probably anything
that is invoking parsing doesn't really need to worry (the parse process
is going to create a whole lot more trash than you will anyway).

> 4. I made a judgement call in terms of resolution: if the
> columnref_hook for param-by-name resolution is called with a non-null
> node (meaning a column was already found), we avoid being an ambiguous
> reference, and prefer the column already found.

The consensus seems to be that we should throw error for ambiguity.

regards, tom lane