PATCH: make plpgsql IN args mutable (v1)

On Thu, Jul 30, 2009 at 4:37 PM, Steve Prentice<prentice@cisco.com> wrote:

Since I didn't get completely shot out of the water and a couple people
seemed to think it was helpful, I'm submitting this patch for consideration
in the next commitfest.

This patch changes plpgsql IN parameters so they are mutable. Previously,
they were being forced constant. This patch modifies the plpgsql.sql
regression test and corresponding .out file. The regression test also makes
sure the passed in parameter does not get changed in the calling function.

I decided not to update the docs for this change because the docs don't
currently indicate that an IN parameter is constant and I didn't want to
encourage it because it isn't universally considered good programming
practice to assign to an IN parameter. If others think we need a doc change
for this, I'll update the patch.

The following function will compile with this patch:

 create or replace function param_assign_test(a int, val int) returns void
as $$
 begin
   a := val;
 end
 $$ language plpgsql;

This function would have failed to compile previously.

We're in the middle of a CommitFest right now for which the deadline
for submissions was 2009-07-14. Please go to
https://commitfest.postgresql.org/action/commitfest_view/open and add
your patch there.

Thanks,

...Robert

On Thu, Jul 30, 2009 at 05:06:17PM -0400, Robert Haas wrote:

On Thu, Jul 30, 2009 at 4:37 PM, Steve Prentice<prentice@cisco.com> wrote:

Since I didn't get completely shot out of the water and a couple people
seemed to think it was helpful, I'm submitting this patch for consideration
in the next commitfest.

This patch changes plpgsql IN parameters so they are mutable. Previously,
they were being forced constant. This patch modifies the plpgsql.sql
regression test and corresponding .out file. The regression test also makes
sure the passed in parameter does not get changed in the calling function.

Wouldn't INOUT parameters cover this case?

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

On Thu, 2009-07-30 at 17:40 -0700, David Fetter wrote:

This patch changes plpgsql IN parameters so they are mutable. Previously,
they were being forced constant. This patch modifies the plpgsql.sql
regression test and corresponding .out file. The regression test also makes
sure the passed in parameter does not get changed in the calling function.

Wouldn't INOUT parameters cover this case?

That was my first, thought, but I don't think it solves his concern. The
out parameters are returned as part of a record, but he actually wants
to mutate the value passed in.

If mutable IN parameters were allowed, I don't even think it could be
allowable to call them from the SQL level, you could only from another
function.

For instance, what would it mean if you did something like:

SELECT foo(a) FROM mytable;

Where foo() mutated it's IN argument? Would that really be an UPDATE?

Regards,
Jeff Davis

Jeff Davis wrote:

If mutable IN parameters were allowed, I don't even think it could be
allowable to call them from the SQL level, you could only from another
function.

For instance, what would it mean if you did something like:

SELECT foo(a) FROM mytable;

Where foo() mutated it's IN argument? Would that really be an UPDATE?

No, surely the mutated value will only be visible within the scope of
the function, i.e. it will be a purely local copy that gets altered.

cheers

andrew

On Thu, 2009-07-30 at 21:45 -0400, Andrew Dunstan wrote:

For instance, what would it mean if y
SELECT foo(a) FROM mytable;

Where foo() mutated it's IN argument? Would that really be an UPDATE?

No, surely the mutated value will only be visible within the scope of
the function, i.e. it will be a purely local copy that gets altered.

Oh, I misunderstood the example here:

http://archives.postgresql.org/pgsql-hackers/2009-07/msg01931.php

I thought he was saying that the PERFORM in test1() _should_ have
mutated "a", when in fact, he was trying to demonstrate that it does not
(with his patch or without).

Regards,
Jeff Davis

2009/7/30 Steve Prentice <prentice@cisco.com>:

Since I didn't get completely shot out of the water and a couple people
seemed to think it was helpful, I'm submitting this patch for consideration
in the next commitfest.

This patch changes plpgsql IN parameters so they are mutable. Previously,
they were being forced constant. This patch modifies the plpgsql.sql
regression test and corresponding .out file. The regression test also makes
sure the passed in parameter does not get changed in the calling function.

I decided not to update the docs for this change because the docs don't
currently indicate that an IN parameter is constant and I didn't want to
encourage it because it isn't universally considered good programming
practice to assign to an IN parameter. If others think we need a doc change
for this, I'll update the patch.

The following function will compile with this patch:

 create or replace function param_assign_test(a int, val int) returns void
as $$
 begin
   a := val;
 end
 $$ language plpgsql;

This behave is in conflict with PL/SQL, what should do some problems.
I thing, so I understand well, why this behave is in PL/SQL. It hasn't
sense in plpgsql, because OUT and INOUT params has little bit
different syntax (calling) and nobody will do similar bugs (perhaps).
What is interesting - this behave is in conformity with SQL/PSM, where
parameters are mutable too.

I am for it. PL/pgSQL doesn't promise compatibility with PL/SQL and
this change should to help some beginners (and this limit is
artificial and unnecessary).

Regards
Pavel Stehule

At 2009-07-30 13:37:16 -0700, prentice@cisco.com wrote:

This patch changes plpgsql IN parameters so they are mutable.

Makes sense, applies fine, works fine.

-- ams

Abhijit Menon-Sen wrote:

At 2009-07-30 13:37:16 -0700, prentice@cisco.com wrote:

This patch changes plpgsql IN parameters so they are mutable.

Makes sense, applies fine, works fine.

How does this compare with PLSQL? I know in Ada an IN argument is in
effect a constant. I understand the utility, because I occasionally
knock against this restriction, but if it's incompatible with PLSQL I
think we should think about it more carefully.

cheers

andrew

At 2009-09-16 08:37:40 -0400, andrew@dunslane.net wrote:

How does this compare with PLSQL?

I don't remember anything of PL/SQL myself, but Pavel Stehule had this
to say in response to the original post:

This behave is in conflict with PL/SQL, what should do some problems.
I thing, so I understand well, why this behave is in PL/SQL. It hasn't
sense in plpgsql, because OUT and INOUT params has little bit
different syntax (calling) and nobody will do similar bugs (perhaps).
What is interesting - this behave is in conformity with SQL/PSM, where
parameters are mutable too.

I am for it. PL/pgSQL doesn't promise compatibility with PL/SQL and
this change should to help some beginners (and this limit is
artificial and unnecessary).

Given the existing OUT/INOUT syntax difference as noted, I don't think
the patch represents a significant problem.

-- ams

On Sep 16, 2009, at 8:37 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

Abhijit Menon-Sen wrote:

At 2009-07-30 13:37:16 -0700, prentice@cisco.com wrote:

This patch changes plpgsql IN parameters so they are mutable.

Makes sense, applies fine, works fine.

How does this compare with PLSQL? I know in Ada an IN argument is in
effect a constant. I understand the utility, because I occasionally
knock against this restriction, but if it's incompatible with PLSQL
I think we should think about it more carefully.

At worst it's an upward-compatible extension, or am I wrong? If it's
useful, which I think it is, what's the harm?

...Robert

Abhijit Menon-Sen wrote:

At 2009-09-16 08:37:40 -0400, andrew@dunslane.net wrote:

How does this compare with PLSQL?

I don't remember anything of PL/SQL myself, but Pavel Stehule had this
to say in response to the original post:

This behave is in conflict with PL/SQL, what should do some problems.
I thing, so I understand well, why this behave is in PL/SQL. It hasn't
sense in plpgsql, because OUT and INOUT params has little bit
different syntax (calling) and nobody will do similar bugs (perhaps).
What is interesting - this behave is in conformity with SQL/PSM, where
parameters are mutable too.

I am for it. PL/pgSQL doesn't promise compatibility with PL/SQL and
this change should to help some beginners (and this limit is
artificial and unnecessary).

Given the existing OUT/INOUT syntax difference as noted, I don't think
the patch represents a significant problem.

I'm not terribly impressed by either of Pavel's arguments. SQL/PSM is
irrelevant, and the existence of one inconsistency doesn't seems to me
to be a good rationale to create another. If there were a major increase
in utility I would be more willing, but at best this overcomes a minor
inconvenience, that is easily worked around.

It probably won't cause any problem with code being migrated from PLSQL,
but it will affect code going the other way. The question is: do we care
about that? I'm prepared to be persuaded that we shouldn't care, but I'm
not quite there yet.

cheers

andrew

Andrew Dunstan wrote:

It probably won't cause any problem with code being migrated from
PLSQL, but it will affect code going the other way. The question is:
do we care about that? I'm prepared to be persuaded that we
shouldn't care, but I'm not quite there yet.

Anybody trying to port code from PL/pgSQL to PL/SQL is going to be a lot
more inconvenienced by the OUT parameter stuff.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Andrew Dunstan <andrew@dunslane.net> writes:

It probably won't cause any problem with code being migrated from PLSQL,
but it will affect code going the other way. The question is: do we care
about that? I'm prepared to be persuaded that we shouldn't care, but I'm
not quite there yet.

IIRC the original complaint was from someone trying to migrate code
from T/SQL or some other not-quite-PLSQL language. Like you, I'm on
the fence about whether to accept this patch, but it does have some
in-migration rationale.

regards, tom lane

I'm not terribly impressed by either of Pavel's arguments. SQL/PSM is
irrelevant, and the existence of one inconsistency doesn't seems to me to be
a good rationale to create another. If there were a major increase in
utility I would be more willing, but at best this overcomes a minor
inconvenience, that is easily worked around.

It probably won't cause any problem with code being migrated from PLSQL, but
it will affect code going the other way. The question is: do we care about
that? I'm prepared to be persuaded that we shouldn't care, but I'm not quite
there yet.

In this case I have not strong opinion. Similarity with SQL/PSM isn't
my main argument. I see, so immutable IN arguments are typical problem
for beginners. Internally arguments are not immutable - so mutable
arguments should help to people who start with PostgreSQL.

But I accept, so this increase difference between plpgsql and pl/sql
what is wrong too.

Regards
Pavel

On Sep 16, 2009, at 6:03 AM, Andrew Dunstan wrote:

Abhijit Menon-Sen wrote:

At 2009-09-16 08:37:40 -0400, andrew@dunslane.net wrote:

How does this compare with PLSQL?

I don't remember anything of PL/SQL myself, but Pavel Stehule had
this
to say in response to the original post:

This behave is in conflict with PL/SQL, what should do some
problems.
I thing, so I understand well, why this behave is in PL/SQL. It
hasn't
sense in plpgsql, because OUT and INOUT params has little bit
different syntax (calling) and nobody will do similar bugs
(perhaps).
What is interesting - this behave is in conformity with SQL/PSM,
where
parameters are mutable too.

I am for it. PL/pgSQL doesn't promise compatibility with PL/SQL and
this change should to help some beginners (and this limit is
artificial and unnecessary).

Given the existing OUT/INOUT syntax difference as noted, I don't
think
the patch represents a significant problem.

I'm not terribly impressed by either of Pavel's arguments. SQL/PSM
is irrelevant, and the existence of one inconsistency doesn't seems
to me to be a good rationale to create another. If there were a
major increase in utility I would be more willing, but at best this
overcomes a minor inconvenience, that is easily worked around.

It probably won't cause any problem with code being migrated from
PLSQL, but it will affect code going the other way. The question is:
do we care about that? I'm prepared to be persuaded that we
shouldn't care, but I'm not quite there yet.

My motivation for submitting the patch was that it makes porting a
huge collection of Informix SPL stored procedures easier. There are so
many differences between plpgsql and SPL that you would think this
wasn't that big of a deal, however, most of the other issues are
easily taken care of with a simple sed script or something slightly
more advanced (e.g. dealing with the declare/define block
differences). This is one of the few compatibility issues where you
really need to review and change lots of code by hand.

The patch doesn't break existing code and doesn't make it any harder
to port code from PL/SQL and on the flip side, this patch with the
named/mixed notation patch from Pavel makes porting from Informix's
SPL much easier.

Thanks for everyone's consideration.

-Steve

On Wed, Sep 16, 2009 at 8:59 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Sep 16, 2009, at 8:37 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

Abhijit Menon-Sen wrote:

At 2009-07-30 13:37:16 -0700, prentice@cisco.com wrote:

This patch changes plpgsql IN parameters so they are mutable.

Makes sense, applies fine, works fine.

How does this compare with PLSQL? I know in Ada an IN argument is in
effect a constant. I understand the utility, because I occasionally knock
against this restriction, but if it's incompatible with PLSQL I think we
should think about it more carefully.

At worst it's an upward-compatible extension, or am I wrong?  If it's
useful, which I think it is, what's the harm?

are we guarding against cases like:
select _foo, adjust_foo(_foo) from bar; -- adjust_foo is inout

??
merlin

On Sep 16, 2009, at 8:49 AM, Merlin Moncure wrote:

On Wed, Sep 16, 2009 at 8:59 AM, Robert Haas <robertmhaas@gmail.com>
wrote:

At worst it's an upward-compatible extension, or am I wrong? If it's
useful, which I think it is, what's the harm?

are we guarding against cases like:
select _foo, adjust_foo(_foo) from bar; -- adjust_foo is inout

Two things:

1) the patch only affects IN parameters,

2) the parameter is a local copy and doesn't affect parameters/
variables outside of its scope.

IIRC the original complaint was from someone trying to migrate code
from T/SQL or some other not-quite-PLSQL language. Like you, I'm on
the fence about whether to accept this patch, but it does have some
in-migration rationale.

As someone who writes a lot of plpgsql, I'm in favor of the patch.

1. Compatibility with PL/SQL, especially in the extra features
direction, has never been a tremendous priority for us before;

2. We don't particularly care if native plpgsql procedures can be
back-ported to PLSQL, and if we did there are much greater compatibility
issues than this one;

3. This patch eliminates a common plpgsql beginner error and saves all
of us heavy plpgsql users some typing, especially when the use of a
mutable variable means that we can eliminate the DECLARE section
entirely, as in:

This:

CREATE PROCEDURE mod ( x int, y int )
RETURNS int LANGUAGE plpgsql
AS $f$
DECLARE
z INT := x;
BEGIN
z := x % y;
RETURN z;
END; $f$

Becomes this:

CREATE PROCEDURE mod ( x int, y int )
RETURNS int LANGUAGE plpgsql
AS $f$
BEGIN
x := x % y;
RETURN x;
END; $f$

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

On Sep 16, 2009, at 13:40 , Josh Berkus wrote:

3. This patch eliminates a common plpgsql beginner error and saves all
of us heavy plpgsql users some typing, especially when the use of a
mutable variable means that we can eliminate the DECLARE section
entirely, as in:

This:

CREATE PROCEDURE mod ( x int, y int )
RETURNS int LANGUAGE plpgsql
AS $f$
DECLARE
z INT := x;
BEGIN
z := x % y;
RETURN z;
END; $f$

This is also currently valid:

CREATE FUNCTION mod (x int, y int)
RETURNS int LANGUAGE plpgsql
AS $f$
DECLARE
z INT := x % y;
BEGIN
RETURN z;
END; $f$

As is this:

CREATE FUNCTION mod (x int, y int)
RETURNS int LANGUAGE plpgsql
AS $f$
BEGIN
RETURN (x % y);
END; $f$

Michael Glaesemann
grzm seespotcode net

Michael,

This is also currently valid:

CREATE FUNCTION mod (x int, y int)
RETURNS int LANGUAGE plpgsql
AS $f$
DECLARE
z INT := x % y;
BEGIN
RETURN z;
END; $f$

As is this:

CREATE FUNCTION mod (x int, y int)
RETURNS int LANGUAGE plpgsql
AS $f$
BEGIN
RETURN (x % y);
END; $f$

Certainly. I was doing that to have a simple example; obviously you
wouldn't write a mod funciton, and you wouldn't do it in plpgsql. There
are other case where the lack of mutability in IN parameters causes you
to create a throwaway variable.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

On Sep 16, 2009, at 15:17 , Josh Berkus wrote:

Michael,

This is also currently valid:

CREATE FUNCTION mod (x int, y int)
RETURNS int LANGUAGE plpgsql
AS $f$
DECLARE
z INT := x % y;
BEGIN
RETURN z;
END; $f$

As is this:

CREATE FUNCTION mod (x int, y int)
RETURNS int LANGUAGE plpgsql
AS $f$
BEGIN
RETURN (x % y);
END; $f$

Certainly. I was doing that to have a simple example; obviously you
wouldn't write a mod funciton, and you wouldn't do it in plpgsql.
There
are other case where the lack of mutability in IN parameters causes
you
to create a throwaway variable.

Have an example at hand? I'd argue that in a case of a function of
more complexity from a code clarity standpoint you'd want to assign to
a new variable that describes what the new value reflects.

Michael Glaesemann
grzm seespotcode net

On Sep 16, 2009, at 12:44 PM, Michael Glaesemann wrote:

Certainly. I was doing that to have a simple example; obviously you
wouldn't write a mod funciton, and you wouldn't do it in plpgsql.
There
are other case where the lack of mutability in IN parameters causes
you
to create a throwaway variable.

Have an example at hand? I'd argue that in a case of a function of
more complexity from a code clarity standpoint you'd want to assign
to a new variable that describes what the new value reflects.

I can't say I disagree with you from a purist standpoint, but for
porting existing code sometimes it's more efficient to port what you
have without rewriting it. In some of the code I'm looking at porting,
this is a very simple example of a common pattern I'm seeing:

create function create_some_object(pobjectid uuid, psomefkobjectid
uuid) returns uuid as $$
begin
if pobjectid is null then
pobjectid := newid()
end if

if psomefkobjectid is null then
select objectid into psomefkobjectid from somefktable where whatever;
end if

-- create the object
return pobjectid
end;

-Steve

Michael,

Have an example at hand? I'd argue that in a case of a function of more
complexity from a code clarity standpoint you'd want to assign to a new
variable that describes what the new value reflects.

Depends on what programming language you're used to. For those of us
who do a lot of pass-by-reference in our non-database code, reusing the
IN variable is "natural". I know not being able to is a longstanding
annoyance for me.

And I really don't think it's the place of the PostgreSQL project to try
to force what some of us think is good PL coding style on people.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh Berkus wrote:

Michael,

Have an example at hand? I'd argue that in a case of a function of more
complexity from a code clarity standpoint you'd want to assign to a new
variable that describes what the new value reflects.

Depends on what programming language you're used to. For those of us
who do a lot of pass-by-reference in our non-database code, reusing the
IN variable is "natural". I know not being able to is a longstanding
annoyance for me.

It's the pass by reference case that would be dangerous, in fact. The
fact that in C all function parameters are passed by value (unlike, say,
FORTRAN) is what makes it safe to modify them inside the function.

Anyway, debates about such thigs tend to get a bit religious. getting
more practical, I'm slightly inclined to say Steve Prentice has made a
good enough case for doing this.

cheers

andrew

Josh Berkus <josh@agliodbs.com> writes:

3. This patch eliminates a common plpgsql beginner error

With respect, that argument is one hundred percent false. I can think
of maybe two complaints about the behavior that we've heard in the last
ten years.

The only real argument I've heard in favor of this is that it will
simplify importing not-too-well-written Informix code. That might be
sufficient reason, but let's not invent claims about it being a common
problem.

regards, tom lane

Steve Prentice <prentice@cisco.com> writes:

This patch changes plpgsql IN parameters so they are mutable.

I've applied this, since the consensus seemed to be in favor of it.

I decided not to update the docs for this change because the docs
don't currently indicate that an IN parameter is constant and I didn't
want to encourage it because it isn't universally considered good
programming practice to assign to an IN parameter. If others think we
need a doc change for this, I'll update the patch.

I agree, no need to say anything one way or the other in the plpgsql docs.
We'll want to mention it in the release notes of course.

regards, tom lane

Thank you!

-Steve

On Sep 19, 2009, at 6:55 PM, Tom Lane wrote:

Steve Prentice <prentice@cisco.com> writes:

This patch changes plpgsql IN parameters so they are mutable.

I've applied this, since the consensus seemed to be in favor of it.

I decided not to update the docs for this change because the docs
don't currently indicate that an IN parameter is constant and I didn't
want to encourage it because it isn't universally considered good
programming practice to assign to an IN parameter. If others think we
need a doc change for this, I'll update the patch.

I agree, no need to say anything one way or the other in the plpgsql
docs.
We'll want to mention it in the release notes of course.

regards, tom lane