Re: information schema parameter_default implementation

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 <peter_e(at)gmx(dot)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(at)postgresql(dot)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(dot)campero(at)anachronics(dot)com
http://www.anachronics.com