Обсуждение: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist
BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist
От
PG Bug reporting form
Дата:
The following bug has been logged on the website:
Bug reference: 15865
Logged by: Keith Fiske
Email address: keith.fiske@crunchydata.com
PostgreSQL version: 11.4
Operating system: CentOS7
Description:
When testing the setup of our monitoring platform, we started running into
an error when using PostgreSQL as a backend for Grafana. We narrowed down
the issue to only occurring with the latest point release of PG,
specifically 11.4, 10.9 and 9.6.14 (previous major versions were not tested
at this time). The issue can be recreated by following the setup steps for
Grafana with a PG backend and it will occur when the Grafana service is
started for the first time and it tries to set up its schema in PG. We do
not see the error occurring with the previous minor versions (11.3, 10.8,
9.6.13).
A standalone, reproducible use-case is as follows. The final, ALTER TABLE
statement (which is generated by Grafana) will cause the error:
-----------------------------
ERROR: relation "UQE_user_login" already exists
-----------------------------
However if each ALTER COLUMN statement is run independently, it seems to
work fine.
-----------------------------
CREATE TABLE public."user" (
id integer NOT NULL,
version integer NOT NULL,
login character varying(190) NOT NULL,
email character varying(190) NOT NULL,
name character varying(255),
password character varying(255),
salt character varying(50),
rands character varying(50),
company character varying(255),
org_id bigint NOT NULL,
is_admin boolean NOT NULL,
email_verified boolean,
theme character varying(255),
created timestamp without time zone NOT NULL,
updated timestamp without time zone NOT NULL,
help_flags1 bigint DEFAULT 0 NOT NULL
);
CREATE SEQUENCE public.user_id_seq1
AS integer
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER TABLE ONLY public."user" ALTER COLUMN id SET DEFAULT
nextval('public.user_id_seq1'::regclass);
SELECT pg_catalog.setval('public.user_id_seq1', 1, false);
ALTER TABLE ONLY public."user" ADD CONSTRAINT user_pkey1 PRIMARY KEY (id);
CREATE UNIQUE INDEX "UQE_user_email" ON public."user" USING btree (email);
CREATE UNIQUE INDEX "UQE_user_login" ON public."user" USING btree (login);
ALTER TABLE "user" ALTER "login" TYPE VARCHAR(190), ALTER "email" TYPE
VARCHAR(190), ALTER "name" TYPE VARCHAR(255), ALTER "password" TYPE
VARCHAR(255), ALTER "salt" TYPE VARCHAR(50), ALTER "rands" TYPE VARCHAR(50),
ALTER "company" TYPE VARCHAR(255), ALTER "theme" TYPE VARCHAR(255);
-----------------------------
On 2019-Jun-20, PG Bug reporting form wrote:
> When testing the setup of our monitoring platform, we started running into
> an error when using PostgreSQL as a backend for Grafana. We narrowed down
> the issue to only occurring with the latest point release of PG,
> specifically 11.4, 10.9 and 9.6.14 (previous major versions were not tested
> at this time). The issue can be recreated by following the setup steps for
> Grafana with a PG backend and it will occur when the Grafana service is
> started for the first time and it tries to set up its schema in PG. We do
> not see the error occurring with the previous minor versions (11.3, 10.8,
> 9.6.13).
Confirmed. Bisection says that
commit e76de886157b7f974d4d247908b242607cfbf043
Author: Tom Lane <tgl@sss.pgh.pa.us>
AuthorDate: Wed Jun 12 12:29:24 2019 -0400
CommitDate: Wed Jun 12 12:29:39 2019 -0400
Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.
is the culprit.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Jun-20, PG Bug reporting form wrote:
>> When testing the setup of our monitoring platform, we started running into
>> an error when using PostgreSQL as a backend for Grafana.
> commit e76de886157b7f974d4d247908b242607cfbf043
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> AuthorDate: Wed Jun 12 12:29:24 2019 -0400
> CommitDate: Wed Jun 12 12:29:39 2019 -0400
> Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.
Yeah, obviously I fat-fingered something there. Looking ...
regards, tom lane
I wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> commit e76de886157b7f974d4d247908b242607cfbf043
>> Author: Tom Lane <tgl@sss.pgh.pa.us>
>> AuthorDate: Wed Jun 12 12:29:24 2019 -0400
>> CommitDate: Wed Jun 12 12:29:39 2019 -0400
>> Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.
> Yeah, obviously I fat-fingered something there. Looking ...
Sigh ... so the answer is that I added the cleanup code (lines
10831..10864 in HEAD) in the wrong place. Putting it in
ATExecAlterColumnType is wrong because that gets executed potentially
multiple times per ALTER command, but I'd coded the cleanup assuming
that it would run only once. So we can end up with duplicate entries
in the changedIndexDefs list.
The right place to put it is in ATPostAlterTypeCleanup, of course.
(I think we could eliminate the changedIndexDefs list altogether and
just build the index-defining commands in the loop that uses them.)
This is a pretty embarrassing bug, reinforcing my hindsight view
that I was firing on very few cylinders last week. It basically
means that any ALTER TABLE that tries to alter the type of more than
one column is going to fail, if any but the last such column has a
dependent plain (non-constraint) index. The test cases added by
e76de8861 were oh so close to noticing that, but not close enough.
I'll go fix it, but do we need to consider a near-term re-release?
regards, tom lane
On Thu, Jun 20, 2019 at 08:20:55PM -0400, Tom Lane wrote: > This is a pretty embarrassing bug, reinforcing my hindsight view > that I was firing on very few cylinders last week. It basically > means that any ALTER TABLE that tries to alter the type of more than > one column is going to fail, if any but the last such column has a > dependent plain (non-constraint) index. The test cases added by > e76de8861 were oh so close to noticing that, but not close enough. > > I'll go fix it, but do we need to consider a near-term re-release? Ugh. That's a possibility. Changing each ALTER TABLE to be run individually can be a pain, and we really ought to push for the fix of the most recent CVE as soon as possible :( -- Michael
Вложения
I wrote:
>> Yeah, obviously I fat-fingered something there. Looking ...
After further review it seems like I was led into this error by a siren
singing something about how we could skip collecting the index definition
string for an index we were going to ignore later. (Cue standard lecture
about premature optimization...) That absolutely *does not* work, because
we might not find out till we're considering some later ALTER TYPE
subcommand that the index depends on a relevant constraint. And we have
to capture the index definition before we alter the type of any column it
depends on, or pg_get_indexdef_string will get very confused. That little
dependency wasn't documented anywhere. I also found a pre-existing
comment that contradicted the new reality but I'd missed removing in
e76de8861.
Here's a patch against HEAD --- since I'm feeling more mortal than usual
right now, I'll put this out for review rather than just pushing it.
It might be easier to review the code changes by just ignoring e76de8861
and diffing against tablecmds.c from before that, as I've done in the
second attachment.
BTW, has anyone got an explanation for the order in which psql is
listing the indexes of "anothertab" in this test case?
regards, tom lane
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb2c5e1..83dc6df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10508,9 +10508,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
SysScanDesc scan;
HeapTuple depTup;
ObjectAddress address;
- ListCell *lc;
- ListCell *prev;
- ListCell *next;
/*
* Clear all the missing values if we're rewriting the table, since this
@@ -10603,11 +10600,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
* performed all the individual ALTER TYPE operations. We have to save
* the info before executing ALTER TYPE, though, else the deparser will
* get confused.
- *
- * There could be multiple entries for the same object, so we must check
- * to ensure we process each one only once. Note: we assume that an index
- * that implements a constraint will not show a direct dependency on the
- * column.
*/
depRel = table_open(DependRelationId, RowExclusiveLock);
@@ -10650,19 +10642,21 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
relKind == RELKIND_PARTITIONED_INDEX)
{
/*
- * Indexes that are directly dependent on the table
- * might be regular indexes or constraint indexes.
- * Constraint indexes typically have only indirect
- * dependencies; but there are exceptions, notably
- * partial exclusion constraints. Hence we must check
- * whether the index depends on any constraint that's
- * due to be rebuilt, which we'll do below after we've
- * found all such constraints.
+ * This de-duplication check is critical for two
+ * independent reasons: we mustn't try to create the
+ * same index twice, and if an index depends on more
+ * than one column whose type is to be altered, we
+ * must capture its definition string before applying
+ * any of the column type changes.
*/
Assert(foundObject.objectSubId == 0);
- tab->changedIndexOids =
- list_append_unique_oid(tab->changedIndexOids,
- foundObject.objectId);
+ if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
+ {
+ tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
+ foundObject.objectId);
+ tab->changedIndexDefs = lappend(tab->changedIndexDefs,
+ pg_get_indexdef_string(foundObject.objectId));
+ }
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10697,6 +10691,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
+
+ /*
+ * As with indexes, de-duplication is critical, and it's
+ * needed because the same constraint could depend on multiple
+ * target columns.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedConstraintOids,
foundObject.objectId))
@@ -10829,41 +10829,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
systable_endscan(scan);
/*
- * Check the collected index OIDs to see which ones belong to the
- * constraint(s) of the table, and drop those from the list of indexes
- * that we need to process; rebuilding the constraints will handle them.
- */
- prev = NULL;
- for (lc = list_head(tab->changedIndexOids); lc; lc = next)
- {
- Oid indexoid = lfirst_oid(lc);
- Oid conoid;
-
- next = lnext(lc);
-
- conoid = get_index_constraint(indexoid);
- if (OidIsValid(conoid) &&
- list_member_oid(tab->changedConstraintOids, conoid))
- tab->changedIndexOids = list_delete_cell(tab->changedIndexOids,
- lc, prev);
- else
- prev = lc;
- }
-
- /*
- * Now collect the definitions of the indexes that must be rebuilt. (We
- * could merge this into the previous loop, but it'd be more complicated
- * for little gain.)
- */
- foreach(lc, tab->changedIndexOids)
- {
- Oid indexoid = lfirst_oid(lc);
-
- tab->changedIndexDefs = lappend(tab->changedIndexDefs,
- pg_get_indexdef_string(indexoid));
- }
-
- /*
* Now scan for dependencies of this column on other things. The only
* thing we should find is the dependency on the column datatype, which we
* want to remove, possibly a collation dependency, and dependencies on
@@ -11242,8 +11207,25 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
def_item, tab->changedIndexDefs)
{
Oid oldId = lfirst_oid(oid_item);
+ Oid conoid;
Oid relid;
+ /*
+ * We must check each index to see if it belongs to a constraint that
+ * we already processed above. Typically this check does not fire
+ * because constraint indexes normally have only dependencies on their
+ * constraint, and thus would not have been found by the dependency
+ * scan in ATExecAlterColumnType. But it's possible for such an index
+ * to also have direct dependencies on the table, for example with a
+ * partial exclusion constraint. In that case we'd have list entries
+ * for both the index and the constraint, and we must disregard the
+ * one for the index.
+ */
+ conoid = get_index_constraint(oldId);
+ if (OidIsValid(conoid) &&
+ list_member_oid(tab->changedConstraintOids, conoid))
+ continue;
+
relid = IndexGetRelation(oldId, false);
ATPostAlterTypeParse(oldId, relid, InvalidOid,
(char *) lfirst(def_item),
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c845a16..01ba41c 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1906,6 +1906,10 @@ alter table anothertab
add exclude using btree (f3 with =);
alter table anothertab
add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add unique(f3,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
\d anothertab
Table "public.anothertab"
Column | Type | Collation | Nullable | Default
@@ -1917,6 +1921,9 @@ alter table anothertab
Indexes:
"anothertab_pkey" PRIMARY KEY, btree (f1)
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f3_f4_key" UNIQUE CONSTRAINT, btree (f3, f4)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
@@ -1936,6 +1943,9 @@ alter table anothertab
Indexes:
"anothertab_pkey" PRIMARY KEY, btree (f1)
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f3_f4_key" UNIQUE CONSTRAINT, btree (f3, f4)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8c8b627..95fa1ce 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1323,6 +1323,10 @@ alter table anothertab
add exclude using btree (f3 with =);
alter table anothertab
add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add unique(f3,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
\d anothertab
alter table anothertab alter column f1 type bigint;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 98519ef..83dc6df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10600,11 +10600,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
* performed all the individual ALTER TYPE operations. We have to save
* the info before executing ALTER TYPE, though, else the deparser will
* get confused.
- *
- * There could be multiple entries for the same object, so we must check
- * to ensure we process each one only once. Note: we assume that an index
- * that implements a constraint will not show a direct dependency on the
- * column.
*/
depRel = table_open(DependRelationId, RowExclusiveLock);
@@ -10646,6 +10641,14 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
if (relKind == RELKIND_INDEX ||
relKind == RELKIND_PARTITIONED_INDEX)
{
+ /*
+ * This de-duplication check is critical for two
+ * independent reasons: we mustn't try to create the
+ * same index twice, and if an index depends on more
+ * than one column whose type is to be altered, we
+ * must capture its definition string before applying
+ * any of the column type changes.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
{
@@ -10688,6 +10691,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
+
+ /*
+ * As with indexes, de-duplication is critical, and it's
+ * needed because the same constraint could depend on multiple
+ * target columns.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedConstraintOids,
foundObject.objectId))
@@ -11198,8 +11207,25 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
def_item, tab->changedIndexDefs)
{
Oid oldId = lfirst_oid(oid_item);
+ Oid conoid;
Oid relid;
+ /*
+ * We must check each index to see if it belongs to a constraint that
+ * we already processed above. Typically this check does not fire
+ * because constraint indexes normally have only dependencies on their
+ * constraint, and thus would not have been found by the dependency
+ * scan in ATExecAlterColumnType. But it's possible for such an index
+ * to also have direct dependencies on the table, for example with a
+ * partial exclusion constraint. In that case we'd have list entries
+ * for both the index and the constraint, and we must disregard the
+ * one for the index.
+ */
+ conoid = get_index_constraint(oldId);
+ if (OidIsValid(conoid) &&
+ list_member_oid(tab->changedConstraintOids, conoid))
+ continue;
+
relid = IndexGetRelation(oldId, false);
ATPostAlterTypeParse(oldId, relid, InvalidOid,
(char *) lfirst(def_item),
On Thu, Jun 20, 2019 at 9:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
>> Yeah, obviously I fat-fingered something there. Looking ...
After further review it seems like I was led into this error by a siren
singing something about how we could skip collecting the index definition
string for an index we were going to ignore later. (Cue standard lecture
about premature optimization...) That absolutely *does not* work, because
we might not find out till we're considering some later ALTER TYPE
subcommand that the index depends on a relevant constraint. And we have
to capture the index definition before we alter the type of any column it
depends on, or pg_get_indexdef_string will get very confused. That little
dependency wasn't documented anywhere. I also found a pre-existing
comment that contradicted the new reality but I'd missed removing in
e76de8861.
Here's a patch against HEAD --- since I'm feeling more mortal than usual
right now, I'll put this out for review rather than just pushing it.
It might be easier to review the code changes by just ignoring e76de8861
and diffing against tablecmds.c from before that, as I've done in the
second attachment.
BTW, has anyone got an explanation for the order in which psql is
listing the indexes of "anothertab" in this test case?
regards, tom lane
Can't really provide a thorough code review, but I did apply the patch to the base 11.4 code (not HEAD from github) and the compound ALTER table statement that was failing before now works without error. Thank you for the quick fix!
Keith Fiske <keith.fiske@crunchydata.com> writes:
> On Thu, Jun 20, 2019 at 9:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Here's a patch against HEAD --- since I'm feeling more mortal than usual
>> right now, I'll put this out for review rather than just pushing it.
> Can't really provide a thorough code review, but I did apply the patch to
> the base 11.4 code (not HEAD from github) and the compound ALTER table
> statement that was failing before now works without error. Thank you for
> the quick fix!
Thanks for testing! However, I had a nagging feeling that I was still
missing something, and this morning I realized what. The proposed
patch basically changes ATExecAlterColumnType's assumptions from
"no constraint index will have any direct dependencies on table columns"
to "if a constraint index has a direct dependency on a table column,
so will its constraint". This is easily shown to not be the case:
regression=# create table foo (f1 int, f2 int);
CREATE TABLE
regression=# alter table foo add exclude using btree (f1 with =) where (f2 > 0);
ALTER TABLE
regression=# select pg_describe_object(classid,objid,objsubid) as obj,
pg_describe_object(refclassid,refobjid,refobjsubid)as ref, deptype from pg_depend where objid >= 'foo'::regclass or
refobjid>= 'foo'::regclass;
obj | ref | deptype
-------------------------------------+-------------------------------------+---------
type foo | table foo | i
type foo[] | type foo | i
table foo | schema public | n
constraint foo_f1_excl on table foo | column f1 of table foo | a
index foo_f1_excl | constraint foo_f1_excl on table foo | i
index foo_f1_excl | column f2 of table foo | a
(6 rows)
Notice that the index has a dependency on column f2 but the constraint
doesn't. So if we change (just) f2, ATExecAlterColumnType never notices
the constraint at all, and kaboom:
regression=# alter table foo alter column f2 type bigint;
ERROR: cannot drop index foo_f1_excl because constraint foo_f1_excl on table foo requires it
HINT: You can drop constraint foo_f1_excl on table foo instead.
This is the same with or without yesterday's patch, and while I didn't
trouble to verify it, I'm quite sure pre-e76de8861 would fail the same.
I'm not exactly convinced that this dependency structure is a Good Thing,
but in any case we don't get to rethink it in released branches. So
we need to make ATExecAlterColumnType cope, and the way to do that seems
to be to do the get_index_constraint check in that function not later on.
In principle this might lead to a few more duplicative
get_index_constraint calls than before, because if a constraint index has
multiple column dependencies we'll have to repeat get_index_constraint for
each one. But I hardly think that case is worth stressing about the
performance of, given it never worked at all before this month.
As before, I attach a patch against HEAD, plus one that assumes e76de8861
has been reverted first, which is likely easier to review.
Unlike yesterday, I'm feeling pretty good about this patch now, but it
still wouldn't hurt for somebody else to go over it.
regards, tom lane
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb2c5e1..8205e08 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10508,9 +10508,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
SysScanDesc scan;
HeapTuple depTup;
ObjectAddress address;
- ListCell *lc;
- ListCell *prev;
- ListCell *next;
/*
* Clear all the missing values if we're rewriting the table, since this
@@ -10603,11 +10600,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
* performed all the individual ALTER TYPE operations. We have to save
* the info before executing ALTER TYPE, though, else the deparser will
* get confused.
- *
- * There could be multiple entries for the same object, so we must check
- * to ensure we process each one only once. Note: we assume that an index
- * that implements a constraint will not show a direct dependency on the
- * column.
*/
depRel = table_open(DependRelationId, RowExclusiveLock);
@@ -10650,19 +10642,58 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
relKind == RELKIND_PARTITIONED_INDEX)
{
/*
- * Indexes that are directly dependent on the table
- * might be regular indexes or constraint indexes.
- * Constraint indexes typically have only indirect
- * dependencies; but there are exceptions, notably
- * partial exclusion constraints. Hence we must check
- * whether the index depends on any constraint that's
- * due to be rebuilt, which we'll do below after we've
- * found all such constraints.
+ * This de-duplication check is critical for two
+ * independent reasons: we mustn't try to recreate the
+ * same index twice, and if an index depends on more
+ * than one column whose type is to be altered, we
+ * must capture its definition string before applying
+ * any of the column type changes.
*/
Assert(foundObject.objectSubId == 0);
- tab->changedIndexOids =
- list_append_unique_oid(tab->changedIndexOids,
- foundObject.objectId);
+ if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
+ {
+ /*
+ * Before adding it as an index-to-rebuild,
+ * though, we'd better see if it belongs to a
+ * constraint, and if so rebuild the constraint
+ * instead. Typically this check does not fire
+ * because constraint indexes normally have only
+ * dependencies on their constraint. But it's
+ * possible for such an index to also have direct
+ * dependencies on the table, for example with a
+ * partial exclusion constraint. In that case we
+ * have to handle this, or bad things ensue.
+ */
+ Oid conoid;
+
+ conoid = get_index_constraint(foundObject.objectId);
+ if (OidIsValid(conoid))
+ {
+ /* Should match OCLASS_CONSTRAINT code below */
+ if (!list_member_oid(tab->changedConstraintOids,
+ conoid))
+ {
+ char *defstring = pg_get_constraintdef_command(conoid);
+
+ tab->changedConstraintOids =
+ lappend_oid(tab->changedConstraintOids,
+ conoid);
+ tab->changedConstraintDefs =
+ lappend(tab->changedConstraintDefs,
+ defstring);
+ }
+ }
+ else
+ {
+ /* OK to handle as plain index */
+ tab->changedIndexOids =
+ lappend_oid(tab->changedIndexOids,
+ foundObject.objectId);
+ tab->changedIndexDefs =
+ lappend(tab->changedIndexDefs,
+ pg_get_indexdef_string(foundObject.objectId));
+ }
+ }
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10697,6 +10728,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
+
+ /*
+ * As with indexes, de-duplication is critical, and it's
+ * needed because the same constraint could depend on multiple
+ * target columns.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedConstraintOids,
foundObject.objectId))
@@ -10829,41 +10866,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
systable_endscan(scan);
/*
- * Check the collected index OIDs to see which ones belong to the
- * constraint(s) of the table, and drop those from the list of indexes
- * that we need to process; rebuilding the constraints will handle them.
- */
- prev = NULL;
- for (lc = list_head(tab->changedIndexOids); lc; lc = next)
- {
- Oid indexoid = lfirst_oid(lc);
- Oid conoid;
-
- next = lnext(lc);
-
- conoid = get_index_constraint(indexoid);
- if (OidIsValid(conoid) &&
- list_member_oid(tab->changedConstraintOids, conoid))
- tab->changedIndexOids = list_delete_cell(tab->changedIndexOids,
- lc, prev);
- else
- prev = lc;
- }
-
- /*
- * Now collect the definitions of the indexes that must be rebuilt. (We
- * could merge this into the previous loop, but it'd be more complicated
- * for little gain.)
- */
- foreach(lc, tab->changedIndexOids)
- {
- Oid indexoid = lfirst_oid(lc);
-
- tab->changedIndexDefs = lappend(tab->changedIndexDefs,
- pg_get_indexdef_string(indexoid));
- }
-
- /*
* Now scan for dependencies of this column on other things. The only
* thing we should find is the dependency on the column datatype, which we
* want to remove, possibly a collation dependency, and dependencies on
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c845a16..5207554 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1901,11 +1901,18 @@ select * from anothertab;
drop table anothertab;
-- Test alter table column type with constraint indexes (cf. bug #15835)
-create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
+create table anothertab(f1 int primary key, f2 int unique,
+ f3 int, f4 int, f5 int);
alter table anothertab
add exclude using btree (f3 with =);
alter table anothertab
add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f5 > 0);
+alter table anothertab
+ add unique(f1,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
\d anothertab
Table "public.anothertab"
Column | Type | Collation | Nullable | Default
@@ -1914,17 +1921,23 @@ alter table anothertab
f2 | integer | | |
f3 | integer | | |
f4 | integer | | |
+ f5 | integer | | |
Indexes:
"anothertab_pkey" PRIMARY KEY, btree (f1)
+ "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+ "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
alter table anothertab alter column f1 type bigint;
alter table anothertab
alter column f2 type bigint,
alter column f3 type bigint,
alter column f4 type bigint;
+alter table anothertab alter column f5 type bigint;
\d anothertab
Table "public.anothertab"
Column | Type | Collation | Nullable | Default
@@ -1933,11 +1946,16 @@ alter table anothertab
f2 | bigint | | |
f3 | bigint | | |
f4 | bigint | | |
+ f5 | bigint | | |
Indexes:
"anothertab_pkey" PRIMARY KEY, btree (f1)
+ "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+ "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
drop table anothertab;
create table another (f1 int, f2 text);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8c8b627..601253c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1318,11 +1318,18 @@ select * from anothertab;
drop table anothertab;
-- Test alter table column type with constraint indexes (cf. bug #15835)
-create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
+create table anothertab(f1 int primary key, f2 int unique,
+ f3 int, f4 int, f5 int);
alter table anothertab
add exclude using btree (f3 with =);
alter table anothertab
add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f5 > 0);
+alter table anothertab
+ add unique(f1,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
\d anothertab
alter table anothertab alter column f1 type bigint;
@@ -1330,6 +1337,7 @@ alter table anothertab
alter column f2 type bigint,
alter column f3 type bigint,
alter column f4 type bigint;
+alter table anothertab alter column f5 type bigint;
\d anothertab
drop table anothertab;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 98519ef..8205e08 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10600,11 +10600,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
* performed all the individual ALTER TYPE operations. We have to save
* the info before executing ALTER TYPE, though, else the deparser will
* get confused.
- *
- * There could be multiple entries for the same object, so we must check
- * to ensure we process each one only once. Note: we assume that an index
- * that implements a constraint will not show a direct dependency on the
- * column.
*/
depRel = table_open(DependRelationId, RowExclusiveLock);
@@ -10646,13 +10641,58 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
if (relKind == RELKIND_INDEX ||
relKind == RELKIND_PARTITIONED_INDEX)
{
+ /*
+ * This de-duplication check is critical for two
+ * independent reasons: we mustn't try to recreate the
+ * same index twice, and if an index depends on more
+ * than one column whose type is to be altered, we
+ * must capture its definition string before applying
+ * any of the column type changes.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
{
- tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
- foundObject.objectId);
- tab->changedIndexDefs = lappend(tab->changedIndexDefs,
- pg_get_indexdef_string(foundObject.objectId));
+ /*
+ * Before adding it as an index-to-rebuild,
+ * though, we'd better see if it belongs to a
+ * constraint, and if so rebuild the constraint
+ * instead. Typically this check does not fire
+ * because constraint indexes normally have only
+ * dependencies on their constraint. But it's
+ * possible for such an index to also have direct
+ * dependencies on the table, for example with a
+ * partial exclusion constraint. In that case we
+ * have to handle this, or bad things ensue.
+ */
+ Oid conoid;
+
+ conoid = get_index_constraint(foundObject.objectId);
+ if (OidIsValid(conoid))
+ {
+ /* Should match OCLASS_CONSTRAINT code below */
+ if (!list_member_oid(tab->changedConstraintOids,
+ conoid))
+ {
+ char *defstring = pg_get_constraintdef_command(conoid);
+
+ tab->changedConstraintOids =
+ lappend_oid(tab->changedConstraintOids,
+ conoid);
+ tab->changedConstraintDefs =
+ lappend(tab->changedConstraintDefs,
+ defstring);
+ }
+ }
+ else
+ {
+ /* OK to handle as plain index */
+ tab->changedIndexOids =
+ lappend_oid(tab->changedIndexOids,
+ foundObject.objectId);
+ tab->changedIndexDefs =
+ lappend(tab->changedIndexDefs,
+ pg_get_indexdef_string(foundObject.objectId));
+ }
}
}
else if (relKind == RELKIND_SEQUENCE)
@@ -10688,6 +10728,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
+
+ /*
+ * As with indexes, de-duplication is critical, and it's
+ * needed because the same constraint could depend on multiple
+ * target columns.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedConstraintOids,
foundObject.objectId))
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index cedc7c6..5207554 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1900,6 +1900,64 @@ select * from anothertab;
(3 rows)
drop table anothertab;
+-- Test alter table column type with constraint indexes (cf. bug #15835)
+create table anothertab(f1 int primary key, f2 int unique,
+ f3 int, f4 int, f5 int);
+alter table anothertab
+ add exclude using btree (f3 with =);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f5 > 0);
+alter table anothertab
+ add unique(f1,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
+\d anothertab
+ Table "public.anothertab"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ f1 | integer | | not null |
+ f2 | integer | | |
+ f3 | integer | | |
+ f4 | integer | | |
+ f5 | integer | | |
+Indexes:
+ "anothertab_pkey" PRIMARY KEY, btree (f1)
+ "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
+ "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
+ "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
+ "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+ "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
+
+alter table anothertab alter column f1 type bigint;
+alter table anothertab
+ alter column f2 type bigint,
+ alter column f3 type bigint,
+ alter column f4 type bigint;
+alter table anothertab alter column f5 type bigint;
+\d anothertab
+ Table "public.anothertab"
+ Column | Type | Collation | Nullable | Default
+--------+--------+-----------+----------+---------
+ f1 | bigint | | not null |
+ f2 | bigint | | |
+ f3 | bigint | | |
+ f4 | bigint | | |
+ f5 | bigint | | |
+Indexes:
+ "anothertab_pkey" PRIMARY KEY, btree (f1)
+ "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
+ "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
+ "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
+ "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+ "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
+
+drop table anothertab;
create table another (f1 int, f2 text);
insert into another values(1, 'one');
insert into another values(2, 'two');
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index e8e094a..601253c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1317,6 +1317,31 @@ select * from anothertab;
drop table anothertab;
+-- Test alter table column type with constraint indexes (cf. bug #15835)
+create table anothertab(f1 int primary key, f2 int unique,
+ f3 int, f4 int, f5 int);
+alter table anothertab
+ add exclude using btree (f3 with =);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f5 > 0);
+alter table anothertab
+ add unique(f1,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
+
+\d anothertab
+alter table anothertab alter column f1 type bigint;
+alter table anothertab
+ alter column f2 type bigint,
+ alter column f3 type bigint,
+ alter column f4 type bigint;
+alter table anothertab alter column f5 type bigint;
+\d anothertab
+
+drop table anothertab;
+
create table another (f1 int, f2 text);
insert into another values(1, 'one');
I wrote:
> BTW, has anyone got an explanation for the order in which psql is
> listing the indexes of "anothertab" in this test case?
Ah, here's the explanation: describe.c is sorting the indexes
with this:
"ORDER BY i.indisprimary DESC, i.indisunique DESC, c2.relname;"
I can see the point of putting the pkey first, I guess, but the preference
for uniques second seems pretty bizarre, especially since
(a) it doesn't distinguish unique constraints from plain unique indexes and
(b) there's no similar preference for exclusion constraints, even though
those might be morally equivalent to a unique constraint.
What do people think of dropping the indisunique sort column here?
Obviously not back-patch material, but it might be more sensible
behavior going forward.
regards, tom lane
On Fri, Jun 21, 2019 at 12:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Keith Fiske <keith.fiske@crunchydata.com> writes:
> On Thu, Jun 20, 2019 at 9:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Here's a patch against HEAD --- since I'm feeling more mortal than usual
>> right now, I'll put this out for review rather than just pushing it.
> Can't really provide a thorough code review, but I did apply the patch to
> the base 11.4 code (not HEAD from github) and the compound ALTER table
> statement that was failing before now works without error. Thank you for
> the quick fix!
Thanks for testing! However, I had a nagging feeling that I was still
missing something, and this morning I realized what. The proposed
patch basically changes ATExecAlterColumnType's assumptions from
"no constraint index will have any direct dependencies on table columns"
to "if a constraint index has a direct dependency on a table column,
so will its constraint". This is easily shown to not be the case:
regression=# create table foo (f1 int, f2 int);
CREATE TABLE
regression=# alter table foo add exclude using btree (f1 with =) where (f2 > 0);
ALTER TABLE
regression=# select pg_describe_object(classid,objid,objsubid) as obj, pg_describe_object(refclassid,refobjid,refobjsubid) as ref, deptype from pg_depend where objid >= 'foo'::regclass or refobjid >= 'foo'::regclass;
obj | ref | deptype
-------------------------------------+-------------------------------------+---------
type foo | table foo | i
type foo[] | type foo | i
table foo | schema public | n
constraint foo_f1_excl on table foo | column f1 of table foo | a
index foo_f1_excl | constraint foo_f1_excl on table foo | i
index foo_f1_excl | column f2 of table foo | a
(6 rows)
Notice that the index has a dependency on column f2 but the constraint
doesn't. So if we change (just) f2, ATExecAlterColumnType never notices
the constraint at all, and kaboom:
regression=# alter table foo alter column f2 type bigint;
ERROR: cannot drop index foo_f1_excl because constraint foo_f1_excl on table foo requires it
HINT: You can drop constraint foo_f1_excl on table foo instead.
This is the same with or without yesterday's patch, and while I didn't
trouble to verify it, I'm quite sure pre-e76de8861 would fail the same.
I'm not exactly convinced that this dependency structure is a Good Thing,
but in any case we don't get to rethink it in released branches. So
we need to make ATExecAlterColumnType cope, and the way to do that seems
to be to do the get_index_constraint check in that function not later on.
In principle this might lead to a few more duplicative
get_index_constraint calls than before, because if a constraint index has
multiple column dependencies we'll have to repeat get_index_constraint for
each one. But I hardly think that case is worth stressing about the
performance of, given it never worked at all before this month.
As before, I attach a patch against HEAD, plus one that assumes e76de8861
has been reverted first, which is likely easier to review.
Unlike yesterday, I'm feeling pretty good about this patch now, but it
still wouldn't hurt for somebody else to go over it.
regards, tom lane
Tested applying the patch against HEAD this time. Combined ALTER TABLE again works without issue.
Keith Fiske <keith.fiske@crunchydata.com> writes:
> On Fri, Jun 21, 2019 at 12:12 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> As before, I attach a patch against HEAD, plus one that assumes e76de8861
>> has been reverted first, which is likely easier to review.
> Tested applying the patch against HEAD this time. Combined ALTER TABLE
> again works without issue.
Again, thanks for testing!
regards, tom lane
I wrote:
> As before, I attach a patch against HEAD, plus one that assumes e76de8861
> has been reverted first, which is likely easier to review.
> Unlike yesterday, I'm feeling pretty good about this patch now, but it
> still wouldn't hurt for somebody else to go over it.
I started to back-patch this, and soon noticed that the content of the
OCLASS_CONSTRAINT case branch in ATExecAlterColumnType has varied across
versions, which makes copy-and-pasting it seem pretty hazardous. Hence
it seems prudent to do slightly more work and split that code out into
a subroutine rather than having two copies. As attached, which is a
hopefully-final patch for HEAD. As before, it presumes reversion of
e76de8861, because it's a lot easier to see what's going on that way.
BTW ... while working on this, I got annoyed by the fact that
ATExecAlterColumnGenericOptions was inserted, no doubt with the aid of a
dartboard, into the middle of a large group of AlterColumnType-related
functions. Would anyone mind a separate patch to relocate it down
past those, probably just before ATExecChangeOwner?
regards, tom lane
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 98519ef..7d62a0f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -456,6 +456,8 @@ static void ATPrepAlterColumnType(List **wqueue,
static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno);
static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
+static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
+static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
List *options, LOCKMODE lockmode);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
@@ -10600,11 +10602,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
* performed all the individual ALTER TYPE operations. We have to save
* the info before executing ALTER TYPE, though, else the deparser will
* get confused.
- *
- * There could be multiple entries for the same object, so we must check
- * to ensure we process each one only once. Note: we assume that an index
- * that implements a constraint will not show a direct dependency on the
- * column.
*/
depRel = table_open(DependRelationId, RowExclusiveLock);
@@ -10647,13 +10644,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
relKind == RELKIND_PARTITIONED_INDEX)
{
Assert(foundObject.objectSubId == 0);
- if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
- {
- tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
- foundObject.objectId);
- tab->changedIndexDefs = lappend(tab->changedIndexDefs,
- pg_get_indexdef_string(foundObject.objectId));
- }
+ RememberIndexForRebuilding(foundObject.objectId, tab);
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10689,18 +10680,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
case OCLASS_CONSTRAINT:
Assert(foundObject.objectSubId == 0);
- if (!list_member_oid(tab->changedConstraintOids,
- foundObject.objectId))
- {
- char *defstring = pg_get_constraintdef_command(foundObject.objectId);
-
- tab->changedConstraintOids =
- lappend_oid(tab->changedConstraintOids,
- foundObject.objectId);
- tab->changedConstraintDefs =
- lappend(tab->changedConstraintDefs,
- defstring);
- }
+ RememberConstraintForRebuilding(foundObject.objectId, tab);
break;
case OCLASS_REWRITE:
@@ -11000,6 +10980,75 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
/*
+ * Subroutine for ATExecAlterColumnType: remember that a constraint needs
+ * to be rebuilt (which we might already know).
+ */
+static void
+RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
+{
+ /*
+ * This de-duplication check is critical for two independent reasons: we
+ * mustn't try to recreate the same constraint twice, and if a constraint
+ * depends on more than one column whose type is to be altered, we must
+ * capture its definition string before applying any of the column type
+ * changes. ruleutils.c will get confused if we ask again later.
+ */
+ if (!list_member_oid(tab->changedConstraintOids, conoid))
+ {
+ /* OK, capture the constraint's existing definition string */
+ char *defstring = pg_get_constraintdef_command(conoid);
+
+ tab->changedConstraintOids = lappend_oid(tab->changedConstraintOids,
+ conoid);
+ tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
+ defstring);
+ }
+}
+
+/*
+ * Subroutine for ATExecAlterColumnType: remember that an index needs
+ * to be rebuilt (which we might already know).
+ */
+static void
+RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+ /*
+ * This de-duplication check is critical for two independent reasons: we
+ * mustn't try to recreate the same index twice, and if an index depends
+ * on more than one column whose type is to be altered, we must capture
+ * its definition string before applying any of the column type changes.
+ * ruleutils.c will get confused if we ask again later.
+ */
+ if (!list_member_oid(tab->changedIndexOids, indoid))
+ {
+ /*
+ * Before adding it as an index-to-rebuild, we'd better see if it
+ * belongs to a constraint, and if so rebuild the constraint instead.
+ * Typically this check fails, because constraint indexes normally
+ * have only dependencies on their constraint. But it's possible for
+ * such an index to also have direct dependencies on table columns,
+ * for example with a partial exclusion constraint.
+ */
+ Oid conoid = get_index_constraint(indoid);
+
+ if (OidIsValid(conoid))
+ {
+ RememberConstraintForRebuilding(conoid, tab);
+ }
+ else
+ {
+ /* OK, capture the index's existing definition string */
+ char *defstring = pg_get_indexdef_string(indoid);
+
+ tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
+ indoid);
+ tab->changedIndexDefs = lappend(tab->changedIndexDefs,
+ defstring);
+ }
+ }
+}
+
+/*
* Returns the address of the modified column
*/
static ObjectAddress
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index cedc7c6..a639601 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1900,6 +1900,64 @@ select * from anothertab;
(3 rows)
drop table anothertab;
+-- Test index handling in alter table column type (cf. bugs #15835, #15865)
+create table anothertab(f1 int primary key, f2 int unique,
+ f3 int, f4 int, f5 int);
+alter table anothertab
+ add exclude using btree (f3 with =);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f5 > 0);
+alter table anothertab
+ add unique(f1,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
+\d anothertab
+ Table "public.anothertab"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ f1 | integer | | not null |
+ f2 | integer | | |
+ f3 | integer | | |
+ f4 | integer | | |
+ f5 | integer | | |
+Indexes:
+ "anothertab_pkey" PRIMARY KEY, btree (f1)
+ "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
+ "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
+ "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
+ "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+ "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
+
+alter table anothertab alter column f1 type bigint;
+alter table anothertab
+ alter column f2 type bigint,
+ alter column f3 type bigint,
+ alter column f4 type bigint;
+alter table anothertab alter column f5 type bigint;
+\d anothertab
+ Table "public.anothertab"
+ Column | Type | Collation | Nullable | Default
+--------+--------+-----------+----------+---------
+ f1 | bigint | | not null |
+ f2 | bigint | | |
+ f3 | bigint | | |
+ f4 | bigint | | |
+ f5 | bigint | | |
+Indexes:
+ "anothertab_pkey" PRIMARY KEY, btree (f1)
+ "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
+ "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f4_idx" UNIQUE, btree (f4)
+ "anothertab_f2_f3_idx" btree (f2, f3)
+ "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
+ "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+ "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
+
+drop table anothertab;
create table another (f1 int, f2 text);
insert into another values(1, 'one');
insert into another values(2, 'two');
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index e8e094a..0369909 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1317,6 +1317,31 @@ select * from anothertab;
drop table anothertab;
+-- Test index handling in alter table column type (cf. bugs #15835, #15865)
+create table anothertab(f1 int primary key, f2 int unique,
+ f3 int, f4 int, f5 int);
+alter table anothertab
+ add exclude using btree (f3 with =);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f4 is not null);
+alter table anothertab
+ add exclude using btree (f4 with =) where (f5 > 0);
+alter table anothertab
+ add unique(f1,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
+
+\d anothertab
+alter table anothertab alter column f1 type bigint;
+alter table anothertab
+ alter column f2 type bigint,
+ alter column f3 type bigint,
+ alter column f4 type bigint;
+alter table anothertab alter column f5 type bigint;
+\d anothertab
+
+drop table anothertab;
+
create table another (f1 int, f2 text);
insert into another values(1, 'one');