[prev in list] [next in list] [prev in thread] [next in thread]
List: postgresql-general
Subject: Re: [HACKERS] information schema parameter_default implementation
From: Ali Dar <ali.munir.dar () gmail ! com>
Date: 2013-01-31 14:02:43
Message-ID: CAAj60S7LzpjSpVrp3ZaKujFeNnuYRP_qDdoedG7dfFDDEUR03g () mail ! gmail ! com
[Download RAW message or body]
Another thing I forget: The patch does not apply because of the changes in
"catversion.h"
Regards,
Ali Dar
On Thu, Jan 31, 2013 at 6:59 PM, Ali Dar <ali.munir.dar@gmail.com> wrote:
> On Wed, Jan 9, 2013 at 4:28 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
>
>> Here is an implementation of the
>> information_schema.parameters.parameter_default column.
>>
>> I ended up writing a C function to decode the whole thing from the
>> system catalogs, because it was too complicated in SQL, so I abandoned
>> the approach discussed in [0].
>>
>>
>> [0]:
>> http://archives.postgresql.org/message-id/1356092400.25658.6.camel@vanquo.pezone.net
>>
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>
>>
> I checked our your patch. There seems to be an issue when we have OUT
> parameters after the DEFAULT values. For example a simple test case is
> given below:
>
> postgres=# CREATE FUNCTION functest1(a int default 1, out b int)
> postgres-# RETURNS int
> postgres-# LANGUAGE SQL
> postgres-# AS 'SELECT $1';
> CREATE FUNCTION
> postgres=#
> postgres=# SELECT ordinal_position, parameter_name, parameter_default FROM
> information_schema.parameters WHERE specific_name LIKE 'functest%' ORDER
> BY 1;
> ordinal_position | parameter_name | parameter_default
> ------------------+----------------+-------------------
> 1 | a | 1
> 2 | b | 1
> (2 rows)
>
> The out parameters gets the same value as the the last default parameter.
> The patch work only when default values are at the end. Switch the
> parameters and it starts working(make OUT parameter as first and default
> one the last one). Below is the example:
>
> postgres=# CREATE FUNCTION functest1(out a int, b int default 1)
> postgres-# RETURNS int
> postgres-# LANGUAGE SQL
> postgres-# AS 'SELECT $1';
> CREATE FUNCTION
> postgres=# SELECT ordinal_position, parameter_name, parameter_default FROM
> information_schema.parameters WHERE specific_name LIKE 'functest%' ORDER
> BY 1;
> ordinal_position | parameter_name | parameter_default
> ------------------+----------------+-------------------
> 1 | a |
> 2 | b | 1
> (2 rows)
>
>
> Some other minor observations:
> 1) Some variables are not lined in pg_get_function_arg_default().
> 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)
> 2) inputargn can be assigned in declaration.
> 3) Function level comment for pg_get_function_arg_default() is missing.
> 4) You can also add comments inside the function, for example the comment
> for the line:
> nth = inputargn - 1 - (proc->pronargs - proc->pronargdefaults);
> 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"?
>
> Regards,
>
> Ali Dar
>
[Attachment #3 (text/html)]
<div dir="ltr">Another thing I forget: The patch does not apply because of the \
changes in "catversion.h"<div><br></div><div style>Regards,</div><div \
style>Ali Dar</div></div><div class="gmail_extra"><br><br><div class="gmail_quote"> \
On Thu, Jan 31, 2013 at 6:59 PM, Ali Dar <span dir="ltr"><<a \
href="mailto:ali.munir.dar@gmail.com" \
target="_blank">ali.munir.dar@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <div dir="ltr"><div class="gmail_extra"><div \
class="gmail_quote"><div><div class="h5">On Wed, Jan 9, 2013 at 4:28 PM, Peter \
Eisentraut <span dir="ltr"><<a href="mailto:peter_e@gmx.net" \
target="_blank">peter_e@gmx.net</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div \
class="h5">Here is an implementation of the<br>
information_schema.parameters.parameter_default column.<br>
<br>
I ended up writing a C function to decode the whole thing from the<br>
system catalogs, because it was too complicated in SQL, so I abandoned<br>
the approach discussed in [0].<br>
<br>
<br>
[0]: <a href="http://archives.postgresql.org/message-id/1356092400.25658.6.camel@vanquo.pezone.net" \
target="_blank">http://archives.postgresql.org/message-id/1356092400.25658.6.camel@vanquo.pezone.net</a><br>
<br><br></div></div>
--<br>
Sent via pgsql-hackers mailing list (<a href="mailto:pgsql-hackers@postgresql.org" \
target="_blank">pgsql-hackers@postgresql.org</a>)<br> To make changes to your \
subscription:<br> <a href="http://www.postgresql.org/mailpref/pgsql-hackers" \
target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><br> \
<br></blockquote></div><br></div><div class="gmail_extra">I checked our your patch. \
There seems to be an issue when we have OUT parameters after the DEFAULT values. For \
example a simple test case is given below:<div><br>
<div><div><font face="courier new, monospace">postgres=# CREATE FUNCTION functest1(a \
int default 1, out b int)</font></div><div><font face="courier new, \
monospace">postgres-# RETURNS int</font></div><div><font face="courier new, \
monospace">postgres-# LANGUAGE SQL</font></div>
<div><font face="courier new, monospace">postgres-# AS 'SELECT \
$1';</font></div><div><font face="courier new, monospace">CREATE \
FUNCTION</font></div><div><font face="courier new, monospace">postgres=# \
</font></div>
<div><font face="courier new, monospace">postgres=# SELECT ordinal_position, \
parameter_name, parameter_default FROM information_schema.parameters WHERE \
specific_name LIKE 'functest%' ORDER BY 1;</font></div><div>
<font face="courier new, monospace"> ordinal_position | parameter_name | \
parameter_default </font></div><div><font face="courier new, \
monospace">------------------+----------------+-------------------</font></div><div><font \
face="courier new, monospace"> 1 | a | 1</font></div>
<div><font face="courier new, monospace"> 2 | b | \
1</font></div><div><font face="courier new, monospace">(2 \
rows)</font></div></div><div><br></div><div>The out parameters gets the same value as \
the the last default parameter. The patch work only when default values are at the \
end. Switch the parameters and it starts working(make OUT parameter as first and \
default one the last one). Below is the example:</div>
<div><br></div><div><div><font face="courier new, monospace">postgres=# CREATE \
FUNCTION functest1(out a int, b int default 1)</font></div><div><font face="courier \
new, monospace">postgres-# RETURNS int</font></div><div>
<font face="courier new, monospace">postgres-# LANGUAGE \
SQL</font></div><div><font face="courier new, monospace">postgres-# AS \
'SELECT $1';</font></div><div><font face="courier new, monospace">CREATE \
FUNCTION</font></div>
<div><font face="courier new, monospace">postgres=# SELECT ordinal_position, \
parameter_name, parameter_default FROM information_schema.parameters WHERE \
specific_name LIKE 'functest%' ORDER BY 1;</font></div><div>
<font face="courier new, monospace"> ordinal_position | parameter_name | \
parameter_default </font></div><div><font face="courier new, \
monospace">------------------+----------------+-------------------</font></div><div><font \
face="courier new, monospace"> 1 | a | </font></div>
<div><font face="courier new, monospace"> 2 | b | \
1</font></div><div><font face="courier new, monospace">(2 \
rows)</font></div><div><br></div></div><div><br></div><div>Some other minor \
observations:</div>
<div>1) Some variables are not lined in pg_get_function_arg_default().</div><div>2) I \
found the following check a bit confusing, maybe you can make it \
better</div><div><font face="courier new, monospace">if (!argmodes || argmodes[i] == \
PROARGMODE_IN || argmodes[i] == PROARGMODE_INOUT || argmodes[i] == \
PROARGMODE_VARIADIC)</font><br>
</div><div>2) inputargn can be assigned in declaration.</div></div><div>3) Function \
level comment for pg_get_function_arg_default() is missing.</div><div>4) You can also \
add comments inside the function, for example the comment for the line:</div>
<div><font face="courier new, monospace">nth = inputargn - 1 - (proc->pronargs - \
proc->pronargdefaults);</font><br></div><div>5) I think the line added in the \
documentation(informational_schema.sgml) is very long. Consider revising. Maybe \
change from:<br>
</div><div><p><font face="courier new, monospace">"The default expression of the \
parameter, or null if none or if the function is not owned by a currently enabled \
role.</font>" TO</p><p><font face="courier new, monospace">"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.</font>"</p>
<p>I don't know what do you exactly mean by: "<span \
style="font-family:'courier new',monospace">function is not owned by a \
currently enabled role"?</span></p><p>Regards,<br></p><p> Ali \
Dar</p></div></div></div> </blockquote></div><br></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic