Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e76de88

Browse files
committed
Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.
ATExecAlterColumnType failed to consider the possibility that an index that needs to be rebuilt might be a child of a constraint that needs to be rebuilt. We missed this so far because usually a constraint index doesn't have a direct dependency on its table, just on the constraint object. But if there's a WHERE clause, then dependency analysis of the WHERE clause results in direct dependencies on the column(s) mentioned in WHERE. This led to trying to drop and rebuild both the constraint and its underlying index. In v11/HEAD, we successfully drop both the index and the constraint, and then try to rebuild both, and of course the second rebuild hits a duplicate-index-name problem. Before v11, it fails with obscure messages about a missing relation OID, due to trying to drop the index twice. This is essentially the same kind of problem noted in commit 20bef2c: the possible dependency linkages are broader than what ATExecAlterColumnType was designed for. It was probably OK when written, but it's certainly been broken since the introduction of partial exclusion constraints. Fix by adding an explicit check for whether any of the indexes-to-be-rebuilt belong to any of the constraints-to-be-rebuilt, and ignoring any that do. In passing, fix a latent bug introduced by commit 8b08f7d: in get_constraint_index() we must "continue" not "break" when rejecting a relation of a wrong relkind. This is harmless today because we don't expect that code path to be taken anyway; but if there ever were any relations to be ignored, the existing coding would have an extremely undesirable dependency on the order of pg_depend entries. Also adjust a couple of obsolete comments. Per bug #15835 from Yaroslav Schekin. Back-patch to all supported branches. Discussion: https://postgr.es/m/15835-32d9b7a76c06a7a9@postgresql.org
1 parent ceac450 commit e76de88

File tree

4 files changed

+118
-13
lines changed

4 files changed

+118
-13
lines changed

src/backend/catalog/pg_depend.c

+10-6
Original file line numberDiff line numberDiff line change
@@ -779,8 +779,8 @@ getOwnedSequence(Oid relid, AttrNumber attnum)
779779

780780
/*
781781
* get_constraint_index
782-
* Given the OID of a unique or primary-key constraint, return the
783-
* OID of the underlying unique index.
782+
* Given the OID of a unique, primary-key, or exclusion constraint,
783+
* return the OID of the underlying index.
784784
*
785785
* Return InvalidOid if the index couldn't be found; this suggests the
786786
* given OID is bogus, but we leave it to caller to decide what to do.
@@ -827,10 +827,13 @@ get_constraint_index(Oid constraintId)
827827
{
828828
char relkind = get_rel_relkind(deprec->objid);
829829

830-
/* This is pure paranoia; there shouldn't be any such */
830+
/*
831+
* This is pure paranoia; there shouldn't be any other relkinds
832+
* dependent on a constraint.
833+
*/
831834
if (relkind != RELKIND_INDEX &&
832835
relkind != RELKIND_PARTITIONED_INDEX)
833-
break;
836+
continue;
834837

835838
indexId = deprec->objid;
836839
break;
@@ -845,8 +848,9 @@ get_constraint_index(Oid constraintId)
845848

846849
/*
847850
* get_index_constraint
848-
* Given the OID of an index, return the OID of the owning unique or
849-
* primary-key constraint, or InvalidOid if no such constraint.
851+
* Given the OID of an index, return the OID of the owning unique,
852+
* primary-key, or exclusion constraint, or InvalidOid if there
853+
* is no owning constraint.
850854
*/
851855
Oid
852856
get_index_constraint(Oid indexId)

src/backend/commands/tablecmds.c

+51-7
Original file line numberDiff line numberDiff line change
@@ -10508,6 +10508,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1050810508
SysScanDesc scan;
1050910509
HeapTuple depTup;
1051010510
ObjectAddress address;
10511+
ListCell *lc;
10512+
ListCell *prev;
10513+
ListCell *next;
1051110514

1051210515
/*
1051310516
* Clear all the missing values if we're rewriting the table, since this
@@ -10646,14 +10649,20 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1064610649
if (relKind == RELKIND_INDEX ||
1064710650
relKind == RELKIND_PARTITIONED_INDEX)
1064810651
{
10652+
/*
10653+
* Indexes that are directly dependent on the table
10654+
* might be regular indexes or constraint indexes.
10655+
* Constraint indexes typically have only indirect
10656+
* dependencies; but there are exceptions, notably
10657+
* partial exclusion constraints. Hence we must check
10658+
* whether the index depends on any constraint that's
10659+
* due to be rebuilt, which we'll do below after we've
10660+
* found all such constraints.
10661+
*/
1064910662
Assert(foundObject.objectSubId == 0);
10650-
if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
10651-
{
10652-
tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
10653-
foundObject.objectId);
10654-
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
10655-
pg_get_indexdef_string(foundObject.objectId));
10656-
}
10663+
tab->changedIndexOids =
10664+
list_append_unique_oid(tab->changedIndexOids,
10665+
foundObject.objectId);
1065710666
}
1065810667
else if (relKind == RELKIND_SEQUENCE)
1065910668
{
@@ -10819,6 +10828,41 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1081910828

1082010829
systable_endscan(scan);
1082110830

10831+
/*
10832+
* Check the collected index OIDs to see which ones belong to the
10833+
* constraint(s) of the table, and drop those from the list of indexes
10834+
* that we need to process; rebuilding the constraints will handle them.
10835+
*/
10836+
prev = NULL;
10837+
for (lc = list_head(tab->changedIndexOids); lc; lc = next)
10838+
{
10839+
Oid indexoid = lfirst_oid(lc);
10840+
Oid conoid;
10841+
10842+
next = lnext(lc);
10843+
10844+
conoid = get_index_constraint(indexoid);
10845+
if (OidIsValid(conoid) &&
10846+
list_member_oid(tab->changedConstraintOids, conoid))
10847+
tab->changedIndexOids = list_delete_cell(tab->changedIndexOids,
10848+
lc, prev);
10849+
else
10850+
prev = lc;
10851+
}
10852+
10853+
/*
10854+
* Now collect the definitions of the indexes that must be rebuilt. (We
10855+
* could merge this into the previous loop, but it'd be more complicated
10856+
* for little gain.)
10857+
*/
10858+
foreach(lc, tab->changedIndexOids)
10859+
{
10860+
Oid indexoid = lfirst_oid(lc);
10861+
10862+
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
10863+
pg_get_indexdef_string(indexoid));
10864+
}
10865+
1082210866
/*
1082310867
* Now scan for dependencies of this column on other things. The only
1082410868
* thing we should find is the dependency on the column datatype, which we

src/test/regress/expected/alter_table.out

+40
Original file line numberDiff line numberDiff line change
@@ -1899,6 +1899,46 @@ select * from anothertab;
18991899
f | IT WAS NULL!
19001900
(3 rows)
19011901

1902+
drop table anothertab;
1903+
-- Test alter table column type with constraint indexes (cf. bug #15835)
1904+
create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
1905+
alter table anothertab
1906+
add exclude using btree (f3 with =);
1907+
alter table anothertab
1908+
add exclude using btree (f4 with =) where (f4 is not null);
1909+
\d anothertab
1910+
Table "public.anothertab"
1911+
Column | Type | Collation | Nullable | Default
1912+
--------+---------+-----------+----------+---------
1913+
f1 | integer | | not null |
1914+
f2 | integer | | |
1915+
f3 | integer | | |
1916+
f4 | integer | | |
1917+
Indexes:
1918+
"anothertab_pkey" PRIMARY KEY, btree (f1)
1919+
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1920+
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
1921+
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1922+
1923+
alter table anothertab alter column f1 type bigint;
1924+
alter table anothertab
1925+
alter column f2 type bigint,
1926+
alter column f3 type bigint,
1927+
alter column f4 type bigint;
1928+
\d anothertab
1929+
Table "public.anothertab"
1930+
Column | Type | Collation | Nullable | Default
1931+
--------+--------+-----------+----------+---------
1932+
f1 | bigint | | not null |
1933+
f2 | bigint | | |
1934+
f3 | bigint | | |
1935+
f4 | bigint | | |
1936+
Indexes:
1937+
"anothertab_pkey" PRIMARY KEY, btree (f1)
1938+
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1939+
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
1940+
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1941+
19021942
drop table anothertab;
19031943
create table another (f1 int, f2 text);
19041944
insert into another values(1, 'one');

src/test/regress/sql/alter_table.sql

+17
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,23 @@ select * from anothertab;
13171317

13181318
drop table anothertab;
13191319

1320+
-- Test alter table column type with constraint indexes (cf. bug #15835)
1321+
create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
1322+
alter table anothertab
1323+
add exclude using btree (f3 with =);
1324+
alter table anothertab
1325+
add exclude using btree (f4 with =) where (f4 is not null);
1326+
1327+
\d anothertab
1328+
alter table anothertab alter column f1 type bigint;
1329+
alter table anothertab
1330+
alter column f2 type bigint,
1331+
alter column f3 type bigint,
1332+
alter column f4 type bigint;
1333+
\d anothertab
1334+
1335+
drop table anothertab;
1336+
13201337
create table another (f1 int, f2 text);
13211338

13221339
insert into another values(1, 'one');

0 commit comments

Comments
 (0)