RFD: Don't force plpgsql IN parameters to constant
Is there a reason we force plpgsql IN parameters to constant? The
reason I ask is because having them mutable would go a long way in
easing a port from Informix's SPL. For better or worse, we have a fair
amount of code in SPL that does something like:
-- pObjectId is an IN parameter
IF pObjectId IS NULL THEN
pObjectId := newid();
END IF;
I understand it may be better to use a different technique here, but
we have a substantial amount of SPL (40k lines) and if we could make
the IN parameters mutable, it would make my day.
Looking at the history of the code, it looks like this has been the
way it has been since the beginning. Tom added a comment in 1995
asking why we force the IN parameters to constant, but the "why?" part
of the comment was removed in a later change to support OUT and INOUT
parameters.
I've attached a patch that would change this behavior. Also, the
test2(int) function below works with the patch, but would fail to
compile without. I also checked to make sure the parameter wasn't
passed by reference and it is not. The test at the bottom returns 't'
meaning test2(int) did not change the a variable in test1().
CREATE OR REPLACE FUNCTION test1() RETURNS INT AS $$
DECLARE
a INT;
BEGIN
a := 1;
PERFORM test2(a);
RETURN a;
END
$$ LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION test2(a INT) RETURNS VOID AS $$
BEGIN
a := 2;
END
$$ LANGUAGE plpgsql;
SELECT test1() = 1;
If this change would be acceptable, I'll proceed in finishing the
patch by updating docs and adding regression tests.
-Steve
Attachments:
notconstant.patchapplication/octet-stream; name=notconstant.patch; x-unix-mode=0644Download
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 028013f..1bec949 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -424,10 +424,6 @@ do_compile(FunctionCallInfo fcinfo,
if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
{
argitemtype = PLPGSQL_NSTYPE_VAR;
- /* input argument vars are forced to be CONSTANT */
- if (argmode == PROARGMODE_IN ||
- argmode == PROARGMODE_VARIADIC)
- ((PLpgSQL_var *) argvariable)->isconst = true;
}
else
{
Steve Prentice wrote:
Is there a reason we force plpgsql IN parameters to constant? The
reason I ask is because having them mutable would go a long way in
easing a port from Informix's SPL. For better or worse, we have a fair
amount of code in SPL that does something like:-- pObjectId is an IN parameter
IF pObjectId IS NULL THEN
pObjectId := newid();
END IF;I understand it may be better to use a different technique here, but
we have a substantial amount of SPL (40k lines) and if we could make
the IN parameters mutable, it would make my day.
First reaction is that it would mean we could never pass them by
reference. I know PLPerl uses in effect pass by copy, but what does
PLPgsql do?
cheers
andrew
On Wed, Jul 29, 2009 at 7:55 PM, Steve Prentice<prentice@cisco.com> wrote:
Is there a reason we force plpgsql IN parameters to constant? The reason I
ask is because having them mutable would go a long way in easing a port from
Informix's SPL. For better or worse, we have a fair amount of code in SPL
that does something like:-- pObjectId is an IN parameter
IF pObjectId IS NULL THEN
pObjectId := newid();
END IF;I understand it may be better to use a different technique here, but we have
a substantial amount of SPL (40k lines) and if we could make the IN
parameters mutable, it would make my day.Looking at the history of the code, it looks like this has been the way it
has been since the beginning. Tom added a comment in 1995 asking why we
force the IN parameters to constant, but the "why?" part of the comment was
removed in a later change to support OUT and INOUT parameters.I've attached a patch that would change this behavior. Also, the test2(int)
function below works with the patch, but would fail to compile without. I
also checked to make sure the parameter wasn't passed by reference and it is
not. The test at the bottom returns 't' meaning test2(int) did not change
the a variable in test1().CREATE OR REPLACE FUNCTION test1() RETURNS INT AS $$
DECLARE
a INT;
BEGIN
a := 1;
PERFORM test2(a);
RETURN a;
END
$$ LANGUAGE plpgsql;CREATE OR REPLACE FUNCTION test2(a INT) RETURNS VOID AS $$
BEGIN
a := 2;
END
$$ LANGUAGE plpgsql;SELECT test1() = 1;
If this change would be acceptable, I'll proceed in finishing the patch by
updating docs and adding regression tests.
Wow. I can imagine about a thousand ways that this could break
existing applications. I would not be prepared to bet a dollar that
anything I've written would survive the impact unscathed.
I have a feeling someone else is going to shoot you out of the water
completely, but all I'll say is it would definitely need to be
OPTIONAL.
...Robert
On Jul 29, 2009, at 5:26 PM, Robert Haas wrote:
Wow. I can imagine about a thousand ways that this could break
existing applications. I would not be prepared to bet a dollar that
anything I've written would survive the impact unscathed.I have a feeling someone else is going to shoot you out of the water
completely, but all I'll say is it would definitely need to be
OPTIONAL.
I guess I don't get how it would break existing applications. All of
the regression tests pass. The parameters are passed as a copy, so it
can't modify your variable that you pass in. Perhaps I'm missing
something--can you elaborate on how this would break existing
applications?
-Steve
On Jul 29, 2009, at 5:23 PM, Andrew Dunstan wrote:
First reaction is that it would mean we could never pass them by
reference. I know PLPerl uses in effect pass by copy, but what does
PLPgsql do?
Isn't this effectively what we accomplish with an IN/OUT parameter?
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jul 29, 2009 at 7:55 PM, Steve Prentice<prentice@cisco.com> wrote:
Is there a reason we force plpgsql IN parameters to constant?
Wow. I can imagine about a thousand ways that this could break
existing applications. I would not be prepared to bet a dollar that
anything I've written would survive the impact unscathed.
Really? AFAICS the only impact is that if you tried to assign to a
parameter variable, it would do it instead of throwing a compile-time
error. It's hard to imagine that breaking any code that works now.
It's true that doing this might foreclose some implementation shortcuts
in future versions of plpgsql, but it's not going to be an issue in
anything that works remotely like the way that plpgsql variables work
now.
Also, if we think it's a good idea, why are we only forcing CONST
for scalar arguments and not composite arguments? (The fact that
plpgsql doesn't even have a way to mark composites as CONST might
be the reason ;-), but it's surely not a real good reason.) And
special trigger arguments like tg_name aren't marked CONST either,
for even less reason.
Now having said all that, I'm not really in favor of Steve's
proposal --- it seems like it mostly would be encouraging dubious
programming practices. But it's hard to say that the arguments
against are more than theoretical/aesthetic ones.
Does anyone happen to know how it works in Oracle's PL/SQL?
I think that following their lead is usually the argument-settler
when it comes to plpgsql behavior.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
First reaction is that it would mean we could never pass them by
reference. I know PLPerl uses in effect pass by copy, but what does
PLPgsql do?
It's not really an issue, because plpgsql keeps track of whether
the current value of the variable "belongs" to it or not. Look
at the "freeval" manipulations in pl_exec.c.
regards, tom lane
On Wed, Jul 29, 2009 at 9:05 PM, Steve Prentice<prentice@cisco.com> wrote:
On Jul 29, 2009, at 5:26 PM, Robert Haas wrote:
Wow. I can imagine about a thousand ways that this could break
existing applications. I would not be prepared to bet a dollar that
anything I've written would survive the impact unscathed.I have a feeling someone else is going to shoot you out of the water
completely, but all I'll say is it would definitely need to be
OPTIONAL.I guess I don't get how it would break existing applications. All of the
regression tests pass. The parameters are passed as a copy, so it can't
modify your variable that you pass in. Perhaps I'm missing something--can
you elaborate on how this would break existing applications?
Well, in my imagination, if you were proposing something completely
different, it would... sorry for the noise.
...Robert
On Wed, Jul 29, 2009 at 9:08 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jul 29, 2009 at 7:55 PM, Steve Prentice<prentice@cisco.com> wrote:
Is there a reason we force plpgsql IN parameters to constant?
Wow. I can imagine about a thousand ways that this could break
existing applications. I would not be prepared to bet a dollar that
anything I've written would survive the impact unscathed.Really? AFAICS the only impact is that if you tried to assign to a
parameter variable, it would do it instead of throwing a compile-time
error. It's hard to imagine that breaking any code that works now.
Yeah, I'm wrong. Sorry.
It's true that doing this might foreclose some implementation shortcuts
in future versions of plpgsql, but it's not going to be an issue in
anything that works remotely like the way that plpgsql variables work
now.Also, if we think it's a good idea, why are we only forcing CONST
for scalar arguments and not composite arguments? (The fact that
plpgsql doesn't even have a way to mark composites as CONST might
be the reason ;-), but it's surely not a real good reason.) And
special trigger arguments like tg_name aren't marked CONST either,
for even less reason.Now having said all that, I'm not really in favor of Steve's
proposal --- it seems like it mostly would be encouraging dubious
programming practices. But it's hard to say that the arguments
against are more than theoretical/aesthetic ones.Does anyone happen to know how it works in Oracle's PL/SQL?
I think that following their lead is usually the argument-settler
when it comes to plpgsql behavior.
Hmm, well if I understand this correctly (now), it's similar to allowing:
void
test2(int a)
{
a = 2;
return;
}
...which is a fairly common programming practice that I don't think is
particularly considered bad style, or at least certainly not by
everyone.
...Robert
On Jul 29, 2009, at 4:55 PM, Steve Prentice wrote:
Tom added a comment in 1995
For the record, I meant 2005.
-Steve
Robert Haas <robertmhaas@gmail.com> writes:
Hmm, well if I understand this correctly (now), it's similar to allowing:
void
test2(int a)
{
a = 2;
return;
}
...which is a fairly common programming practice that I don't think is
particularly considered bad style, or at least certainly not by
everyone.
Right, you could only change the local copy of the parameter value.
It's certainly not demonstrably horrible ...
regards, tom lane
Steve Prentice <prentice@cisco.com> writes:
On Jul 29, 2009, at 4:55 PM, Steve Prentice wrote:
Tom added a comment in 1995
For the record, I meant 2005.
I was intending to say something like "I've been around this project
a long time, but not THAT long" ...
regards, tom lane
Tom Lane wrote:
Is there a reason we force plpgsql IN parameters to constant?
Now having said all that, I'm not really in favor of Steve's
proposal --- it seems like it mostly would be encouraging dubious
programming practices. But it's hard to say that the arguments
against are more than theoretical/aesthetic ones.Does anyone happen to know how it works in Oracle's PL/SQL?
I think that following their lead is usually the argument-settler
when it comes to plpgsql behavior.
Oracle 10.2.0.4:
CREATE PROCEDURE test2(a NUMBER) AUTHID CURRENT_USER AS
BEGIN
a := 2;
END;
/
PLS-00363: expression 'A' cannot be used as an assignment target
So it does not work in Oracle.
The proposed feature would have come handy for me once or twice,
but maybe that is more a sign of my sloppy coding habits than
anything else ...
Still, +1 from me for the proposal.
In my experience, restrictive languages have never kept people
from writing bad and confusing code.
What about introducing a keyword CONSTANT in the parameter list
to force the old behaviour?
(This would remove us further from Oracle though.)
Yours,
Laurenz Albe
Tom Lane wrote:
Steve Prentice <prentice@cisco.com> writes:
On Jul 29, 2009, at 4:55 PM, Steve Prentice wrote:
Tom added a comment in 1995
For the record, I meant 2005.
I was intending to say something like "I've been around this project
a long time, but not THAT long" ...
I was looking at a 1997 pgsql-hackers mailbox yesterday and I noticed
that "tgl" back then was Thomas Lockhart.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.