Re: BUG #17706: ALTER TYPE leads to crash

On Wed, Dec 7, 2022 at 9:17 PM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:

>
> On Thu, Dec 8, 2022 at 4:51 AM PG Bug reporting form <
> noreply(at)postgresql(dot)org> wrote:
>
>> With PL/pgSQL:
>>
>> create type foo as (a int, b int);
>>
>> create function bar() returns record as $$
>> declare
>> r foo := row(123, 2^30);
>> begin
>> alter type foo alter attribute b type text;
>> return r;
>> end;
>> $$ language plpgsql;
>>
>> postgres=# select bar();
>> server closed the connection unexpectedly
>> This probably means the server terminated abnormally
>> before or while processing the request.
>>
>> LOG: server process (PID 394076) was terminated by signal 11:
>> Segmentation
>> fault
>>
>> (Here 2^30 is interpreted as a string length.)
>
>
> ISTM after the alter operation, the attribute description of 'b' has
> been altered to cope with type text, but the data layout inside the heap
> tuple stays unchanged. So when we fetch attribute 'b', what we get is
> an integer pointer into the tuple's data area storing value 2^30, due to
> type text is not attbyval.
>
> Then later we interpret that integer pointer as a varlena pointer, which
> is not correct.
>
> But I'm not sure how to fix it. Is there an easy way to also alter the
> data layout inside the tuple?
>
>
From the docs:

The variants to add and drop attributes are part of the SQL standard; the
other variants are PostgreSQL extensions.

I think I understand why the standard didn't include "ALTER TYPE ... SET
DATA TYPE"...

We didn't even add the USING clause that exists for ALTER TABLE

(realizes there is no table involved, and that MVCC prevents this
particular pl/pgsql scoped issue generally)

We don't need to fix this though (i.e., make the query continue to somehow
work), we need to detect the situation and return some kind of error as
opposed to crashing the server. The transaction itself is doing something
illegal and can be forced to abort. Alternatively, maybe the ALTER TYPE
command can emit the error that it is unable to alter the type as it is
already in use by the current session. But that would close off possible
non-problematic uses of ALTER TYPE in this situation

I don't see how you could generalize altering the data layout inside the
tuple without a USING clause stating how to cast the stored value to the
new type - the USING clause exists for a reason in ALTER TABLE.

Apparently there is a cache invalidation that happens at CCI on the ALTER
TYPE and when the new type definition shows up extra and missing columns
are added (null) or removed as needed (i.e., neither ADD nor DROP cause
issues). Which suggests that the desired solution is one that simply
converts the SEGFAULT into a non-server-crashing error and document the
general inability (or desire, IMO) to do better in a back-patchable way
(even if the user isn't getting a segfault there is still a problem - just
replace "text" in the example with "bigint".

David J.