Is this really really as designed or defined in some standard

On Mon, Sep 01, 2008 at 12:55:21AM +0300, Hannu Krosing wrote:

It seems that we allow several function arguments to have same
name (or is it label :)

Ugh!

hannu=# create or replace function ff(a int, a int) returns int language
plpgsql as $$begin return $1+$2; end;$$;
CREATE FUNCTION
hannu=# select ff(1,1);
ff
----
2
(1 row)

hannu=# select ff(1,2);
ff
----
3
(1 row)

hannu=# create or replace function ffa(a int, a int) returns int
language plpgsql as $$begin return a + a; end;$$;
CREATE FUNCTION
hannu=# select ffa(1,2);
ffa
-----
2
(1 row)

Is this defined by some standard or just an oversight ?

This looks like a bug.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

hannu=# create or replace function ffa(a int, a int) returns int
language plpgsql as $$begin return a + a; end;$$;
CREATE FUNCTION
hannu=# select ffa(1,2);
ffa
-----
2
(1 row)

Is this defined by some standard or just an oversight ?

It's just an oversight.

What about the similar program

create or replace function ffa(int, int)
returns int language plpgsql
as $$
DECLARE
a ALIAS FOR $1;
a ALIAS FOR $2;
begin
return a + a;
end;
$$;

I think ffa(a int, a int) should give an error but I don't know if the
ALIAS example above should (or even if it does, I don't have a pg
installation here to try it).

/Dennis

Hello

2008/8/31 Hannu Krosing <hannu@2ndquadrant.com>:

It seems that we allow several function arguments to have same
name (or is it label :)

hannu=# create or replace function ff(a int, a int) returns int language
plpgsql as $$begin return $1+$2; end;$$;
CREATE FUNCTION
hannu=# select ff(1,1);
ff
----
2
(1 row)

hannu=# select ff(1,2);
ff
----
3
(1 row)

hannu=# create or replace function ffa(a int, a int) returns int
language plpgsql as $$begin return a + a; end;$$;
CREATE FUNCTION
hannu=# select ffa(1,2);
ffa
-----
2
(1 row)

Is this defined by some standard or just an oversight ?

what is problem? You have two diferent functions. I don't see anything wrong.

Pavel

Pavel Stehule wrote:

Hello

2008/8/31 Hannu Krosing <hannu@2ndquadrant.com>:

It seems that we allow several function arguments to have same
name (or is it label :)

hannu=# create or replace function ff(a int, a int) returns int language
plpgsql as $$begin return $1+$2; end;$$;
CREATE FUNCTION
hannu=# select ff(1,1);
ff
----
2
(1 row)

hannu=# select ff(1,2);
ff
----
3
(1 row)

hannu=# create or replace function ffa(a int, a int) returns int
language plpgsql as $$begin return a + a; end;$$;
CREATE FUNCTION
hannu=# select ffa(1,2);
ffa
-----
2
(1 row)

Is this defined by some standard or just an oversight ?

what is problem? You have two diferent functions. I don't see anything wrong.

Take a look at the second function again. It's certainly not behaviour
that I would expect :-) (I would expect a syntax error)

//Magnus

2008/9/1 Magnus Hagander <magnus@hagander.net>:

Pavel Stehule wrote:

Hello

2008/8/31 Hannu Krosing <hannu@2ndquadrant.com>:

It seems that we allow several function arguments to have same
name (or is it label :)

hannu=# create or replace function ff(a int, a int) returns int language
plpgsql as $$begin return $1+$2; end;$$;
CREATE FUNCTION
hannu=# select ff(1,1);
ff
----
2
(1 row)

hannu=# select ff(1,2);
ff
----
3
(1 row)

hannu=# create or replace function ffa(a int, a int) returns int
language plpgsql as $$begin return a + a; end;$$;
CREATE FUNCTION
hannu=# select ffa(1,2);
ffa
-----
2
(1 row)

Is this defined by some standard or just an oversight ?

what is problem? You have two diferent functions. I don't see anything wrong.

Take a look at the second function again. It's certainly not behaviour
that I would expect :-) (I would expect a syntax error)

I see it now - it's really bug

Pavel

On Mon, 2008-09-01 at 11:15 +0200, Pavel Stehule wrote:

2008/9/1 Magnus Hagander <magnus@hagander.net>:

Pavel Stehule wrote:

Hello

2008/8/31 Hannu Krosing <hannu@2ndquadrant.com>:

hannu=# create or replace function ffa(a int, a int) returns int
language plpgsql as $$begin return a + a; end;$$;
CREATE FUNCTION
hannu=# select ffa(1,2);
ffa
-----
2
(1 row)

Is this defined by some standard or just an oversight ?

what is problem? You have two diferent functions. I don't see anything wrong.

Take a look at the second function again. It's certainly not behaviour
that I would expect :-) (I would expect a syntax error)

I see it now - it's really bug

There are a few places, where repeating labels are allowed, for example
select can produce such record

hannu=# select 1 as a, 2 as a;
a | a
---+---
1 | 2
(1 row)

But it is not allowed in TYPE or table definitions

hannu=# create type aa as (a int, a int);
ERROR: column "a" specified more than once

hannu=# create table aa (a int, a int);
ERROR: column "a" specified more than once

It probably is also not allowed in function/procedure argument list, but
I was not sure that any standard would not require it.

So, should this be fixed at calling / SQL side (by not allowing
repeating argument names) or at pl side for each pl separately ?

--------------
Hannu

Hannu Krosing <hannu@2ndQuadrant.com> writes:

So, should this be fixed at calling / SQL side (by not allowing
repeating argument names) or at pl side for each pl separately ?

I'm for fixing it just once, ie, in CREATE FUNCTION. I can't imagine
any scenario where it's a good idea to have duplicate function parameter
names.

However, since this is a behavioral change that could break code that
works now, I think it should be a HEAD-only change; no backpatch.

regards, tom lane

2008/9/1 Tom Lane <tgl@sss.pgh.pa.us>:

Hannu Krosing <hannu@2ndQuadrant.com> writes:

So, should this be fixed at calling / SQL side (by not allowing
repeating argument names) or at pl side for each pl separately ?

I'm for fixing it just once, ie, in CREATE FUNCTION. I can't imagine
any scenario where it's a good idea to have duplicate function parameter
names.

However, since this is a behavioral change that could break code that
works now, I think it should be a HEAD-only change; no backpatch.

I agree - it's could break only 100% wrong code, but it could problems
in minor update.

Could you backpach only warning?

regards
Pavel

"Pavel Stehule" <pavel.stehule@gmail.com> writes:

2008/9/1 Tom Lane <tgl@sss.pgh.pa.us>:

However, since this is a behavioral change that could break code that
works now, I think it should be a HEAD-only change; no backpatch.

I agree - it's could break only 100% wrong code, but it could problems
in minor update.

Could you backpach only warning?

I'm not for that. I dislike back-patching changes that are not the same
as what goes into CVS HEAD, because that means those changes will go out
in minor releases of stable branches without any detectable amount of
developer testing.

If we thought this was a change that really deserved incremental
warnings, then the right thing would be to make it a warning in 8.4 and
an error in some later release. And maybe even make a GUC variable to
control that behavior. But I don't think it's worth that.

An error starting in 8.4 seems entirely sufficient from where I sit.

BTW, there are actually two separate issues here: input parameters and
output parameters. After brief thought it seems like we should enforce
uniqueness of non-omitted parameter names for IN parameters (including
INOUT), and separately enforce uniqueness of non-omitted parameter names
for OUT parameters (including INOUT).

regards, tom lane

2008/9/2 Tom Lane <tgl@sss.pgh.pa.us>:

"Pavel Stehule" <pavel.stehule@gmail.com> writes:

2008/9/1 Tom Lane <tgl@sss.pgh.pa.us>:

However, since this is a behavioral change that could break code that
works now, I think it should be a HEAD-only change; no backpatch.

I agree - it's could break only 100% wrong code, but it could problems
in minor update.

Could you backpach only warning?

I'm not for that. I dislike back-patching changes that are not the same
as what goes into CVS HEAD, because that means those changes will go out
in minor releases of stable branches without any detectable amount of
developer testing.

If we thought this was a change that really deserved incremental
warnings, then the right thing would be to make it a warning in 8.4 and
an error in some later release. And maybe even make a GUC variable to
control that behavior. But I don't think it's worth that.

+1

An error starting in 8.4 seems entirely sufficient from where I sit.

BTW, there are actually two separate issues here: input parameters and
output parameters. After brief thought it seems like we should enforce
uniqueness of non-omitted parameter names for IN parameters (including
INOUT), and separately enforce uniqueness of non-omitted parameter names
for OUT parameters (including INOUT).

It's well thought, but I afraid so this can hide some bug, and it's
little bit dangerous.

I thing, so we can simply duplicate values in result then allowing
duplicate params in function.

postgres=# create function foo(out a int, out b int) returns record as
$$ select 1,2 $$ language sql;
CREATE FUNCTION
postgres=# select a, a, b from foo();
a | a | b
---+---+---
1 | 1 | 2
(1 row)

Duplicate arguments are more like copy-paste bug then good style. We
check it in column definition too:

postgres=# create function foo1() returns record as $$ select 1,1,2 $$
language sql;
CREATE FUNCTION
postgres=# select * from foo1() as (c int,c int,c int);
ERROR: column name "c" specified more than once

Pavel

"Pavel Stehule" <pavel.stehule@gmail.com> writes:

2008/9/2 Tom Lane <tgl@sss.pgh.pa.us>:

BTW, there are actually two separate issues here: input parameters and
output parameters. After brief thought it seems like we should enforce
uniqueness of non-omitted parameter names for IN parameters (including
INOUT), and separately enforce uniqueness of non-omitted parameter names
for OUT parameters (including INOUT).

It's well thought, but I afraid so this can hide some bug, and it's
little bit dangerous.

I thing, so we can simply duplicate values in result then allowing
duplicate params in function.

Um ... what? I'm not sure what behavior you're proposing here.

regards, tom lane

2008/9/2 Tom Lane <tgl@sss.pgh.pa.us>:

"Pavel Stehule" <pavel.stehule@gmail.com> writes:

2008/9/2 Tom Lane <tgl@sss.pgh.pa.us>:

BTW, there are actually two separate issues here: input parameters and
output parameters. After brief thought it seems like we should enforce
uniqueness of non-omitted parameter names for IN parameters (including
INOUT), and separately enforce uniqueness of non-omitted parameter names
for OUT parameters (including INOUT).

It's well thought, but I afraid so this can hide some bug, and it's
little bit dangerous.

I thing, so we can simply duplicate values in result then allowing
duplicate params in function.

Um ... what? I'm not sure what behavior you're proposing here.

regards, tom lane

I am sorry - I really have to learn english. Simply I don't thing, so
duplicit OUT parameters is good idea, but I am haven't strong
objections - some programmer's bugs are visible in this case.

regards
Pavel