Horton

Thread

Commits

  1. Tighten enforcement of variable CONSTANT markings in plpgsql.

  1. Missing can't-assign-to-constant checks in plpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-28T21:52:09Z

    I happened to notice that there are a couple of places in plpgsql
that will let you assign a new value to a variable that's marked
CONSTANT:

* We don't complain if an output parameter in a CALL statement
is constant.

* We don't complain if a refcursor variable is constant, even
though OPEN may assign a new value to it.

The attached quick-hack patch closes both of these oversights.

Perhaps the OPEN change is a little too aggressive, since if
you give the refcursor variable some non-null initial value,
OPEN won't change it; in that usage a CONSTANT marking could
be allowed.  But I really seriously doubt that anybody out
there is marking such variables as constants, so I thought
throwing the error at compile time was better than postponing
it to runtime so we could handle that.

Regardless of which way we handle that point, I'm inclined to
change this only in HEAD.  Probably people wouldn't thank us
for making the back branches more strict.

			regards, tom lane

PS: I didn't do it here, but I'm kind of tempted to pull out
all the cursor-related tests in plpgsql.sql and move them to
a new test file under src/pl/plpgsql/src/sql/.  They look
pretty self-contained, and I doubt they're worth keeping in
the core tests.

  2. Re: Missing can't-assign-to-constant checks in plpgsql

    Pavel Stehule <pavel.stehule@gmail.com> — 2022-04-28T22:11:09Z

    čt 28. 4. 2022 v 23:52 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:

> I happened to notice that there are a couple of places in plpgsql
> that will let you assign a new value to a variable that's marked
> CONSTANT:
>
> * We don't complain if an output parameter in a CALL statement
> is constant.
>
> * We don't complain if a refcursor variable is constant, even
> though OPEN may assign a new value to it.
>
> The attached quick-hack patch closes both of these oversights.
>
> Perhaps the OPEN change is a little too aggressive, since if
> you give the refcursor variable some non-null initial value,
> OPEN won't change it; in that usage a CONSTANT marking could
> be allowed.  But I really seriously doubt that anybody out
> there is marking such variables as constants, so I thought
> throwing the error at compile time was better than postponing
> it to runtime so we could handle that.
>
> Regardless of which way we handle that point, I'm inclined to
> change this only in HEAD.  Probably people wouldn't thank us
> for making the back branches more strict.
>

+1

I can implement these checks in plpgsql_check. So possible issues can be
detected and fixed on older versions by using plpgsql_check.

Regards

Pavel


>                         regards, tom lane
>
> PS: I didn't do it here, but I'm kind of tempted to pull out
> all the cursor-related tests in plpgsql.sql and move them to
> a new test file under src/pl/plpgsql/src/sql/.  They look
> pretty self-contained, and I doubt they're worth keeping in
> the core tests.
>
>

  3. Re: Missing can't-assign-to-constant checks in plpgsql

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-30T15:57:55Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> čt 28. 4. 2022 v 23:52 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
>> Perhaps the OPEN change is a little too aggressive, since if
>> you give the refcursor variable some non-null initial value,
>> OPEN won't change it; in that usage a CONSTANT marking could
>> be allowed.  But I really seriously doubt that anybody out
>> there is marking such variables as constants, so I thought
>> throwing the error at compile time was better than postponing
>> it to runtime so we could handle that.
>> 
>> Regardless of which way we handle that point, I'm inclined to
>> change this only in HEAD.  Probably people wouldn't thank us
>> for making the back branches more strict.

> +1

After sleeping on it, I got cold feet about breaking arguably
legal code, so I made OPEN check at runtime instead.  Which
was probably a good thing anyway, because it made me notice
that exec_stmt_forc() needed a check too.  AFAICS there are no
other places in pl_exec.c that are performing assignments to
variables not checked at parse time.

Pushed that way.

			regards, tom lane

  4. Re: Missing can't-assign-to-constant checks in plpgsql

    Pavel Stehule <pavel.stehule@gmail.com> — 2022-05-01T17:25:17Z

    Hi


>> Regardless of which way we handle that point, I'm inclined to
>> change this only in HEAD.  Probably people wouldn't thank us
>> for making the back branches more strict.
>>
>
> +1
>
> I can implement these checks in plpgsql_check. So possible issues can be
> detected and fixed on older versions by using plpgsql_check.
>

new related checks are implemented on plpgsql_check 2.1.4

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>>                         regards, tom lane
>>
>> PS: I didn't do it here, but I'm kind of tempted to pull out
>> all the cursor-related tests in plpgsql.sql and move them to
>> a new test file under src/pl/plpgsql/src/sql/.  They look
>> pretty self-contained, and I doubt they're worth keeping in
>> the core tests.
>>
>>