Re: BUG #17872: Dropping an attribute of a composite type breaks indexes over the type silently

Alexander Lakhin <exclusion@gmail.com> writes:

27.03.2023 21:20, Tom Lane wrote:

Hmm ... really?  I'd just concluded that a partitioned table is okay
as long as it doesn't yet have any partitions.  Even if the modified
type is a partitioning column, there's no structure yet that could
depend on the contents of the type.  (If it does have partitions,
we'll fail when we get to one of those.)

The following query leads to a failure on showing a partition definition:
CREATE TABLE tbl(a int, b int) PARTITION BY LIST ((tbl));
CREATE TABLE tblp PARTITION OF tbl FOR VALUES IN ('(2,4)');
ALTER TABLE tbl ALTER COLUMN a TYPE char(5);

Sure, but there you already have a partition.  If you only had "tbl"
then there would be no stored partition bounds.

Yes, that's a separate case when a value of the composite type "tbl" stored
in relpartbound for "tblp", but there is no dependency of "tbl"/"tblp" on the
type "tbl".

The similar situation:
CREATE TYPE ctype AS (a int, b int);
CREATE TABLE tbl(x ctype) PARTITION BY LIST (x);
CREATE TABLE tblp PARTITION OF tbl FOR VALUES IN ('(2,4)');
ALTER TYPE ctype ALTER ATTRIBUTE b TYPE char(5);
is prevented with
ERROR:  cannot alter type "ctype" because column "tbl.x" uses it
because there is dependency {"tbl", objsubid = 1} on the "ctype".

Regarding dropping attributes/columns, I've added the
find_composite_type_dependencies() call inside ATPrepDropColumn() and found
that it breaks cases that were valid before (according to regression tests). E.g.:
 CREATE TYPE test_type3 AS (a int);
 CREATE TABLE test_tbl3 (c) AS SELECT '(1)'::test_type3;
 ALTER TYPE test_type3 DROP ATTRIBUTE a, ADD ATTRIBUTE b int;
+ERROR:  cannot alter type "test_type3" because column "test_tbl3.c" uses it

So I agree, that disabling the drop operation when a composite type has
dependencies is not a harmless change. On the other hand, the code might be
not ready to deal with the uniqueness assumption violated.
For example:
CREATE TYPE ctype AS (a int, b int);
CREATE TABLE lp (t ctype) PARTITION BY RANGE (t);
CREATE TABLE lp_1_0 PARTITION OF lp FOR VALUES FROM (ROW(1, 01)::ctype) TO (ROW(1, 10)::ctype);
CREATE TABLE lp_1_1 PARTITION OF lp FOR VALUES FROM (ROW(1, 11)::ctype) TO (ROW(1, 20)::ctype);
ALTER TYPE ctype DROP ATTRIBUTE b;
INSERT INTO lp values(ROW(1)::ctype);
leads to an assertion failure:
...
#5  0x000055d3131e314f in ExceptionalCondition (
    conditionName=conditionName@entry=0x55d313357b85 "next_index == nparts",
    fileName=fileName@entry=0x55d313357887 "partbounds.c", lineNumber=lineNumber@entry=884) at assert.c:66
#6  0x000055d312fe2e2b in create_range_bounds (boundspecs=boundspecs@entry=0x55d314e4b028,
    nparts=nparts@entry=2, key=key@entry=0x55d314ed7170, mapping=mapping@entry=0x7fffa34bc438)
    at partbounds.c:884
#7  0x000055d312fe46c7 in partition_bounds_create (boundspecs=boundspecs@entry=0x55d314e4b028, nparts=2,
    key=key@entry=0x55d314ed7170, mapping=mapping@entry=0x7fffa34bc438) at partbounds.c:336
#8  0x000055d312fe6a2c in RelationBuildPartitionDesc (rel=rel@entry=0x7f609da02920,
    omit_detached=omit_detached@entry=true) at partdesc.c:271
#9  0x000055d312fe6c99 in RelationGetPartitionDesc (rel=rel@entry=0x7f609da02920,
    omit_detached=<optimized out>) at partdesc.c:110
...
Maybe that exact Assert could be replaced with just elog(ERROR), but I'm
afraid it's not the only such place that exists today and will appear
tomorrow. At least without a comprehensive testing this way seems dangerous
to me. Though maybe postgres should be robust enough to not crash in such
situations (caused by a composite type modification or by something else).

Best regards,
Alexander