Peter, This patch no longer applies, because CATALOG_VERSION_NO in src/include/catalog/catversion.h has changed. I touched the patch and got it to apply without other problems (I haven't built yet).
Regards, 2013/11/14 Peter Eisentraut <pete...@gmx.net> > Updated patch attached. > > On Sat, 2013-11-09 at 12:09 +0530, Amit Khandekar wrote: > > > > 2) I found the following check a bit confusing, maybe you can make > > it > > > > better > > > > if (!argmodes || argmodes[i] == PROARGMODE_IN || argmodes[i] == > > > > PROARGMODE_INOUT || argmodes[i] == PROARGMODE_VARIADIC) > > > > > > Factored that out into a separate helper function. > > > > > > The statement needs to be split into 80 columns width lines. > > done > > > > > 3) Function level comment for pg_get_function_arg_default() is > > > > missing. > > > > > > I think the purpose of the function is clear. > > > > Unless a reader goes through the definition, it is not obvious whether > > the second function argument represents the parameter position in > > input parameters or it is the parameter position in *all* the function > > parameters (i.e. proargtypes or proallargtypes). I think at least a > > one-liner description of what this function does should be present. > > done > > > > > > > > 4) You can also add comments inside the function, for example the > > > > comment for the line: > > > > nth = inputargn - 1 - (proc->pronargs - proc->pronargdefaults); > > > > > > Suggestion? > > > > Yes, it is difficult to understand what's the logic behind this > > calculation, until we go read the documentation related to > > pg_proc.proargdefaults. I think this should be sufficient: > > /* > > * proargdefaults elements always correspond to the last N input > > arguments, > > * where N = pronargdefaults. So calculate the nth_default accordingly. > > */ > > done > > > There should be an initial check to see if nth_arg is passed a > > negative value. It is used as an index into the argmodes array, so a > > -ve array index can cause a crash. Because > > pg_get_function_arg_default() can now be called by a user also, we > > need to validate the input values. I am ok with returning with an > > error "Invalid argument". > > done > > > --- > > Instead of : > > deparse_expression_pretty(node, NIL, false, false, 0, 0) > > you can use : > > deparse_expression(node, NIL, false, false) > > > done > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- Rodolfo Campero Anachronics S.R.L. Tel: (54 11) 4899 2088 rodolfo.camp...@anachronics.com http://www.anachronics.com
