Thread

1. Patch - Reference Function Parameters by Name

   George Gensure <werkt0@gmail.com> — 2009-11-18T14:57:07Z

   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
   

2. Re: Patch - Reference Function Parameters by Name

   Tom Lane <tgl@sss.pgh.pa.us> — 2009-11-18T17:23:28Z

   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