Thread: Something's busted in plpgsql composite-variable handling

While answering someone's question I was motivated to try this:

regression=# create type comp_type as (f1 int, f2 int);
CREATE TYPE
regression=# create function foo(int) returns int language plpgsql as '
declare z comp_type;
begin
  z.f1 = $1; z.f2 = $1+1;
  return z.f1 + z.f2;
end';
CREATE FUNCTION
regression=# select foo(32);
 foo 
-----
  65
(1 row)

regression=# drop type comp_type;
DROP TYPE
regression=# create type comp_type as (f1 int, f2 int);
CREATE TYPE
regression=# select foo(32);
ERROR:  could not open relation with OID 52068
CONTEXT:  PL/pgSQL function foo(integer) line 4 at assignment

That's not very nice.  What's worse is that it works cleanly in v10,
making this a regression, no doubt caused by the hacking I did on
plpgsql's handling of composite variables.

I'll create an open item for this.  I've not yet looked into it
in any more detail than observing the failure.

            regards, tom lane

I wrote:
> [ dropping and recreating a composite type confuses plpgsql ]
> That's not very nice.  What's worse is that it works cleanly in v10,
> making this a regression, no doubt caused by the hacking I did on
> plpgsql's handling of composite variables.

After further poking, I've concluded that it's largely chance that
I tried an example that works in v10, because a lot of related
cases don't.  This particular test case only accesses the record's
fields by name, and in the PLPGSQL_DTYPE_ROW code paths those are
compiled into separate scalar variables that are still valid even
though an access to the whole record would fail.  I'm dubious that
it's a common use-case to create a composite variable that is only
used as if it were a set of independent variables.

Also, you can get similar failures by dropping and recreating
other sorts of user-defined types, not just composites, and those
errors go way back.

So, if we were to do something about this, I think it ought to involve
recompiling the function if it contains any reference to a user-defined
type that got dropped since last time; that's not specific to composite
variables.  Maybe that's something we'll get motivated to do someday ...
but I'm not sure how to do it without taking a performance hit for
checking for the situation, and in any case it seems like new development
not something to tackle in late beta.

So I'm now inclined to withdraw this as an open item.  On the other
hand, it is a bit worrisome that I happened to hit on a case that
worked better before.  Maybe I'm wrong to judge this unlikely to
happen in the field.

Thoughts?

            regards, tom lane

> On Aug 26, 2018, at 4:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
>> [ dropping and recreating a composite type confuses plpgsql ]
>> That's not very nice.  What's worse is that it works cleanly in v10,
>> making this a regression, no doubt caused by the hacking I did on
>> plpgsql's handling of composite variables.
>
> So I'm now inclined to withdraw this as an open item.  On the other
> hand, it is a bit worrisome that I happened to hit on a case that
> worked better before.  Maybe I'm wrong to judge this unlikely to
> happen in the field.
>
> Thoughts?

Typically if you’re creating a composite type, you’re planning to store
data in that type, so you’re probably not going to just drop it without
an appropriate migration strategy around it, which would (hopefully)
prevent the above case from happening.

I wouldn’t let this block the release, so +1 for removing from open
items.

Jonathan

2018-08-28 17:04 GMT+02:00 Jonathan S. Katz <jkatz@postgresql.org>:

On Aug 28, 2018, at 10:45 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:

2018-08-28 16:38 GMT+02:00 Jonathan S. Katz <jkatz@postgresql.org>:

> On Aug 26, 2018, at 4:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> I wrote:
>> [ dropping and recreating a composite type confuses plpgsql ]
>> That's not very nice.  What's worse is that it works cleanly in v10,
>> making this a regression, no doubt caused by the hacking I did on
>> plpgsql's handling of composite variables.
> 
> So I'm now inclined to withdraw this as an open item.  On the other
> hand, it is a bit worrisome that I happened to hit on a case that
> worked better before.  Maybe I'm wrong to judge this unlikely to
> happen in the field.
> 
> Thoughts?

Typically if you’re creating a composite type, you’re planning to store
data in that type, so you’re probably not going to just drop it without
an appropriate migration strategy around it, which would (hopefully)
prevent the above case from happening.

I wouldn’t let this block the release, so +1 for removing from open
items.

That depends - the question is - what is a reason of this issue, and how to fix it?

Tom explained the cause and a proposed a fix earlier in the thread, and

cautioned that it could involve a performance hit.

It is not strong issue, but it is issue, that breaks without outage deployment.

Have you encountered this issue in the field? It is a bug, but it seems to

be an edge case based on normal usage of PostgreSQL, and I still don’t

see a reason why it needs to be fixed prior to the release of 11. If there’s

an easier solution for solving it, yes, we could go ahead, but it sounds like

there’s a nontrivial amount of work + testing to do.

I do think it should be fixed for 12 now that we’ve identified it. We could move

it from the “Open Items” to the “Live Issues” list for what it’s worth.

+1

I've gone ahead and moved this to "Live Issues" - Thanks!

Jonathan