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

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(at)cisco(dot)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(at)gmail(dot)com> writes:
> On Wed, Jul 29, 2009 at 7:55 PM, Steve Prentice<prentice(at)cisco(dot)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(at)dunslane(dot)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(at)cisco(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Wed, Jul 29, 2009 at 7:55 PM, Steve Prentice<prentice(at)cisco(dot)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(at)gmail(dot)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(at)cisco(dot)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(at)cisco(dot)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.