plpgsql fails to reinitialize record variables at block re-entry

Started by Tom Laneover 8 years ago3 messageshackers
Jump to latest
#1Tom 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?

regards, tom lane

Attachments:

reinit-record-variables-at-block-reentry.patchtext/x-diff; charset=us-ascii; name=reinit-record-variables-at-block-reentry.patchDownload+68-16
#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#1)
Re: plpgsql fails to reinitialize record variables at block re-entry

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

Show quoted text

regards, tom lane

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#2)
Re: plpgsql fails to reinitialize record variables at block re-entry

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