Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf946a40

Browse files
committed
Further fix ALTER COLUMN TYPE's handling of indexes and index constraints.
This patch reverts all the code changes of commite76de88, which turnsout to have been seriously misguided. We can't wait till later to computethe definition string for an index; we must capture that before applyingthe data type change for any column it depends on, else ruleutils.c willdeliverr wrong/misleading results. (This fine point was documentednowhere, of course.)I'd also managed to forget that ATExecAlterColumnType executes once perALTER COLUMN TYPE clause, not once per statement; which resulted in thecode being basically completely broken for any case in which multiple ALTERCOLUMN TYPE clauses are applied to a table having non-constraint indexesthat must be rebuilt. Through very bad luck, none of the existing testcases nor the ones added bye76de88 caught that, but of course it wassoon found in the field.The previous patch also had an implicit assumption that if a constraint'sindex had a dependency on a table column, so would the constraint --- butthat isn't actually true, so it didn't fix such cases.Instead of trying to delete unneeded index dependencies later, do theis-there-a-constraint lookup immediately on seeing an index dependency,and switch to remembering the constraint if so. In the unusual case ofmultiple column dependencies for a constraint index, this will result induplicate constraint lookups, but that's not that horrible compared to allthe other work that happens here. Besides, such cases did not work at allbefore, so it's hard to argue that they're performance-critical for anyone.Per bug #15865 from Keith Fiske. As before, back-patch to all supportedbranches.Discussion:https://postgr.es/m/15865-17940eacc8f8b081@postgresql.org
1 parentb00326d commitf946a40

File tree

3 files changed

+103
-72
lines changed

3 files changed

+103
-72
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 73 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,8 @@ static void ATPrepAlterColumnType(List **wqueue,
456456
static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno);
457457
static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
458458
AlterTableCmd *cmd, LOCKMODE lockmode);
459+
static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
460+
static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
459461
static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
460462
List *options, LOCKMODE lockmode);
461463
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
@@ -10508,9 +10510,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1050810510
SysScanDesc scan;
1050910511
HeapTupledepTup;
1051010512
ObjectAddress address;
10511-
ListCell *lc;
10512-
ListCell *prev;
10513-
ListCell *next;
1051410513

1051510514
/*
1051610515
* Clear all the missing values if we're rewriting the table, since this
@@ -10603,11 +10602,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1060310602
* performed all the individual ALTER TYPE operations. We have to save
1060410603
* the info before executing ALTER TYPE, though, else the deparser will
1060510604
* get confused.
10606-
*
10607-
* There could be multiple entries for the same object, so we must check
10608-
* to ensure we process each one only once. Note: we assume that an index
10609-
* that implements a constraint will not show a direct dependency on the
10610-
* column.
1061110605
*/
1061210606
depRel = table_open(DependRelationId, RowExclusiveLock);
1061310607

@@ -10649,20 +10643,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1064910643
if (relKind == RELKIND_INDEX ||
1065010644
relKind == RELKIND_PARTITIONED_INDEX)
1065110645
{
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-
*/
1066210646
Assert(foundObject.objectSubId == 0);
10663-
tab->changedIndexOids =
10664-
list_append_unique_oid(tab->changedIndexOids,
10665-
foundObject.objectId);
10647+
RememberIndexForRebuilding(foundObject.objectId, tab);
1066610648
}
1066710649
else if (relKind == RELKIND_SEQUENCE)
1066810650
{
@@ -10698,18 +10680,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1069810680

1069910681
case OCLASS_CONSTRAINT:
1070010682
Assert(foundObject.objectSubId == 0);
10701-
if (!list_member_oid(tab->changedConstraintOids,
10702-
foundObject.objectId))
10703-
{
10704-
char *defstring = pg_get_constraintdef_command(foundObject.objectId);
10705-
10706-
tab->changedConstraintOids =
10707-
lappend_oid(tab->changedConstraintOids,
10708-
foundObject.objectId);
10709-
tab->changedConstraintDefs =
10710-
lappend(tab->changedConstraintDefs,
10711-
defstring);
10712-
}
10683+
RememberConstraintForRebuilding(foundObject.objectId, tab);
1071310684
break;
1071410685

1071510686
case OCLASS_REWRITE:
@@ -10828,41 +10799,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1082810799

1082910800
systable_endscan(scan);
1083010801

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-
Oidindexoid = lfirst_oid(lc);
10840-
Oidconoid;
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-
Oidindexoid = lfirst_oid(lc);
10861-
10862-
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
10863-
pg_get_indexdef_string(indexoid));
10864-
}
10865-
1086610802
/*
1086710803
* Now scan for dependencies of this column on other things. The only
1086810804
* thing we should find is the dependency on the column datatype, which we
@@ -11043,6 +10979,75 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1104310979
return address;
1104410980
}
1104510981

10982+
/*
10983+
* Subroutine for ATExecAlterColumnType: remember that a constraint needs
10984+
* to be rebuilt (which we might already know).
10985+
*/
10986+
static void
10987+
RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
10988+
{
10989+
/*
10990+
* This de-duplication check is critical for two independent reasons: we
10991+
* mustn't try to recreate the same constraint twice, and if a constraint
10992+
* depends on more than one column whose type is to be altered, we must
10993+
* capture its definition string before applying any of the column type
10994+
* changes. ruleutils.c will get confused if we ask again later.
10995+
*/
10996+
if (!list_member_oid(tab->changedConstraintOids, conoid))
10997+
{
10998+
/* OK, capture the constraint's existing definition string */
10999+
char *defstring = pg_get_constraintdef_command(conoid);
11000+
11001+
tab->changedConstraintOids = lappend_oid(tab->changedConstraintOids,
11002+
conoid);
11003+
tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
11004+
defstring);
11005+
}
11006+
}
11007+
11008+
/*
11009+
* Subroutine for ATExecAlterColumnType: remember that an index needs
11010+
* to be rebuilt (which we might already know).
11011+
*/
11012+
static void
11013+
RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
11014+
{
11015+
/*
11016+
* This de-duplication check is critical for two independent reasons: we
11017+
* mustn't try to recreate the same index twice, and if an index depends
11018+
* on more than one column whose type is to be altered, we must capture
11019+
* its definition string before applying any of the column type changes.
11020+
* ruleutils.c will get confused if we ask again later.
11021+
*/
11022+
if (!list_member_oid(tab->changedIndexOids, indoid))
11023+
{
11024+
/*
11025+
* Before adding it as an index-to-rebuild, we'd better see if it
11026+
* belongs to a constraint, and if so rebuild the constraint instead.
11027+
* Typically this check fails, because constraint indexes normally
11028+
* have only dependencies on their constraint. But it's possible for
11029+
* such an index to also have direct dependencies on table columns,
11030+
* for example with a partial exclusion constraint.
11031+
*/
11032+
Oidconoid = get_index_constraint(indoid);
11033+
11034+
if (OidIsValid(conoid))
11035+
{
11036+
RememberConstraintForRebuilding(conoid, tab);
11037+
}
11038+
else
11039+
{
11040+
/* OK, capture the index's existing definition string */
11041+
char *defstring = pg_get_indexdef_string(indoid);
11042+
11043+
tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
11044+
indoid);
11045+
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
11046+
defstring);
11047+
}
11048+
}
11049+
}
11050+
1104611051
/*
1104711052
* Returns the address of the modified column
1104811053
*/

‎src/test/regress/expected/alter_table.out

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1900,12 +1900,19 @@ select * from anothertab;
19001900
(3 rows)
19011901

19021902
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);
1903+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1904+
create table anothertab(f1 int primary key, f2 int unique,
1905+
f3 int, f4 int, f5 int);
19051906
alter table anothertab
19061907
add exclude using btree (f3 with =);
19071908
alter table anothertab
19081909
add exclude using btree (f4 with =) where (f4 is not null);
1910+
alter table anothertab
1911+
add exclude using btree (f4 with =) where (f5 > 0);
1912+
alter table anothertab
1913+
add unique(f1,f4);
1914+
create index on anothertab(f2,f3);
1915+
create unique index on anothertab(f4);
19091916
\d anothertab
19101917
Table "public.anothertab"
19111918
Column | Type | Collation | Nullable | Default
@@ -1914,17 +1921,23 @@ alter table anothertab
19141921
f2 | integer | | |
19151922
f3 | integer | | |
19161923
f4 | integer | | |
1924+
f5 | integer | | |
19171925
Indexes:
19181926
"anothertab_pkey" PRIMARY KEY, btree (f1)
1927+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
19191928
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1929+
"anothertab_f4_idx" UNIQUE, btree (f4)
1930+
"anothertab_f2_f3_idx" btree (f2, f3)
19201931
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
19211932
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1933+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
19221934

19231935
alter table anothertab alter column f1 type bigint;
19241936
alter table anothertab
19251937
alter column f2 type bigint,
19261938
alter column f3 type bigint,
19271939
alter column f4 type bigint;
1940+
alter table anothertab alter column f5 type bigint;
19281941
\d anothertab
19291942
Table "public.anothertab"
19301943
Column | Type | Collation | Nullable | Default
@@ -1933,11 +1946,16 @@ alter table anothertab
19331946
f2 | bigint | | |
19341947
f3 | bigint | | |
19351948
f4 | bigint | | |
1949+
f5 | bigint | | |
19361950
Indexes:
19371951
"anothertab_pkey" PRIMARY KEY, btree (f1)
1952+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
19381953
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1954+
"anothertab_f4_idx" UNIQUE, btree (f4)
1955+
"anothertab_f2_f3_idx" btree (f2, f3)
19391956
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
19401957
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1958+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
19411959

19421960
drop table anothertab;
19431961
create table another (f1 int, f2 text);

‎src/test/regress/sql/alter_table.sql

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,19 +1317,27 @@ select * from anothertab;
13171317

13181318
droptable anothertab;
13191319

1320-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1321-
createtableanothertab(f1intprimary key, f2int unique, f3int, f4int);
1320+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1321+
createtableanothertab(f1intprimary key, f2int unique,
1322+
f3int, f4int, f5int);
13221323
altertable anothertab
13231324
add exclude using btree (f3 with=);
13241325
altertable anothertab
13251326
add exclude using btree (f4 with=)where (f4is not null);
1327+
altertable anothertab
1328+
add exclude using btree (f4 with=)where (f5>0);
1329+
altertable anothertab
1330+
add unique(f1,f4);
1331+
createindexon anothertab(f2,f3);
1332+
createunique indexon anothertab(f4);
13261333

13271334
\d anothertab
13281335
altertable anothertab alter column f1 typebigint;
13291336
altertable anothertab
13301337
alter column f2 typebigint,
13311338
alter column f3 typebigint,
13321339
alter column f4 typebigint;
1340+
altertable anothertab alter column f5 typebigint;
13331341
\d anothertab
13341342

13351343
droptable anothertab;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp