Horton

Thread

Commits

  1. Fix plpgsql to reinitialize record variables at block re-entry.

  1. plpgsql fails to reinitialize record variables at block re-entry

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-12-09T06:24:31Z

    Consider

regression=# do $$
regression$# declare r record;
regression$# begin
regression$#   raise notice '%', r;
regression$# end$$;
ERROR:  record "r" is not assigned yet
DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.
CONTEXT:  SQL statement "SELECT r"
PL/pgSQL function inline_code_block line 4 at RAISE

Fine, you're supposed to assign something to the record before you use it.
But look at this:

regression=# do $$
regression$# begin
regression$# for i in 1..3 loop
regression$#   declare r record;
regression$#   begin
regression$#     if i = 1 then
regression$#       r := row(i);
regression$#     end if;
regression$#     raise notice '%', r;
regression$#   end;
regression$# end loop;
regression$# end$$;
NOTICE:  (1)
NOTICE:  (1)
NOTICE:  (1)
DO

Surely that ought to have failed at the i=2 iteration.  There is
code in plpgsql's exec_stmt_block() that tries to reinitialize
PLPGSQL_DTYPE_REC variables, but it's never reached (as a quick
look at coverage.postgresql.org will confirm), because what it
scans is only the variables attached to the block by
plpgsql_add_initdatums() --- and that function thinks it should
only pay attention to PLPGSQL_DTYPE_VAR variables.

The attached patch fixes this by teaching plpgsql_add_initdatums()
to also list PLPGSQL_DTYPE_REC variables, though I failed to resist
the temptation to make a couple of nearby cosmetic improvements.

What I'm not sure about is whether to back-patch this.  The current
behavior is indubitably not right, but we've had no field complaints,
and it's not entirely far-fetched that some poor sod might have written
code that depends on it working this way.  So maybe we should leave
it alone in the back branches.  Thoughts?

			regards, tom lane

  2. Re: plpgsql fails to reinitialize record variables at block re-entry

    Pavel Stehule <pavel.stehule@gmail.com> — 2017-12-09T06:47:29Z

    2017-12-09 7:24 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:

> Consider
>
> regression=# do $$
> regression$# declare r record;
> regression$# begin
> regression$#   raise notice '%', r;
> regression$# end$$;
> ERROR:  record "r" is not assigned yet
> DETAIL:  The tuple structure of a not-yet-assigned record is indeterminate.
> CONTEXT:  SQL statement "SELECT r"
> PL/pgSQL function inline_code_block line 4 at RAISE
>
> Fine, you're supposed to assign something to the record before you use it.
> But look at this:
>
> regression=# do $$
> regression$# begin
> regression$# for i in 1..3 loop
> regression$#   declare r record;
> regression$#   begin
> regression$#     if i = 1 then
> regression$#       r := row(i);
> regression$#     end if;
> regression$#     raise notice '%', r;
> regression$#   end;
> regression$# end loop;
> regression$# end$$;
> NOTICE:  (1)
> NOTICE:  (1)
> NOTICE:  (1)
> DO
>
> Surely that ought to have failed at the i=2 iteration.  There is
> code in plpgsql's exec_stmt_block() that tries to reinitialize
> PLPGSQL_DTYPE_REC variables, but it's never reached (as a quick
> look at coverage.postgresql.org will confirm), because what it
> scans is only the variables attached to the block by
> plpgsql_add_initdatums() --- and that function thinks it should
> only pay attention to PLPGSQL_DTYPE_VAR variables.
>
> The attached patch fixes this by teaching plpgsql_add_initdatums()
> to also list PLPGSQL_DTYPE_REC variables, though I failed to resist
> the temptation to make a couple of nearby cosmetic improvements.
>
> What I'm not sure about is whether to back-patch this.  The current
> behavior is indubitably not right, but we've had no field complaints,
> and it's not entirely far-fetched that some poor sod might have written
> code that depends on it working this way.  So maybe we should leave
> it alone in the back branches.  Thoughts?
>

you are correct

+1

Pavel


>                         regards, tom lane
>
>

  3. Re: plpgsql fails to reinitialize record variables at block re-entry

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-12-09T17:19:49Z

    Pavel Stehule <pavel.stehule@gmail.com> writes:
> 2017-12-09 7:24 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
>> Surely that ought to have failed at the i=2 iteration.  There is
>> code in plpgsql's exec_stmt_block() that tries to reinitialize
>> PLPGSQL_DTYPE_REC variables, but it's never reached (as a quick
>> look at coverage.postgresql.org will confirm), because what it
>> scans is only the variables attached to the block by
>> plpgsql_add_initdatums() --- and that function thinks it should
>> only pay attention to PLPGSQL_DTYPE_VAR variables.

> +1

Pushed.  Some excavation in our git history says this bug has been
there since the initial commit of plpgsql, making it a little over
19 years old.  Might be a new record for the age of a PG bug.

			regards, tom lane