From: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Subject: Re: BUG #17872: Dropping an attribute of a composite type breaks indexes over the type silently
Date: Tue, 28 Mar 2023 19:00:00 +0300
Message-ID: <65c49195-7117-67c4-5bfe-45ca8832dfe3@gmail.com> (raw)
In-Reply-To: <3332642.1679943981@sss.pgh.pa.us>
References: <17872-d0fbb799dc3fd85d@postgresql.org>
<3169550.1679928579@sss.pgh.pa.us>
<3182314.1679935605@sss.pgh.pa.us>
<1aa2eec0-2017-3870-6d88-228d08c587d3@gmail.com>
<3267914.1679941213@sss.pgh.pa.us>
<67cb509e-93c7-1c7b-2388-d7e00277b344@gmail.com>
<3332642.1679943981@sss.pgh.pa.us>
27.03.2023 22:06, Tom Lane wrote:
> 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, tgl@sss.pgh.pa.us, peter.eisentraut@enterprisedb.com
Subject: Re: BUG #17872: Dropping an attribute of a composite type breaks indexes over the type silently
In-Reply-To: <65c49195-7117-67c4-5bfe-45ca8832dfe3@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox