Re: information schema parameter_default implementation

Here is an updated patch which fixes the bug you have pointed out.

On Thu, 2013-01-31 at 18:59 +0500, Ali Dar wrote:

> I checked our your patch. There seems to be an issue when we have OUT
> parameters after the DEFAULT values.

Fixed.

> Some other minor observations:
> 1) Some variables are not lined in pg_get_function_arg_default().

Are you referring to indentation issues?  I think the indentation is
good, but pgindent will fix it anyway.

> 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.

>
> 2) inputargn can be assigned in declaration.

I'd prefer to initialize it close to where it is used.

> 3) Function level comment for pg_get_function_arg_default() is
> missing.

I think the purpose of the function is clear.

> 4) You can also add comments inside the function, for example the
> comment for the line:
> nth = inputargn - 1 - (proc->pronargs - proc->pronargdefaults);

Suggestion?

> 5) I think the line added in the
> documentation(informational_schema.sgml) is very long. Consider
> revising. Maybe change from:
>
> "The default expression of the parameter, or null if none or if the
> function is not owned by a currently enabled role." TO
>
> "The default expression of the parameter, or null if none was
> specified. It will also be null if the function is not owned by a
> currently enabled role."
>
> I don't know what do you exactly mean by: "function is not owned by a
> currently enabled role"?

I think this style is used throughout the documentation of the
information schema.  We need to keep the descriptions reasonably
compact, but I'm willing to entertain other opinions.




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers