Re: Out parameters handling

Hi

It was one of my worst Friday's finding out that this brain dead
implementation of out parameters had been part of fuck up again.
This time we did notice it two days too late.
I wish for a way to use out parameters in functions only through some
predefined prefix like in triggers new and old. Means i would like to limit
referencing to out parameters to one prefix only defined in the beginning of
declare section of stored procedure.
It really sucks what kind of mistakes you can pass to production
unknowingly. I would much prefer a way to prevent such nonsense.
Here was the case where out parameters were with same names with select into
field names resulting in null outcome. Just yesterday we had similar case
with update statement.

regards
Asko

On Fri, Mar 6, 2009 at 4:29 PM, Asko Oja <ascoja(at)gmail(dot)com> wrote:

> It was one of my worst Friday's finding out that this brain dead
> implementation of out parameters had been part of fuck up again.

:)

>
> This time we did notice it two days too late.
> I wish for a way to use out parameters in functions only through some
> predefined prefix like in triggers new and old. Means i would like to limit
> referencing to out parameters to one prefix only defined in the beginning of
> declare section of stored procedure.
> It really sucks what kind of mistakes you can pass to production
> unknowingly. I would much prefer a way to prevent such nonsense.
> Here was the case where out parameters were with same names with select
> into field names resulting in null outcome. Just yesterday we had similar
> case with update statement.
>

Well, it's a problem with the language not parsing things correctly and
doing, in many cases, brain-dead replacements. I don't know of any
developer using OUT parameters that doesn't run into this problem at one
time or another :(

--
Jonah H. Harris, Senior DBA
myYearbook.com

>>> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> wrote:
> On Fri, Mar 6, 2009 at 4:29 PM, Asko Oja <ascoja(at)gmail(dot)com> wrote:
>> It really sucks what kind of mistakes you can pass to production
>> unknowingly. I would much prefer a way to prevent such nonsense.
>> Here was the case where out parameters were with same names with
>> select into field names resulting in null outcome. Just yesterday
>> we had similar case with update statement.
>
> Well, it's a problem with the language not parsing things correctly
> and doing, in many cases, brain-dead replacements. I don't know of
> any developer using OUT parameters that doesn't run into this
> problem at one time or another :(

I find the PostgreSQL implementation of OUT parameters, well,
surprising. I've used databases where stored procedures can have a
RETURN value, OUT parameters, and result streams as three discreet
things which can't be mistaken for one another -- which seems more
sensible. Is this issue in PostgreSQL a spin-off of not having stored
procedures, and trying to shoehorn SP behavior into functions?

I suspect that a really good fix would require a new version of the
PostgreSQL protocol.

-Kevin

Kevin Grittner escribió:

> I find the PostgreSQL implementation of OUT parameters, well,
> surprising. I've used databases where stored procedures can have a
> RETURN value, OUT parameters, and result streams as three discreet
> things which can't be mistaken for one another -- which seems more
> sensible. Is this issue in PostgreSQL a spin-off of not having stored
> procedures, and trying to shoehorn SP behavior into functions?

I think the current behavior is more a result of Postgres not having
host variables.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Fri, Mar 6, 2009 at 4:29 PM, Asko Oja <ascoja(at)gmail(dot)com> wrote:
> It was one of my worst Friday's finding out that this brain dead
> implementation of out parameters had been part of fuck up again.
> This time we did notice it two days too late.
> I wish for a way to use out parameters in functions only through some
> predefined prefix like in triggers new and old. Means i  would like to limit
> referencing to out parameters to one prefix only defined in the beginning of
> declare section of stored procedure.
> It really sucks what kind of mistakes you can pass to production
> unknowingly. I would much prefer a way to prevent such nonsense.
> Here was the case where out parameters were with same names with select into
> field names resulting in null outcome. Just yesterday we had similar case
> with update statement.

This is indeed sucky, but sadly it goes well beyond out parameters.
For example:

rhaas=# CREATE FUNCTION test(v integer) RETURNS integer AS $$
BEGIN
RETURN (SELECT v.id FROM foo v WHERE v.id = v);
END
$$ LANGUAGE plpgsql;
ERROR: syntax error at or near "$1"
LINE 1: SELECT (SELECT v.id FROM foo $1 WHERE v.id = $1 )
^
QUERY: SELECT (SELECT v.id FROM foo $1 WHERE v.id = $1 )
CONTEXT: SQL statement in PL/PgSQL function "test" near line 2

It's obviously quite impossible for "foo v" to mean "foo $1", but that
doesn't stop the compiler from substituting it. (The error message
isn't great either). And then of course you can select an
in-parameter when you meant to select a column:

CREATE FUNCTION test(id integer) RETURNS integer AS $$
BEGIN
RETURN (SELECT id FROM foo WHERE v.id < id);
END
$$ LANGUAGE plpgsql;

Of course in a simple example like this you might be lucky enough to
notice the problem, but in a more complicated function with several
large queries and a few loops it's very easy to miss. I usually
manage to catch them before I roll them out, but I've definitely
wasted a lot of time being confused about why the results didn't make
any sense.

As someone pointed out downthread, what we really need is a
distinction between host variables and guest variables.

http://www.postgresql.org/docs/8.3/static/ecpg-variables.html

I wonder whether it would be possible to make PL/pgsql take :foo to
mean the parameter named foo, and then provide an option to make that
THE ONLY WAY to refer to the parameter foo. For
backward-compatibility, and compatibility with (ahem) other database
products, we probably don't want to remove the option to have foo
mean... any damn thing named foo you can put your hands on. But it
would be nice to at least have the option of disabling that behavior
when compatibility is not an issue, and correctness is.

...Robert

Robert,

> I wonder whether it would be possible to make PL/pgsql take :foo to
> mean the parameter named foo, and then provide an option to make that
> THE ONLY WAY to refer to the parameter foo. For
> backward-compatibility, and compatibility with (ahem) other database
> products, we probably don't want to remove the option to have foo
> mean... any damn thing named foo you can put your hands on. But it
> would be nice to at least have the option of disabling that behavior
> when compatibility is not an issue, and correctness is.

Thing is, anybody can institute their own naming convention. I've long
used v_ as a prefix. Allowing : would save me some keystrokes, but
that's about it.

--Josh

On Fri, Mar 6, 2009 at 8:44 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
> Robert,
>
> Thing is, anybody can institute their own naming convention.  I've long used
> v_ as a prefix.  Allowing : would save me some keystrokes, but that's about
> it.
>
> --Josh

True... but there doesn't seem to be any shortage of people who are
annoyed by the current behavior. Maybe we should all just learn to
live with it.

...Robert

It wouldn't be so bad if you could assign internal and external column names.

Within the function you call the column "v_foo" but the caller of the
function receives column "foo" instead.

OUT v_foo varchar AS "foo"

Another alternative is requiring a prefix like plout for the
replacement to occur:

( OUT foo varchar )

BEGIN
SELECT foo.somename INTO plout.foo FROM foo WHERE id = 10;

RETURN NEXT;

RETURN;
END;

On Sat, Mar 7, 2009 at 8:50 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Mar 6, 2009 at 8:44 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>> Robert,
>>
>> Thing is, anybody can institute their own naming convention.  I've long used
>> v_ as a prefix.  Allowing : would save me some keystrokes, but that's about
>> it.
>>
>> --Josh
>
> True... but there doesn't seem to be any shortage of people who are
> annoyed by the current behavior.  Maybe we should all just learn to
> live with it.
>
> ...Robert
>
> --
> 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
>

On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
> It wouldn't be so bad if you could assign internal and external column names.
>
> Within the function you call the column "v_foo" but the caller of the
> function receives column "foo" instead.
>
> OUT v_foo varchar AS "foo"
>
>
> Another alternative is requiring a prefix like plout for the
> replacement to occur:
>
> ( OUT foo varchar )
>
> BEGIN
>  SELECT foo.somename INTO plout.foo FROM foo WHERE id = 10;
>
>  RETURN NEXT;
>
>  RETURN;
> END;

This is a good point. Uglifying the parameter names is sort of OK for
input parameters, but is much more annoying for output parameters.

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
>> It wouldn't be so bad if you could assign internal and external column names.

> This is a good point. Uglifying the parameter names is sort of OK for
> input parameters, but is much more annoying for output parameters.

How much of this pain would go away if we changed over to the arguably
correct (as in Or*cle does it that way) scoping for names, wherein the
parser first tries to match a name against column names of tables of the
current SQL statement, and only failing that looks to see if they are
plpgsql variables?

regards, tom lane

2009/3/7 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
>> It wouldn't be so bad if you could assign internal and external column names.
>>
>> Within the function you call the column "v_foo" but the caller of the
>> function receives column "foo" instead.
>>
>> OUT v_foo varchar AS "foo"
>>
>>
>> Another alternative is requiring a prefix like plout for the
>> replacement to occur:
>>
>> ( OUT foo varchar )
>>
>> BEGIN
>>  SELECT foo.somename INTO plout.foo FROM foo WHERE id = 10;
>>
>>  RETURN NEXT;
>>
>>  RETURN;
>> END;
>
> This is a good point.  Uglifying the parameter names is sort of OK for
> input parameters, but is much more annoying for output parameters.
>
> ...Robert
>

hello

actually - function name should be used as label now. This code is working:

postgres=# create or replace function fx2(a integer, out b integer,
out c integer) as $$
begin
fx2.b := a + 10; fx2.c := a + 30;
return;
end; $$ language plpgsql;
CREATE FUNCTION
postgres=# select * from fx2(20);
┌────┬────┐
│ b │ c │
├────┼────┤
│ 30 │ 50 │
└────┴────┘
(1 row)

regards
Pavel Stehule

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
>>> It wouldn't be so bad if you could assign internal and external column names.
>
>> This is a good point. Uglifying the parameter names is sort of OK for
>> input parameters, but is much more annoying for output parameters.
>
> How much of this pain would go away if we changed over to the arguably
> correct (as in Or*cle does it that way) scoping for names, wherein the
> parser first tries to match a name against column names of tables of the
> current SQL statement, and only failing that looks to see if they are
> plpgsql variables?

I'm not sure that's any better. The case where I've run into this is when I
have something like:

balance := new value
UPDATE tab SET balance = balance

In that case the only way we could get it right is if we default to the local
variable but only in contexts where an expression is valid.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> I'm not sure that's any better. The case where I've run into this is when I
> have something like:
> balance := new value
> UPDATE tab SET balance = balance
> In that case the only way we could get it right is if we default to the local
> variable but only in contexts where an expression is valid.

AFAICS getting that "right" would require the parser to develop advanced
mind reading capabilities. We could probably fix it to know that the
first "balance" must be a table column name, but there is no principled
way to make a choice about the second one; and you could easily invent
slightly different scenarios where resolving it as the column name is
the right thing.

Anyway, I'm unsure whether this is related to the complaints upthread,
which is why I was asking.

regards, tom lane

> actually - function name should be used as label now. This code is working:

Not helpful for me. The most typical conflict I have is actually the
OUT parameter and table name, not a column of the table.

Really don't want to prefix all tables with a hardcoded schema or do
variable substitution for loading the document.

Not fond of prefixing with function name either as a) many of my
functions have very long names and b) they change names occasionally,
particularly during development.

A short prefix like "out" would be useful. I would immediately start
prefixing all uses.

rbt=# begin;
BEGIN
rbt=# create table b (col integer);
CREATE TABLE
rbt=# insert into b values (2);
INSERT 0 1
rbt=# create or replace function fx2(a integer, out b integer) as $$
rbt$# begin
rbt$# SELECT col
rbt$# INTO fx2.b
rbt$# FROM b;
rbt$#
rbt$# return;
rbt$# end; $$ language plpgsql;
ERROR: syntax error at or near "$1"
LINE 1: SELECT col FROM $1
^
QUERY: SELECT col FROM $1
CONTEXT: SQL statement in PL/PgSQL function "fx2" near line 4
rbt=#

On Sat, Mar 7, 2009 at 11:32 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
>>> It wouldn't be so bad if you could assign internal and external column names.
>
>> This is a good point.  Uglifying the parameter names is sort of OK for
>> input parameters, but is much more annoying for output parameters.
>
> How much of this pain would go away if we changed over to the arguably
> correct (as in Or*cle does it that way) scoping for names, wherein the
> parser first tries to match a name against column names of tables of the
> current SQL statement, and only failing that looks to see if they are
> plpgsql variables?

This would solve all of my conflicts correctly. I nearly always use
RETURN QUERY with OUT parameters.

An alternative would be the requirement to prefix out parameters with
"out", "export", or something similar, so the plain non-prefixed name
is never replaced.

"b" in the below is the table.

I hit this quite a bit since my historical table name might be
"foo_bar_baz" which is the same as the most relevant name for the out
parameter.

I've debated renaming all of my tables t_* on more than one occasion
as a workaround in applications which exclusively use functions to
access/write data.

create or replace function read_some_data_from_data_region(a integer,
out b integer) as $$
begin
SELECT col
INTO out.b
FROM b;

return;
end; $$ language plpgsql;

Hello

2009/3/7 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
>> I'm not sure that's any better. The case where I've run into this is when I
>> have something like:
>>  balance := new value
>>  UPDATE tab SET balance = balance
>> In that case the only way we could get it right is if we default to the local
>> variable but only in contexts where an expression is valid.
>
> AFAICS getting that "right" would require the parser to develop advanced
> mind reading capabilities.  We could probably fix it to know that the
> first "balance" must be a table column name, but there is no principled
> way to make a choice about the second one; and you could easily invent
> slightly different scenarios where resolving it as the column name is
> the right thing.
>
> Anyway, I'm unsure whether this is related to the complaints upthread,
> which is why I was asking.
>
>                        regards, tom lane

I thing, we mainly need detection of this situation. It is same as
detection of ambiguous column names in SQL. PL/pgSQL has enough tools
for solving - main problem is in detection. After detection of some
possible conflict we should to raise exception or warning (controlled
by GUC).

regards
Pavel Stehule

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

Hello

2009/3/7 Rod Taylor <rod(dot)taylor(at)gmail(dot)com>:
>> actually - function name should be used as label now. This code is working:
>
> Not helpful for me. The most typical conflict I have is actually the
> OUT parameter and table name, not a column of the table.
>

This conflict I never meet. And I afraid so this should not be solved.
One typical beginer's bug has similar symptoms.

create function foo(tablename varchar, param varchar, paramname
varchar) returns ..
begin
select into .. .. from tablename where .paramname = param
....

This is bug - who can understand, if this is desired behave or nonsense.

you have to use dynamic SQL. All what are inside literal, are independent.

postgres=# create table wrong(a integer);
CREATE TABLE
postgres=# insert into wrong values(10);
INSERT 0 1
postgres=# create function fx3(out wrong varchar) returns setof varchar as $$
begin
for wrong in execute 'select * from wrong'
loop
return next;
end loop;
return; end; $$ language plpgsql;
CREATE FUNCTION
postgres=# select * from fx3();
┌───────┐
│ wrong │
├───────┤
│ 10 │
└───────┘
(1 row)

regards
Pavel Stehule

Actually dynamic sql are little bit uncomfortable. It's much better in 8.4.

regards
Pavel Stehule

> Really don't want to prefix all tables with a hardcoded schema or do
> variable substitution for loading the document.
>
> Not fond of prefixing with function name either as a) many of my
> functions have very long names and b) they change names occasionally,
> particularly during development.
>
> A short prefix like "out" would be useful. I would immediately start
> prefixing all uses.
>
> rbt=# begin;
> BEGIN
> rbt=# create table b (col integer);
> CREATE TABLE
> rbt=# insert into b values (2);
> INSERT 0 1
> rbt=# create or replace function fx2(a integer, out b integer) as $$
> rbt$# begin
> rbt$#   SELECT col
> rbt$#     INTO fx2.b
> rbt$#     FROM b;
> rbt$#
> rbt$#   return;
> rbt$# end; $$ language plpgsql;
> ERROR:  syntax error at or near "$1"
> LINE 1: SELECT col FROM  $1
>                         ^
> QUERY:  SELECT col FROM  $1
> CONTEXT:  SQL statement in PL/PgSQL function "fx2" near line 4
> rbt=#
>

Hi,

Le 7 mars 09 à 02:44, Josh Berkus a écrit :
> Thing is, anybody can institute their own naming convention. I've
> long used v_ as a prefix. Allowing : would save me some keystrokes,
> but that's about it.

What I usually do in those cases is abusing the ALIAS option of
DECLARE (because as mentioned somewhere else in this thread, you
generally don't want to have that ugly OUT parameters, you want a nice
API) :

CREATE OR REPLACE FUNCTION test_out
(
IN a integer,
IN b integer,
OUT s integer
)
RETURNS setof integer
LANGUAGE PLPGSQL
AS $f$
DECLARE
v_s ALIAS FOR $3;
BEGIN
FOR v_s IN SELECT generate_series(a, b)
LOOP
v_s := v_s * v_s;
RETURN NEXT;
END LOOP;
RETURN;
END;
$f$;

CREATE FUNCTION
dim=# SELECT * FROM test_out(2, 4);
s
----
4
9
16
(3 rows)

I'd sure be happy not having to do it explicitly, but schema-style
prefixing has the drawback of needing to avoid any user defined
schema. Maybe pg_plout would do?

Regards,
--
dim

In fact, maybe a new option to set the OUT parameters prefix to use
from within the function body would do?

Le 7 mars 09 à 19:56, Dimitri Fontaine a écrit :
> CREATE OR REPLACE FUNCTION test_out
> (
> IN a integer,
> IN b integer,
> OUT s integer
> )
> RETURNS setof integer

SET out_prefix TO 'v_'

> LANGUAGE PLPGSQL
> AS $f$

Those two following lines would be deprecated:

> DECLARE
> v_s ALIAS FOR $3;

> BEGIN
> FOR v_s IN SELECT generate_series(a, b)
> LOOP
> v_s := v_s * v_s;
> RETURN NEXT;
> END LOOP;
> RETURN;
> END;
> $f$;
>
> CREATE FUNCTION
> dim=# SELECT * FROM test_out(2, 4);
> s
> ----
> 4
> 9
> 16
> (3 rows)

--
dim

Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> I'd sure be happy not having to do it explicitly, but schema-style
> prefixing has the drawback of needing to avoid any user defined
> schema.

No, not really, because it'd be the wrong number of naming levels.

Assuming that we were to switch to Oracle-style naming rules, we
would have:

x in the context of a table name = table x

x.y in the context of a table name = table y, schema x

x in the context of an expression = first of
column x from some table of the current command
most-closely-nested plpgsql variable x

x.y in the context of an expression = first of
column y from table x of the current command
plpgsql variable y in block x

The important point here is that the main SQL parser can tell whether
it's looking at a table name or a column name, whereas plpgsql is
currently too stupid for that and will always substitute for a name
that matches a plpgsql variable name. Once we get rid of that problem
there isn't really any conflict with schema names. You might have a
conflict between table aliases and block names, but that can be
dealt with by local renaming of aliases within the problematic command.

(Note: as pointed out by Pavel, it's already the case that named
parameters are implicitly assigned a block name equal to the function
name; so you can qualify them if you have to.)

regards, tom lane

On Sat, Mar 7, 2009 at 11:32 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
>>> It wouldn't be so bad if you could assign internal and external column names.
>
>> This is a good point.  Uglifying the parameter names is sort of OK for
>> input parameters, but is much more annoying for output parameters.
>
> How much of this pain would go away if we changed over to the arguably
> correct (as in Or*cle does it that way) scoping for names, wherein the
> parser first tries to match a name against column names of tables of the
> current SQL statement, and only failing that looks to see if they are
> plpgsql variables?

I think that would definitely be an improvement. Would that mean that
in a query like the following:

SELECT t.id FROM test t WHERE t.id = 17

...it wouldn't consider replacing "t"? That all by itself would be an
improvement...

I actually feel like the best thing to do would be to error out if
there's an ambiguous reference. If you write this:

SELECT id FROM foo, bar WHERE foo.a = bar.a

...it will complain if both foo.id and bar.id are defined. So if I write:

SELECT id FROM foo

...shouldn't it complain if both foo.id and <parameter namespace>.id
are defined?

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I think that would definitely be an improvement. Would that mean that
> in a query like the following:

> SELECT t.id FROM test t WHERE t.id = 17

> ...it wouldn't consider replacing "t"? That all by itself would be an
> improvement...

It's already the case that plpgsql knows enough to not replace "t"
in the context "t.something". But I suppose you are talking about the
alias declaration. Yeah, that should get better if we push this into
the main parser.

> I actually feel like the best thing to do would be to error out if
> there's an ambiguous reference. If you write this:
> SELECT id FROM foo, bar WHERE foo.a = bar.a
> ...it will complain if both foo.id and bar.id are defined. So if I write:
> SELECT id FROM foo
> ...shouldn't it complain if both foo.id and <parameter namespace>.id
> are defined?

No, on the principle that more closely nested definitions take
precedence. The reason the first example merits an error is that the
two possible sources of the name have equal precedence.

regards, tom lane

On Sat, Mar 7, 2009 at 5:08 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I think that would definitely be an improvement.  Would that mean that
>> in a query like the following:
>
>> SELECT t.id FROM test t WHERE t.id = 17
>
>> ...it wouldn't consider replacing "t"?  That all by itself would be an
>> improvement...
>
> It's already the case that plpgsql knows enough to not replace "t"
> in the context "t.something".  But I suppose you are talking about the
> alias declaration.  Yeah, that should get better if we push this into
> the main parser.

+1 from me then.

>> I actually feel like the best thing to do would be to error out if
>> there's an ambiguous reference.  If you write this:
>> SELECT id FROM foo, bar WHERE foo.a = bar.a
>> ...it will complain if both foo.id and bar.id are defined.  So if I write:
>> SELECT id FROM foo
>> ...shouldn't it complain if both foo.id and <parameter namespace>.id
>> are defined?
>
> No, on the principle that more closely nested definitions take
> precedence.  The reason the first example merits an error is that the
> two possible sources of the name have equal precedence.

That's reasonable, but I'm not a huge fan. The fact that host and
guest variables live in the same namespace is a huge source of bugs.
Your idea above is an improvement IMO but I wish there were some way
to make it airtight.

...Robert

2009/3/7 Dimitri Fontaine <dfontaine(at)hi-media(dot)com>:
> In fact, maybe a new option to set the OUT parameters prefix to use from
> within the function body would do?
>
> Le 7 mars 09 à 19:56, Dimitri Fontaine a écrit :
>>
>> CREATE OR REPLACE FUNCTION test_out
>> (
>>  IN  a integer,
>>  IN  b integer,
>>  OUT s integer
>> )
>> RETURNS setof integer
>
>  SET out_prefix TO 'v_'

-1

this is out of PL languages. There is not well enough solved access to
table out variables. Actually these variables are same as out
variables, but internally we should distinct between. For example:
PL/pgPSM don't declare it as variables - so there isn't possible any
conflict.

fragment of plpgpsm code

create or replace function test_out(a int, b int)
returns table (s int) as $$
return table(select s from some)
$$ language plpgpsm

is correct.

regards
Pavel Stehule

>> LANGUAGE PLPGSQL
>> AS $f$
>
> Those two following lines would be deprecated:
>
>> DECLARE
>>  v_s ALIAS FOR $3;
>
>
>> BEGIN
>>  FOR v_s IN SELECT generate_series(a, b)
>>  LOOP
>>   v_s := v_s * v_s;
>>   RETURN NEXT;
>>  END LOOP;
>>  RETURN;
>> END;
>> $f$;
>>
>> CREATE FUNCTION
>> dim=# SELECT * FROM test_out(2, 4);
>> s
>> ----
>>  4
>>  9
>> 16
>> (3 rows)
>
> --
> dim
>
>
> --
> 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
>

On Sat, Mar 7, 2009 at 9:29 PM, Dimitri Fontaine <dfontaine(at)hi-media(dot)com>wrote:

> In fact, maybe a new option to set the OUT parameters prefix to use from
> within the function body would do?
>
> Le 7 mars 09 à 19:56, Dimitri Fontaine a écrit :
>
>> CREATE OR REPLACE FUNCTION test_out
>> (
>> IN a integer,
>> IN b integer,
>> OUT s integer
>> )
>> RETURNS setof integer
>>
>
> SET out_prefix TO 'v_'
>
> LANGUAGE PLPGSQL
>> AS $f$
>>
>
That's what we also would like to have. In addition it should also make out
parameters unusable without that prefix. Then we could make it our coding
standard and feel relatively safe again.

>
>> Those two following lines would be deprecated:
>
>
> DECLARE
>> v_s ALIAS FOR $3;
>>
>
>
> BEGIN
>> FOR v_s IN SELECT generate_series(a, b)
>> LOOP
>> v_s := v_s * v_s;
>> RETURN NEXT;
>> END LOOP;
>> RETURN;
>> END;
>> $f$;
>>
>> CREATE FUNCTION
>> dim=# SELECT * FROM test_out(2, 4);
>> s
>> ----
>> 4
>> 9
>> 16
>> (3 rows)
>>
>
> --
> dim
>
>

Hello Robert,

I have been bitten by this problem many times as well.

> I wonder whether it would be possible to make PL/pgsql take :foo to
> mean the parameter named foo, and then provide an option to make that
> THE ONLY WAY to refer to the parameter foo.  For
> backward-compatibility, and compatibility with (ahem) other database
> products, we probably don't want to remove the option to have foo
> mean... any damn thing named foo you can put your hands on.  But it
> would be nice to at least have the option of disabling that behavior
> when compatibility is not an issue, and correctness is.

This is one of the things I wanted to start looking at for 8.5.
My idea was to optionally use : or @ (not sure which is more popular) to
specify this token is only a variable. Do not try to match it to columns or
other database object. If the variable did not start with : or @ then normal
rules would apply for backwards compatibility.

No idea how feasible this plan is, I was just hoping to find a way to solve this
problem.

Thanks,

- Ryan

Ryan Bradetich <rbradetich(at)gmail(dot)com> writes:
> This is one of the things I wanted to start looking at for 8.5.
> My idea was to optionally use : or @ (not sure which is more popular) to
> specify this token is only a variable.

This whole line of thought is really a terrible idea IMHO. plpgsql is
supposed to follow Oracle's pl/sql syntax, not invent random syntax of
its own. I believe that 80% of the problems here are occurring because
we used a crude substitution method that got the priorities backwards
from the way Oracle does it.

regards, tom lane

On Sun, Mar 8, 2009 at 4:36 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Ryan Bradetich <rbradetich(at)gmail(dot)com> writes:
>> This is one of the things I wanted to start looking at for 8.5.
>> My idea was to optionally use : or @ (not sure which is more popular) to
>> specify this token is only a variable.
>
> This whole line of thought is really a terrible idea IMHO.  plpgsql is
> supposed to follow Oracle's pl/sql syntax, not invent random syntax of
> its own.  I believe that 80% of the problems here are occurring because
> we used a crude substitution method that got the priorities backwards
> from the way Oracle does it.

Fair Enough. I just hope what every solution the community decides upon
solves this problem. It is a very annoying problem to track down and I tend
to get even more agitated when I figure out this is the problem.

I do not want to distract from the release efforts, so I will withhold further
comments until the 8.5 development cycle.

Thanks,

- Ryan

2009/3/9 Ryan Bradetich <rbradetich(at)gmail(dot)com>:
> On Sun, Mar 8, 2009 at 4:36 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Ryan Bradetich <rbradetich(at)gmail(dot)com> writes:
>>> This is one of the things I wanted to start looking at for 8.5.
>>> My idea was to optionally use : or @ (not sure which is more popular) to
>>> specify this token is only a variable.
>>
>> This whole line of thought is really a terrible idea IMHO.  plpgsql is
>> supposed to follow Oracle's pl/sql syntax, not invent random syntax of
>> its own.  I believe that 80% of the problems here are occurring because
>> we used a crude substitution method that got the priorities backwards
>> from the way Oracle does it.
>
> Fair Enough.   I just hope what every solution the community decides upon
> solves this problem.  It is a very annoying problem to track down and I tend
> to get even more agitated when I figure out this is the problem.
>
> I do not want to distract from the release efforts, so I will withhold further
> comments until the 8.5 development cycle.
>

We could relative simple don't add OUT variables into namespace.
Personally I prefer using dynamic sql for this case - 8.4 will support
RETURN QUERY EXECUTE too, but I don't see big problem in following
solution. With special interpret parameter #without_out_paramnames (or
some similar) we should protect "nice" out variables.

/* out parameters are accessible via $notation */
create function foo(OUT nicevar integer) returns setof record as $$
#without_out_paramnames
begin
return query select nicevar from .....
end
$$ language ...

with dynamic sql it is easy too

create function foo(out nicevar integer) returns ...
begin
return query execute 'select nicevar from ... '
end
$$ language

regard
Pavel Stehule

some special prefixes or special syntax is some what I dislike.

> Thanks,
>
> - Ryan
>
> --
> 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
>

On 3/7/09, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > On Sat, Mar 7, 2009 at 9:08 AM, Rod Taylor <rod(dot)taylor(at)gmail(dot)com> wrote:
> >> It wouldn't be so bad if you could assign internal and external column names.
>
> > This is a good point. Uglifying the parameter names is sort of OK for
> > input parameters, but is much more annoying for output parameters.
>
> How much of this pain would go away if we changed over to the arguably
> correct (as in Or*cle does it that way) scoping for names, wherein the
> parser first tries to match a name against column names of tables of the
> current SQL statement, and only failing that looks to see if they are
> plpgsql variables?

It would decrease the frequency of problems, but the problems that
will stay will be more obscure than before - currently you can guess
how the query will be parsed by just looking at function code, but
with oracle style parsing you need to know the table definitions also.

So my vote would go to some sort of alias or record variable
that contains either all arguments (so we can get rid of i_ prefixes)
or only out/inout variables.

This should be optional and user-chooseable at function start, so it can
be tied with local coding style. This seems to fit better to SQL style
of using table or column aliases to make name resolution clear.

I don't have clear idea of syntax for that, some variants:

DECLARE
args ALIAS FOR IN|OUT|INOUT ARGS;
args PREFIX FOR ARGS|OUTARGS|INARGS;
ARGALIAS ret;
RENAME funcname TO foo;

The PREFIX seems best of those as it also hints that the args will not be
available in plain form. Not sure what is the good way to specify the
IN|OUT|INOUT.

I'm not against the Oracle-style parsing, if the prefix solution is voted
down, it will be the next best thing. But my problem with it is that it
will make me actually less confident than current solution that
I really understand what a piece of SQL will actually end up doing.

Also it will introduce new ways to silent breakdowns: what if someone
adds new column to table with same name as function argument?

Btw - the prefix and the Oracle-style parsing are actually orthogonal
to each other so we could also have both.

--
marko

>> How much of this pain would go away if we changed over to the arguably
>> correct (as in Or*cle does it that way) scoping for names, wherein the
>> parser first tries to match a name against column names of tables of the
>> current SQL statement, and only failing that looks to see if they are
>> plpgsql variables?

-1 on this. If we're to have definite rules, I would prefer that stuff
gets assumed to be a variable *first*, and then object definitions are
only examined after the system fails to find a matching variable name.

That priority makes it much easier to debug a function than the Oracle way.

--Josh

Josh Berkus <josh(at)agliodbs(dot)com> writes:
>>> How much of this pain would go away if we changed over to the arguably
>>> correct (as in Or*cle does it that way) scoping for names, wherein the
>>> parser first tries to match a name against column names of tables of the
>>> current SQL statement, and only failing that looks to see if they are
>>> plpgsql variables?

> -1 on this. If we're to have definite rules, I would prefer that stuff
> gets assumed to be a variable *first*, and then object definitions are
> only examined after the system fails to find a matching variable name.

Well, we have boatloads of bug reports that say you're wrong on that,
not to mention the Oracle precedent.

regards, tom lane