Thread: PATCH: make plpgsql IN args mutable (v1)
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.
-Steve
Attachment
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 > This function would have failed to compile previously. > > -Steve > > > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > >
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 > cheers > > andrew > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
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$ DECLAREz INT := x; BEGINz := x % y;RETURN z; END; $f$ Becomes this: CREATE PROCEDURE mod ( x int, y int ) RETURNS int LANGUAGE plpgsql AS $f$ BEGINx := 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 $$ beginif pobjectid is null then pobjectid := newid()end ifif psomefkobjectid is null then select objectid into psomefkobjectidfrom somefktable where whatever;end if-- create the objectreturn 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