From 0f33482f89a359ea730b24e16f73e8eaff5b56df Mon Sep 17 00:00:00 2001 From: Nikhil Sontakke Date: Thu, 13 Aug 2026 15:08:26 +0530 Subject: [PATCH v3] Refuse to drop a column whose row type is stored elsewhere A composite datum records only the OID and typmod of its row type, not the shape it was built with, so stored values are read back against the current definition. Dropping an attribute therefore reaches backwards: it vanishes from values written before the drop, and values that were distinct become equal. Under a unique index that leaves the table holding data the server still rejects in new rows and the index can no longer be rebuilt from: REINDEX fails, and so does restoring the database from its own dump. The same happens to values of the type stored as constants in views, constraints, defaults and function defaults. Attribute versioning does not catch this, by design: a dropped attribute keeps its attlen/attalign, so values formed before the drop remain safe to deform. They are just no longer the values that were stored. ALTER COLUMN TYPE already refuses to retype a row type that is stored elsewhere, through find_composite_type_dependencies(), but the call is gated on a rewrite being queued and the drop path queues none. Call it from ATExecDropColumn() as well. This covers ALTER TABLE ... DROP COLUMN and ALTER TYPE ... DROP ATTRIBUTE alike, since a table's row type is a composite type too, and it is done per level so that inheritance children are covered. Since find_composite_type_dependencies() now also checks constants stored in expressions, those are covered too. Several existing tests depended on the old behaviour: - alter_table's test_typex dropped an attribute of a stored type, and test_tbl3 built a pg_upgrade fixture that way; it now drops the attribute before creating the table. - alter_table's attdrop_* test stored values and then dropped columns of their row types. The part that remains possible, values held in plpgsql variables outliving a drop, is kept by dropping the storing table before the columns; the rest now checks that the drop is refused and that stored values of the narrowed types behave. - rowtypes' compositetable_drop dropped a column in the middle of a FieldStore; it now checks that the drop is refused there too. - pg_upgrade's 009_composite_types stored datums and then dropped columns. It still does that against an old installation that allows it, and otherwise drops the column before storing the datum, which reads back the same. - The test_decoding isolation test oldest_xmin used the drop only as a DDL that updates a pg_attribute tuple a concurrent reader has already seen, which RENAME ATTRIBUTE does equally well. --- .../test_decoding/expected/oldest_xmin.out | 4 +- contrib/test_decoding/specs/oldest_xmin.spec | 5 +- src/backend/commands/tablecmds.c | 12 ++ src/bin/pg_upgrade/t/009_composite_types.pl | 63 +++++-- src/test/regress/expected/alter_table.out | 159 ++++++++++++------ src/test/regress/expected/rowtypes.out | 10 +- src/test/regress/sql/alter_table.sql | 104 +++++++++--- src/test/regress/sql/rowtypes.sql | 5 +- 8 files changed, 274 insertions(+), 88 deletions(-) diff --git a/contrib/test_decoding/expected/oldest_xmin.out b/contrib/test_decoding/expected/oldest_xmin.out index 57268b38d33..70f3b258287 100644 --- a/contrib/test_decoding/expected/oldest_xmin.out +++ b/contrib/test_decoding/expected/oldest_xmin.out @@ -10,7 +10,7 @@ f step s1_begin: BEGIN; step s1_insert: INSERT INTO harvest VALUES ((1, 2, 3)); -step s0_alter: ALTER TYPE basket DROP ATTRIBUTE mangos; +step s0_alter: ALTER TYPE basket RENAME ATTRIBUTE mangos TO plums; step s0_commit: COMMIT; step s0_checkpoint: CHECKPOINT; step s0_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); @@ -49,7 +49,7 @@ f step s1_begin: BEGIN; step s1_insert: INSERT INTO harvest VALUES ((1, 2, 3)); -step s0_alter: ALTER TYPE basket DROP ATTRIBUTE mangos; +step s0_alter: ALTER TYPE basket RENAME ATTRIBUTE mangos TO plums; step s0_commit: COMMIT; step s0_checkpoint: CHECKPOINT; step s0_advance_slot: SELECT slot_name FROM pg_replication_slot_advance('isolation_slot', pg_current_wal_lsn()); diff --git a/contrib/test_decoding/specs/oldest_xmin.spec b/contrib/test_decoding/specs/oldest_xmin.spec index 7f2fe3d7ed7..97245f3d452 100644 --- a/contrib/test_decoding/specs/oldest_xmin.spec +++ b/contrib/test_decoding/specs/oldest_xmin.spec @@ -20,7 +20,10 @@ session "s0" setup { SET synchronous_commit=on; } step "s0_begin" { BEGIN; } step "s0_getxid" { SELECT pg_current_xact_id() IS NULL; } -step "s0_alter" { ALTER TYPE basket DROP ATTRIBUTE mangos; } +# Any DDL that updates a pg_attribute tuple s1 has already read will do here. +# Renaming rather than dropping, because dropping an attribute of a type that +# a table stores is refused. +step "s0_alter" { ALTER TYPE basket RENAME ATTRIBUTE mangos TO plums; } step "s0_commit" { COMMIT; } step "s0_checkpoint" { CHECKPOINT; } step "s0_vacuum" { VACUUM pg_attribute; } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index aa89c9a4637..b9eb14ab183 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10055,6 +10055,18 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, errmsg("cannot drop column \"%s\" because it is part of the partition key of relation \"%s\"", colName, RelationGetRelationName(rel)))); + /* + * Don't drop it if some other relation stores values of this relation's + * row type. A composite datum records only the OID and typmod of its row + * type, never the shape it was built with, so every stored value is read + * back against the current definition. Dropping the column would + * therefore make the attribute disappear from values that were written + * before the drop: values that were distinct can become equal, which can + * leave stored data violating a unique index that can then no longer be + * rebuilt. ALTER COLUMN TYPE already refuses for the same reason. + */ + find_composite_type_dependencies(rel->rd_rel->reltype, rel, NULL); + ReleaseSysCache(tuple); /* diff --git a/src/bin/pg_upgrade/t/009_composite_types.pl b/src/bin/pg_upgrade/t/009_composite_types.pl index 77b887b07bb..ce56cb77bfb 100644 --- a/src/bin/pg_upgrade/t/009_composite_types.pl +++ b/src/bin/pg_upgrade/t/009_composite_types.pl @@ -98,20 +98,10 @@ $old->safe_psql( ALTER TYPE rowver_comp ADD ATTRIBUTE b int; INSERT INTO rowver_tbl VALUES (2, '(2,22)'); - -- A stored composite datum with attributes that are later dropped. CREATE TABLE rowdrop_src (a int); ALTER TABLE rowdrop_src ADD COLUMN b int; - CREATE TABLE rowdrop_tbl (r rowdrop_src); - INSERT INTO rowdrop_tbl VALUES (ROW(1,2)); - ALTER TABLE rowdrop_src DROP COLUMN b; - - -- The same for a composite type, whose versions pg_dump collects - -- separately. CREATE TYPE rowdropt_comp AS (a int); ALTER TYPE rowdropt_comp ADD ATTRIBUTE b int; - CREATE TABLE rowdropt_tbl (c rowdropt_comp); - INSERT INTO rowdropt_tbl VALUES (ROW(1,2)); - ALTER TYPE rowdropt_comp DROP ATTRIBUTE b; -- Restore the attribute versions of a view with a stored datum made -- narrower than the view by a subsequent CREATE OR REPLACE VIEW. @@ -141,7 +131,6 @@ $old->safe_psql( -- A table whose column versions are cleared below, to simulate columns -- created before updating to a release with attribute versioning. CREATE TABLE rowunvdrop_src (a int, b int, c int); - CREATE TABLE rowunvdrop_tbl (r rowunvdrop_src); -- Further relations with a row type, with differing attribute -- versions, to verify pg_upgrade preserves their versions. @@ -161,6 +150,30 @@ $old->safe_psql( CREATE TYPE rowzero_comp AS (); }); +# A stored composite datum with attributes that are later dropped. +store_then_drop( + $old, q{ + CREATE TABLE rowdrop_tbl (r rowdrop_src); + INSERT INTO rowdrop_tbl VALUES (ROW(1,2)); + ALTER TABLE rowdrop_src DROP COLUMN b; +}, q{ + ALTER TABLE rowdrop_src DROP COLUMN b; + CREATE TABLE rowdrop_tbl (r rowdrop_src); + INSERT INTO rowdrop_tbl VALUES (ROW(1)); +}); + +# The same for a composite type, whose versions pg_dump collects separately. +store_then_drop( + $old, q{ + CREATE TABLE rowdropt_tbl (c rowdropt_comp); + INSERT INTO rowdropt_tbl VALUES (ROW(1,2)); + ALTER TYPE rowdropt_comp DROP ATTRIBUTE b; +}, q{ + ALTER TYPE rowdropt_comp DROP ATTRIBUTE b; + CREATE TABLE rowdropt_tbl (c rowdropt_comp); + INSERT INTO rowdropt_tbl VALUES (ROW(1)); +}); + # With versioning, clearing the type's versions produces a versioned datum # with version zero. This is distinct from an unversioned datum. if ($has_versioning) @@ -180,8 +193,16 @@ $old->safe_psql( -- Other attribute options and dropping a column must cope with columns -- lacking a version. ALTER TABLE rowunvdrop_src ALTER COLUMN c SET (n_distinct = 5); +}); +store_then_drop( + $old, q{ + CREATE TABLE rowunvdrop_tbl (r rowunvdrop_src); INSERT INTO rowunvdrop_tbl VALUES (ROW(1, 2, 3)); ALTER TABLE rowunvdrop_src DROP COLUMN b; +}, q{ + ALTER TABLE rowunvdrop_src DROP COLUMN b; + CREATE TABLE rowunvdrop_tbl (r rowunvdrop_src); + INSERT INTO rowunvdrop_tbl VALUES (ROW(1, 3)); }); if ($test_unversioned) @@ -386,6 +407,26 @@ SKIP: 'unversioned datum reports a version mismatch after type alteration'); } +# Store a composite datum, then drop a column of its row type. Servers that +# refuse to drop a column of a row type stored in a table (as all versions +# since that was forbidden do) get the column dropped first instead. The +# datum then reads back the same, and the dropped column is still exercised +# by pg_upgrade. +sub store_then_drop +{ + my ($node, $store_then_drop, $drop_then_store) = @_; + + my ($ret, $stdout, $stderr) = + $node->psql('postgres', "BEGIN;\n$store_then_drop\nCOMMIT;"); + return if $ret == 0; + + die "unexpected failure storing a datum, then dropping a column: $stderr" + unless $stderr =~ + /cannot alter (?:type|table) "[^"]+" because column "[^"]+" uses (?:it|its row type)/; + $node->safe_psql('postgres', $drop_then_store); + return; +} + sub versions_query { my ($version_expr) = @_; diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index f99b2b9e663..50d945b0d70 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -183,7 +183,7 @@ SELECT rownum, r, (r).c1 FROM atcomp_inc_ref ORDER BY rownum; 3 | (3,30,300) | 3 (3 rows) --- Store composites with columns that are then subsequently removed +-- Store composites of row types that subsequently lose columns CREATE TABLE attdrop_tail (t1 int, t2 text, t3 int8, t4 float8); CREATE TABLE attdrop_mid (m1 int, m2 text, m3 int8, m4 float8); CREATE TABLE attdrop_tozero (z1 int); @@ -204,22 +204,33 @@ SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* F r4-null-rows | | | | | | | | | | | | | | | (4 rows) --- while at it, make sure this works not just for top-level SQL, but also --- plpgsql, by doing the drop inside a DO block. +-- Dropping a column is refused while a table stores values of its row type, +-- as the column would disappear from the stored values. +ALTER TABLE attdrop_tail DROP COLUMN t4; -- fails +ERROR: cannot alter table "attdrop_tail" because column "attdrop_test.tail" uses its row type +ALTER TABLE attdrop_toadd DROP COLUMN a2; -- fails +ERROR: cannot alter table "attdrop_toadd" because column "attdrop_test.toadd" uses its row type +-- Values held outside of tables, here in plpgsql variables, can still outlive +-- a drop. Make sure that such values, formed before the drop, can still be +-- deformed after it: load them, drop the table storing the row types, and then +-- drop the columns. +CREATE TYPE attdrop_rec AS (key text, tail attdrop_tail, mid attdrop_mid, tozero attdrop_tozero, toadd attdrop_toadd); DO $$ DECLARE - r1 attdrop_test; + r1 attdrop_rec; r2 record; - r3 attdrop_test; + r3 attdrop_rec; r3_t attdrop_tail; r3_m attdrop_mid; r3_z attdrop_tozero; r3_a attdrop_toadd; BEGIN - r1 = at FROM attdrop_test at WHERE key = 'r1-all-present'; + r1 = ROW(at.key, at.tail, at.mid, at.tozero, at.toadd)::attdrop_rec + FROM attdrop_test at WHERE key = 'r1-all-present'; SELECT * INTO r2 FROM attdrop_test t WHERE key = 'r2-some-nulls'; - r3 = at FROM attdrop_test at WHERE key = 'r3-other-nulls'; + r3 = ROW(at.key, at.tail, at.mid, at.tozero, at.toadd)::attdrop_rec + FROM attdrop_test at WHERE key = 'r3-other-nulls'; r3_t = tail FROM attdrop_test at WHERE key = 'r3-other-nulls'; r3_m = r3.mid; r3_z = r3.tozero; @@ -231,6 +242,8 @@ BEGIN RAISE NOTICE 'r3 before drop: %', r3; RAISE NOTICE 'r3_* before drop: t: %, m: %, z: %, a: %', r3_t, r3_m, r3_z, r3_a; + DROP TABLE attdrop_test; + ALTER TABLE attdrop_tail DROP COLUMN t4; ALTER TABLE attdrop_mid DROP COLUMN m3; ALTER TABLE attdrop_tozero DROP COLUMN z1; @@ -251,50 +264,41 @@ NOTICE: r1 after drop: (r1-all-present,"(1,1,1)","(1,1,1)","()","(1)") NOTICE: r2 after drop: (r2-some-nulls,"(2,,2)","(,2,2)","()","(2)") NOTICE: r3 after drop: (r3-other-nulls,"(,3,)","(3,,)","()","(3)") NOTICE: r3_* after drop: t: (,3,), m: (3,,), z: (), a: (3) --- show plain executor after drop -SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; - key | tail | mid | tozero | toadd | t1 | t2 | t3 | m1 | m2 | m4 | a1 -----------------+---------+---------+--------+-------+----+----+----+----+----+----+---- - r1-all-present | (1,1,1) | (1,1,1) | () | (1) | 1 | 1 | 1 | 1 | 1 | 1 | 1 - r2-some-nulls | (2,,2) | (,2,2) | () | (2) | 2 | | 2 | | 2 | 2 | 2 - r3-other-nulls | (,3,) | (3,,) | () | (3) | | 3 | | 3 | | | 3 - r4-null-rows | | | | | | | | | | | -(4 rows) - --- check that updating the datums works after the drop, and that a new rows look right too -UPDATE attdrop_test SET tail.t1 = -(tail).t1, mid.m4 = -1, toadd.a1 = -(toadd).a1 WHERE key = 'r1-all-present'; +-- Store values of the narrowed row types, and check that they, and updates +-- to them, look right. The table is left behind to test pg_upgrade with row +-- types that have dropped columns. +CREATE TABLE attdrop_test(key text, tail attdrop_tail, mid attdrop_mid, tozero attdrop_tozero, toadd attdrop_toadd); INSERT INTO attdrop_test(key, tail, mid, tozero, toadd) VALUES + ('r4-null-rows', NULL, NULL, NULL, NULL), ('r5-post-drop-1', ROW(5, 5, 5), ROW(5, 5, 5), ROW(), ROW(5)), - ('r6-post-drop-2', ROW(6, 6, 6), ROW(6, 6, 6), ROW(), ROW(6)); + ('r6-post-drop-2', ROW(6, NULL, 6), ROW(NULL, 6, NULL), ROW(), ROW(6)); +UPDATE attdrop_test SET tail.t1 = -(tail).t1, mid.m4 = -1, toadd.a1 = -(toadd).a1 WHERE key = 'r5-post-drop-1'; SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; key | tail | mid | tozero | toadd | t1 | t2 | t3 | m1 | m2 | m4 | a1 ----------------+----------+----------+--------+-------+----+----+----+----+----+----+---- - r1-all-present | (-1,1,1) | (1,1,-1) | () | (-1) | -1 | 1 | 1 | 1 | 1 | -1 | -1 - r2-some-nulls | (2,,2) | (,2,2) | () | (2) | 2 | | 2 | | 2 | 2 | 2 - r3-other-nulls | (,3,) | (3,,) | () | (3) | | 3 | | 3 | | | 3 r4-null-rows | | | | | | | | | | | - r5-post-drop-1 | (5,5,5) | (5,5,5) | () | (5) | 5 | 5 | 5 | 5 | 5 | 5 | 5 - r6-post-drop-2 | (6,6,6) | (6,6,6) | () | (6) | 6 | 6 | 6 | 6 | 6 | 6 | 6 -(6 rows) + r5-post-drop-1 | (-5,5,5) | (5,5,-1) | () | (-5) | -5 | 5 | 5 | 5 | 5 | -1 | -5 + r6-post-drop-2 | (6,,6) | (,6,) | () | (6) | 6 | | 6 | | 6 | | 6 +(3 rows) --- add a new column, insert row, drop another set of columns +-- Adding a column is still allowed while values are stored; the stored values +-- read as NULL for it. Dropping one is refused again. ALTER TABLE attdrop_toadd ADD COLUMN a3 int; -ALTER TABLE attdrop_tail DROP COLUMN t3; -ALTER TABLE attdrop_mid DROP COLUMN m2; +ALTER TABLE attdrop_tail DROP COLUMN t3; -- fails +ERROR: cannot alter table "attdrop_tail" because column "attdrop_test.tail" uses its row type +ALTER TABLE attdrop_mid DROP COLUMN m2; -- fails +ERROR: cannot alter table "attdrop_mid" because column "attdrop_test.mid" uses its row type INSERT INTO attdrop_test(key, tail, mid, tozero, toadd) VALUES - ('r7-post-add', ROW(7, 7), ROW(7, 7), ROW(), ROW(7, 77)); -UPDATE attdrop_test SET tail.t2 = (tail).t2||'-u', mid.m4 = -2, toadd.a1 = -(toadd).a1, toadd.a3 = 23 WHERE key = 'r2-some-nulls'; + ('r7-post-add', ROW(7, 7, 7), ROW(7, 7, 7), ROW(), ROW(7, 77)); +UPDATE attdrop_test SET tail.t2 = (tail).t2||'-u', mid.m4 = -2, toadd.a1 = -(toadd).a1, toadd.a3 = 23 WHERE key = 'r6-post-drop-2'; SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; - key | tail | mid | tozero | toadd | t1 | t2 | m1 | m4 | a1 | a3 -----------------+--------+--------+--------+---------+----+----+----+----+----+---- - r1-all-present | (-1,1) | (1,-1) | () | (-1,) | -1 | 1 | 1 | -1 | -1 | - r2-some-nulls | (2,) | (,-2) | () | (-2,23) | 2 | | | -2 | -2 | 23 - r3-other-nulls | (,3) | (3,) | () | (3,) | | 3 | 3 | | 3 | - r4-null-rows | | | | | | | | | | - r5-post-drop-1 | (5,5) | (5,5) | () | (5,) | 5 | 5 | 5 | 5 | 5 | - r6-post-drop-2 | (6,6) | (6,6) | () | (6,) | 6 | 6 | 6 | 6 | 6 | - r7-post-add | (7,7) | (7,7) | () | (7,77) | 7 | 7 | 7 | 7 | 7 | 77 -(7 rows) + key | tail | mid | tozero | toadd | t1 | t2 | t3 | m1 | m2 | m4 | a1 | a3 +----------------+----------+----------+--------+---------+----+----+----+----+----+----+----+---- + r4-null-rows | | | | | | | | | | | | + r5-post-drop-1 | (-5,5,5) | (5,5,-1) | () | (-5,) | -5 | 5 | 5 | 5 | 5 | -1 | -5 | + r6-post-drop-2 | (6,,6) | (,6,-2) | () | (-6,23) | 6 | | 6 | | 6 | -2 | -6 | 23 + r7-post-add | (7,7,7) | (7,7,7) | () | (7,77) | 7 | 7 | 7 | 7 | 7 | 7 | 7 | 77 +(4 rows) -- Adding a column to a view goes through ATExecAddColumn; make sure that -- composite tuples using that row type can be deformed correctly. @@ -3661,29 +3665,88 @@ Inherits: DROP TABLE test_tbl2_subclass, test_tbl2; DROP TYPE test_type2; +-- Dropping an attribute is refused once a relation with storage has a column +-- of the type, whether or not anything else depends on the attribute, and +-- CASCADE does not override it. CREATE TYPE test_typex AS (a int, b text); CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0)); ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails -ERROR: cannot drop column a of composite type test_typex because other objects depend on it -DETAIL: constraint test_tblx_y_check on table test_tblx depends on column a of composite type test_typex -HINT: Use DROP ... CASCADE to drop the dependent objects too. -ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE; -NOTICE: drop cascades to constraint test_tblx_y_check on table test_tblx +ERROR: cannot alter type "test_typex" because column "test_tblx.y" uses it +ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE; -- fails +ERROR: cannot alter type "test_typex" because column "test_tblx.y" uses it \d test_tblx Table "public.test_tblx" Column | Type | Collation | Nullable | Default --------+------------+-----------+----------+--------- x | integer | | | y | test_typex | | | +Check constraints: + "test_tblx_y_check" CHECK ((y).a > 0) DROP TABLE test_tblx; DROP TYPE test_typex; +-- Values of a composite type record only its OID and typmod, never the shape +-- they were built with, so a stored value is read back against whatever the +-- type looks like at read time. Dropping an attribute would therefore make it +-- disappear from values written before the drop, which is refused: values that +-- were distinct would become equal, and where the column carries a unique +-- index the table would be left holding data that violates it and that the +-- index can no longer be rebuilt from. +CREATE TYPE test_stored AS (a int, b int); +CREATE TABLE test_stored_tbl (v test_stored); +CREATE UNIQUE INDEX test_stored_tbl_v ON test_stored_tbl (v); +INSERT INTO test_stored_tbl VALUES (ROW(1, 2)::test_stored), + (ROW(1, 3)::test_stored); +ALTER TYPE test_stored DROP ATTRIBUTE b; -- fails +ERROR: cannot alter type "test_stored" because column "test_stored_tbl.v" uses it +-- still two distinct values, and the index still rebuilds from them +SELECT count(DISTINCT v) FROM test_stored_tbl; + count +------- + 2 +(1 row) + +REINDEX INDEX test_stored_tbl_v; +-- once nothing stores the type, dropping the attribute is allowed again +DROP TABLE test_stored_tbl; +ALTER TYPE test_stored DROP ATTRIBUTE b; +DROP TYPE test_stored; +-- The same applies to a table's row type, which is a composite type too. +CREATE TABLE test_rowtype (a int, b int); +CREATE TABLE test_rowtype_ref (v test_rowtype); +ALTER TABLE test_rowtype DROP COLUMN b; -- fails +ERROR: cannot alter table "test_rowtype" because column "test_rowtype_ref.v" uses its row type +DROP TABLE test_rowtype_ref; +ALTER TABLE test_rowtype DROP COLUMN b; +DROP TABLE test_rowtype; +-- Values of the type stored as constants in expressions are covered too. +CREATE TYPE test_stored_const AS (a int, b int); +CREATE VIEW test_stored_view AS + SELECT '(1,2)'::test_stored_const = '(1,3)'::test_stored_const AS eq; +ALTER TYPE test_stored_const DROP ATTRIBUTE b; -- fails +ERROR: cannot alter type "test_stored_const" because rule _RETURN on view test_stored_view uses it +SELECT eq FROM test_stored_view; + eq +---- + f +(1 row) + +DROP VIEW test_stored_view; +CREATE TABLE test_stored_chk + (i int CHECK ('(1,2)'::test_stored_const <> '(1,3)'::test_stored_const)); +ALTER TYPE test_stored_const DROP ATTRIBUTE b; -- fails +ERROR: cannot alter type "test_stored_const" because constraint test_stored_chk_check on table test_stored_chk uses it +INSERT INTO test_stored_chk VALUES (1); +DROP TABLE test_stored_chk; +DROP TYPE test_stored_const; -- This test isn't that interesting on its own, but the purpose is to leave -- behind a table to test pg_upgrade with. The table has a composite type --- column in it, and the composite type has a dropped attribute. +-- column in it, and the composite type has a dropped attribute. The attribute +-- is dropped before the table is created, since it could not be dropped +-- afterwards. 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; +CREATE TABLE test_tbl3 (c) AS SELECT '(1)'::test_type3; CREATE TYPE test_type_empty AS (); DROP TYPE test_type_empty; -- diff --git a/src/test/regress/expected/rowtypes.out b/src/test/regress/expected/rowtypes.out index c402160d488..51316db946c 100644 --- a/src/test/regress/expected/rowtypes.out +++ b/src/test/regress/expected/rowtypes.out @@ -1528,13 +1528,17 @@ BEGIN EXECUTE 'ALTER TABLE compositetable_type_1 DROP COLUMN b'; RETURN 12345; END $$; --- assign to v.b while b is dropped - the value disappears +-- assign to v.b while trying to drop b - the drop is refused, as +-- compositetable_holder stores values of the row type UPDATE compositetable_holder SET v.b = compositetable_drop(); --- 12345 is nowhere to be found +ERROR: cannot alter table "compositetable_type_1" because column "compositetable_holder.v" uses its row type +CONTEXT: SQL statement "ALTER TABLE compositetable_type_1 DROP COLUMN b" +PL/pgSQL function compositetable_drop() line 3 at EXECUTE +-- the stored value is unchanged SELECT v::text AS stored_value FROM compositetable_holder; stored_value -------------- - (1) + (1,2) (1 row) DROP FUNCTION compositetable_drop(); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index a6b4265b811..884f189b534 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -218,7 +218,7 @@ INSERT INTO atcomp_inc_ref VALUES ('3', '(3,30,300)'); SELECT rownum, r, (r).c1 FROM atcomp_inc_ref ORDER BY rownum; --- Store composites with columns that are then subsequently removed +-- Store composites of row types that subsequently lose columns CREATE TABLE attdrop_tail (t1 int, t2 text, t3 int8, t4 float8); CREATE TABLE attdrop_mid (m1 int, m2 text, m3 int8, m4 float8); CREATE TABLE attdrop_tozero (z1 int); @@ -235,22 +235,32 @@ INSERT INTO attdrop_test(key, tail, mid, tozero, toadd) VALUES -- plain executor, before drop SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; --- while at it, make sure this works not just for top-level SQL, but also --- plpgsql, by doing the drop inside a DO block. +-- Dropping a column is refused while a table stores values of its row type, +-- as the column would disappear from the stored values. +ALTER TABLE attdrop_tail DROP COLUMN t4; -- fails +ALTER TABLE attdrop_toadd DROP COLUMN a2; -- fails + +-- Values held outside of tables, here in plpgsql variables, can still outlive +-- a drop. Make sure that such values, formed before the drop, can still be +-- deformed after it: load them, drop the table storing the row types, and then +-- drop the columns. +CREATE TYPE attdrop_rec AS (key text, tail attdrop_tail, mid attdrop_mid, tozero attdrop_tozero, toadd attdrop_toadd); DO $$ DECLARE - r1 attdrop_test; + r1 attdrop_rec; r2 record; - r3 attdrop_test; + r3 attdrop_rec; r3_t attdrop_tail; r3_m attdrop_mid; r3_z attdrop_tozero; r3_a attdrop_toadd; BEGIN - r1 = at FROM attdrop_test at WHERE key = 'r1-all-present'; + r1 = ROW(at.key, at.tail, at.mid, at.tozero, at.toadd)::attdrop_rec + FROM attdrop_test at WHERE key = 'r1-all-present'; SELECT * INTO r2 FROM attdrop_test t WHERE key = 'r2-some-nulls'; - r3 = at FROM attdrop_test at WHERE key = 'r3-other-nulls'; + r3 = ROW(at.key, at.tail, at.mid, at.tozero, at.toadd)::attdrop_rec + FROM attdrop_test at WHERE key = 'r3-other-nulls'; r3_t = tail FROM attdrop_test at WHERE key = 'r3-other-nulls'; r3_m = r3.mid; r3_z = r3.tozero; @@ -262,6 +272,8 @@ BEGIN RAISE NOTICE 'r3 before drop: %', r3; RAISE NOTICE 'r3_* before drop: t: %, m: %, z: %, a: %', r3_t, r3_m, r3_z, r3_a; + DROP TABLE attdrop_test; + ALTER TABLE attdrop_tail DROP COLUMN t4; ALTER TABLE attdrop_mid DROP COLUMN m3; ALTER TABLE attdrop_tozero DROP COLUMN z1; @@ -275,23 +287,25 @@ BEGIN END; $$; --- show plain executor after drop -SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; - --- check that updating the datums works after the drop, and that a new rows look right too -UPDATE attdrop_test SET tail.t1 = -(tail).t1, mid.m4 = -1, toadd.a1 = -(toadd).a1 WHERE key = 'r1-all-present'; +-- Store values of the narrowed row types, and check that they, and updates +-- to them, look right. The table is left behind to test pg_upgrade with row +-- types that have dropped columns. +CREATE TABLE attdrop_test(key text, tail attdrop_tail, mid attdrop_mid, tozero attdrop_tozero, toadd attdrop_toadd); INSERT INTO attdrop_test(key, tail, mid, tozero, toadd) VALUES + ('r4-null-rows', NULL, NULL, NULL, NULL), ('r5-post-drop-1', ROW(5, 5, 5), ROW(5, 5, 5), ROW(), ROW(5)), - ('r6-post-drop-2', ROW(6, 6, 6), ROW(6, 6, 6), ROW(), ROW(6)); + ('r6-post-drop-2', ROW(6, NULL, 6), ROW(NULL, 6, NULL), ROW(), ROW(6)); +UPDATE attdrop_test SET tail.t1 = -(tail).t1, mid.m4 = -1, toadd.a1 = -(toadd).a1 WHERE key = 'r5-post-drop-1'; SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; --- add a new column, insert row, drop another set of columns +-- Adding a column is still allowed while values are stored; the stored values +-- read as NULL for it. Dropping one is refused again. ALTER TABLE attdrop_toadd ADD COLUMN a3 int; -ALTER TABLE attdrop_tail DROP COLUMN t3; -ALTER TABLE attdrop_mid DROP COLUMN m2; +ALTER TABLE attdrop_tail DROP COLUMN t3; -- fails +ALTER TABLE attdrop_mid DROP COLUMN m2; -- fails INSERT INTO attdrop_test(key, tail, mid, tozero, toadd) VALUES - ('r7-post-add', ROW(7, 7), ROW(7, 7), ROW(), ROW(7, 77)); -UPDATE attdrop_test SET tail.t2 = (tail).t2||'-u', mid.m4 = -2, toadd.a1 = -(toadd).a1, toadd.a3 = 23 WHERE key = 'r2-some-nulls'; + ('r7-post-add', ROW(7, 7, 7), ROW(7, 7, 7), ROW(), ROW(7, 77)); +UPDATE attdrop_test SET tail.t2 = (tail).t2||'-u', mid.m4 = -2, toadd.a1 = -(toadd).a1, toadd.a3 = 23 WHERE key = 'r6-post-drop-2'; SELECT key, tail, mid, tozero, toadd, (tail).*, (mid).*, (tozero).*, (toadd).* FROM attdrop_test ORDER BY key; -- Adding a column to a view goes through ATExecAddColumn; make sure that @@ -2296,20 +2310,68 @@ ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE; DROP TABLE test_tbl2_subclass, test_tbl2; DROP TYPE test_type2; +-- Dropping an attribute is refused once a relation with storage has a column +-- of the type, whether or not anything else depends on the attribute, and +-- CASCADE does not override it. CREATE TYPE test_typex AS (a int, b text); CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0)); ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails -ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE; +ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE; -- fails \d test_tblx DROP TABLE test_tblx; DROP TYPE test_typex; +-- Values of a composite type record only its OID and typmod, never the shape +-- they were built with, so a stored value is read back against whatever the +-- type looks like at read time. Dropping an attribute would therefore make it +-- disappear from values written before the drop, which is refused: values that +-- were distinct would become equal, and where the column carries a unique +-- index the table would be left holding data that violates it and that the +-- index can no longer be rebuilt from. +CREATE TYPE test_stored AS (a int, b int); +CREATE TABLE test_stored_tbl (v test_stored); +CREATE UNIQUE INDEX test_stored_tbl_v ON test_stored_tbl (v); +INSERT INTO test_stored_tbl VALUES (ROW(1, 2)::test_stored), + (ROW(1, 3)::test_stored); +ALTER TYPE test_stored DROP ATTRIBUTE b; -- fails +-- still two distinct values, and the index still rebuilds from them +SELECT count(DISTINCT v) FROM test_stored_tbl; +REINDEX INDEX test_stored_tbl_v; +-- once nothing stores the type, dropping the attribute is allowed again +DROP TABLE test_stored_tbl; +ALTER TYPE test_stored DROP ATTRIBUTE b; +DROP TYPE test_stored; + +-- The same applies to a table's row type, which is a composite type too. +CREATE TABLE test_rowtype (a int, b int); +CREATE TABLE test_rowtype_ref (v test_rowtype); +ALTER TABLE test_rowtype DROP COLUMN b; -- fails +DROP TABLE test_rowtype_ref; +ALTER TABLE test_rowtype DROP COLUMN b; +DROP TABLE test_rowtype; + +-- Values of the type stored as constants in expressions are covered too. +CREATE TYPE test_stored_const AS (a int, b int); +CREATE VIEW test_stored_view AS + SELECT '(1,2)'::test_stored_const = '(1,3)'::test_stored_const AS eq; +ALTER TYPE test_stored_const DROP ATTRIBUTE b; -- fails +SELECT eq FROM test_stored_view; +DROP VIEW test_stored_view; +CREATE TABLE test_stored_chk + (i int CHECK ('(1,2)'::test_stored_const <> '(1,3)'::test_stored_const)); +ALTER TYPE test_stored_const DROP ATTRIBUTE b; -- fails +INSERT INTO test_stored_chk VALUES (1); +DROP TABLE test_stored_chk; +DROP TYPE test_stored_const; + -- This test isn't that interesting on its own, but the purpose is to leave -- behind a table to test pg_upgrade with. The table has a composite type --- column in it, and the composite type has a dropped attribute. +-- column in it, and the composite type has a dropped attribute. The attribute +-- is dropped before the table is created, since it could not be dropped +-- afterwards. 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; +CREATE TABLE test_tbl3 (c) AS SELECT '(1)'::test_type3; CREATE TYPE test_type_empty AS (); DROP TYPE test_type_empty; diff --git a/src/test/regress/sql/rowtypes.sql b/src/test/regress/sql/rowtypes.sql index 5aef9f5a3c3..ad417cfeaf4 100644 --- a/src/test/regress/sql/rowtypes.sql +++ b/src/test/regress/sql/rowtypes.sql @@ -684,10 +684,11 @@ BEGIN RETURN 12345; END $$; --- assign to v.b while b is dropped - the value disappears +-- assign to v.b while trying to drop b - the drop is refused, as +-- compositetable_holder stores values of the row type UPDATE compositetable_holder SET v.b = compositetable_drop(); --- 12345 is nowhere to be found +-- the stored value is unchanged SELECT v::text AS stored_value FROM compositetable_holder; DROP FUNCTION compositetable_drop(); -- 2.54.0 (Apple Git-157)