Message-ID: <1aa2eec0-2017-3870-6d88-228d08c587d3@gmail.com>
Date: Mon, 27 Mar 2023 21:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17872: Dropping an attribute of a composite type breaks
 indexes over the type silently
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org,
 Peter Eisentraut <peter.eisentraut@enterprisedb.com>
References: <17872-d0fbb799dc3fd85d@postgresql.org>
 <3169550.1679928579@sss.pgh.pa.us> <3182314.1679935605@sss.pgh.pa.us>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <3182314.1679935605@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/1aa2eec0-2017-3870-6d88-228d08c587d3%40gmail.com>
Precedence: bulk

Hi Tom,

27.03.2023 19:46, Tom Lane wrote:
> I wrote:
>> PG Bug reporting form <noreply@postgresql.org> writes:
>>> The following script:
>>> CREATE TYPE ctype AS (i int, j int);
>>> CREATE TABLE ctbl(a int, cf ctype);
>>> CREATE UNIQUE INDEX ctbl_idx ON ctbl(cf);
>>> INSERT INTO ctbl VALUES (1, '(1, 2)'::ctype), (2, '(1, 1)'::ctype);
>>> ALTER TYPE ctype DROP ATTRIBUTE j;
>>> Results in the UNIQUE constraint broken:
>>>
>> Meh.  I'm happy to classify this as "so don't do that".
> On the other hand, this seems considerably more troubling:
>
> regression=# CREATE TYPE ctype AS (i int, j int);
> CREATE TYPE
> regression=# CREATE TABLE ctbl(a int, b int);
> CREATE TABLE
> regression=# CREATE UNIQUE INDEX ctbl_idx ON ctbl((row(a,b)::ctype));
> CREATE INDEX
> regression=# ALTER TYPE ctype ALTER ATTRIBUTE j type numeric;
> ALTER TYPE
>
> If we had any data in the index, it'd now be completely broken.

Yes, ALTERing the attribute/column leads to much worse things. In particular,
I've seen a server crash and the "compressed pglz data is corrupt" error.
(I can present the concrete queries if they can be of interest to you
(maybe for including in regression tests).)

As to the race condition possibility, I'm not sure that this consideration
should be applicable only to this kind of DDL. (I've got a collection of
crashes caused by race conditions in the existing code (#17182 and alike).
I had plans to discuss this issue separately.)

> This should be forbidden, but find_composite_type_dependencies
> has no idea whatever that indexes might contain expression
> columns of the target datatype.  It does find a pg_depend
> entry showing the index as dependent on the composite type,
> but it ignores it because objsubid = 0 --- and would ignore it
> also because of the relkind.
>
> I think what we should do, instead of just ignoring objsubid = 0,
> is to look through the index's columns and see if any have
> atttypid equal to the target type.  If not, then the composite
> type is used in an expression but not stored on disk, so it's
> just as safe (or not) as a reference in a view.

Yes, I think something like that can resolve the issue.
But I would also note that the problem is not with indexes only, but also
with "... partition by list(comp_type_value)", for example.

Best regards,
Alexander