Обсуждение: Patch - Reference Function Parameters by Name

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:

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
2. I inserted myself more than I had expected along the road from SQL
to columnref_hook, and I'm not sure all of those lookups of parameter
names and function name are required.
3. Since it was mentioned in an earlier email that the <function
name>.<parameter name> syntax was desired, I went ahead and added
that, but it required another passthrough as indicated in 2
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.

Passes all tests in make check, and I'll add some tests for this after
I get feedback for the above items.

Regards,
-George

George Gensure <werkt0@gmail.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