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

Commitcb8962c

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 parent05399b1 commitcb8962c

File tree

3 files changed

+125
-93
lines changed

3 files changed

+125
-93
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 95 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,9 @@ static void ATPrepAlterColumnType(List **wqueue,
415415
staticboolATColumnChangeRequiresRewrite(Node*expr,AttrNumbervarattno);
416416
staticObjectAddressATExecAlterColumnType(AlteredTableInfo*tab,Relationrel,
417417
AlterTableCmd*cmd,LOCKMODElockmode);
418+
staticvoidRememberConstraintForRebuilding(Oidconoid,AlteredTableInfo*tab,
419+
DependencyTypedeptype);
420+
staticvoidRememberIndexForRebuilding(Oidindoid,AlteredTableInfo*tab);
418421
staticObjectAddressATExecAlterColumnGenericOptions(Relationrel,constchar*colName,
419422
List*options,LOCKMODElockmode);
420423
staticvoidATPostAlterTypeCleanup(List**wqueue,AlteredTableInfo*tab,
@@ -9021,9 +9024,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
90219024
SysScanDescscan;
90229025
HeapTupledepTup;
90239026
ObjectAddressaddress;
9024-
ListCell*lc;
9025-
ListCell*prev;
9026-
ListCell*next;
90279027

90289028
attrelation=heap_open(AttributeRelationId,RowExclusiveLock);
90299029

@@ -9092,11 +9092,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
90929092
* performed all the individual ALTER TYPE operations. We have to save
90939093
* the info before executing ALTER TYPE, though, else the deparser will
90949094
* get confused.
9095-
*
9096-
* There could be multiple entries for the same object, so we must check
9097-
* to ensure we process each one only once. Note: we assume that an index
9098-
* that implements a constraint will not show a direct dependency on the
9099-
* column.
91009095
*/
91019096
depRel=heap_open(DependRelationId,RowExclusiveLock);
91029097

@@ -9137,20 +9132,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
91379132

91389133
if (relKind==RELKIND_INDEX)
91399134
{
9140-
/*
9141-
* Indexes that are directly dependent on the table
9142-
* might be regular indexes or constraint indexes.
9143-
* Constraint indexes typically have only indirect
9144-
* dependencies; but there are exceptions, notably
9145-
* partial exclusion constraints. Hence we must check
9146-
* whether the index depends on any constraint that's
9147-
* due to be rebuilt, which we'll do below after we've
9148-
* found all such constraints.
9149-
*/
91509135
Assert(foundObject.objectSubId==0);
9151-
tab->changedIndexOids=
9152-
list_append_unique_oid(tab->changedIndexOids,
9153-
foundObject.objectId);
9136+
RememberIndexForRebuilding(foundObject.objectId,tab);
91549137
}
91559138
elseif (relKind==RELKIND_SEQUENCE)
91569139
{
@@ -9171,39 +9154,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
91719154

91729155
caseOCLASS_CONSTRAINT:
91739156
Assert(foundObject.objectSubId==0);
9174-
if (!list_member_oid(tab->changedConstraintOids,
9175-
foundObject.objectId))
9176-
{
9177-
char*defstring=pg_get_constraintdef_command(foundObject.objectId);
9178-
9179-
/*
9180-
* Put NORMAL dependencies at the front of the list and
9181-
* AUTO dependencies at the back. This makes sure that
9182-
* foreign-key constraints depending on this column will
9183-
* be dropped before unique or primary-key constraints of
9184-
* the column; which we must have because the FK
9185-
* constraints depend on the indexes belonging to the
9186-
* unique constraints.
9187-
*/
9188-
if (foundDep->deptype==DEPENDENCY_NORMAL)
9189-
{
9190-
tab->changedConstraintOids=
9191-
lcons_oid(foundObject.objectId,
9192-
tab->changedConstraintOids);
9193-
tab->changedConstraintDefs=
9194-
lcons(defstring,
9195-
tab->changedConstraintDefs);
9196-
}
9197-
else
9198-
{
9199-
tab->changedConstraintOids=
9200-
lappend_oid(tab->changedConstraintOids,
9201-
foundObject.objectId);
9202-
tab->changedConstraintDefs=
9203-
lappend(tab->changedConstraintDefs,
9204-
defstring);
9205-
}
9206-
}
9157+
RememberConstraintForRebuilding(foundObject.objectId,tab,
9158+
foundDep->deptype);
92079159
break;
92089160

92099161
caseOCLASS_REWRITE:
@@ -9322,41 +9274,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
93229274

93239275
systable_endscan(scan);
93249276

9325-
/*
9326-
* Check the collected index OIDs to see which ones belong to the
9327-
* constraint(s) of the table, and drop those from the list of indexes
9328-
* that we need to process; rebuilding the constraints will handle them.
9329-
*/
9330-
prev=NULL;
9331-
for (lc=list_head(tab->changedIndexOids);lc;lc=next)
9332-
{
9333-
Oidindexoid=lfirst_oid(lc);
9334-
Oidconoid;
9335-
9336-
next=lnext(lc);
9337-
9338-
conoid=get_index_constraint(indexoid);
9339-
if (OidIsValid(conoid)&&
9340-
list_member_oid(tab->changedConstraintOids,conoid))
9341-
tab->changedIndexOids=list_delete_cell(tab->changedIndexOids,
9342-
lc,prev);
9343-
else
9344-
prev=lc;
9345-
}
9346-
9347-
/*
9348-
* Now collect the definitions of the indexes that must be rebuilt. (We
9349-
* could merge this into the previous loop, but it'd be more complicated
9350-
* for little gain.)
9351-
*/
9352-
foreach(lc,tab->changedIndexOids)
9353-
{
9354-
Oidindexoid=lfirst_oid(lc);
9355-
9356-
tab->changedIndexDefs=lappend(tab->changedIndexDefs,
9357-
pg_get_indexdef_string(indexoid));
9358-
}
9359-
93609277
/*
93619278
* Now scan for dependencies of this column on other things. The only
93629279
* thing we should find is the dependency on the column datatype, which we
@@ -9460,6 +9377,95 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
94609377
returnaddress;
94619378
}
94629379

9380+
/*
9381+
* Subroutine for ATExecAlterColumnType: remember that a constraint needs
9382+
* to be rebuilt (which we might already know).
9383+
*/
9384+
staticvoid
9385+
RememberConstraintForRebuilding(Oidconoid,AlteredTableInfo*tab,
9386+
DependencyTypedeptype)
9387+
{
9388+
/*
9389+
* This de-duplication check is critical for two independent reasons: we
9390+
* mustn't try to recreate the same constraint twice, and if a constraint
9391+
* depends on more than one column whose type is to be altered, we must
9392+
* capture its definition string before applying any of the column type
9393+
* changes. ruleutils.c will get confused if we ask again later.
9394+
*/
9395+
if (!list_member_oid(tab->changedConstraintOids,conoid))
9396+
{
9397+
/* OK, capture the constraint's existing definition string */
9398+
char*defstring=pg_get_constraintdef_command(conoid);
9399+
9400+
/*
9401+
* Put NORMAL dependencies at the front of the list and AUTO
9402+
* dependencies at the back. This makes sure that foreign-key
9403+
* constraints depending on this column will be dropped before unique
9404+
* or primary-key constraints of the column; which we must have
9405+
* because the FK constraints depend on the indexes belonging to the
9406+
* unique constraints.
9407+
*/
9408+
if (deptype==DEPENDENCY_NORMAL)
9409+
{
9410+
tab->changedConstraintOids=lcons_oid(conoid,
9411+
tab->changedConstraintOids);
9412+
tab->changedConstraintDefs=lcons(defstring,
9413+
tab->changedConstraintDefs);
9414+
}
9415+
else
9416+
{
9417+
tab->changedConstraintOids=lappend_oid(tab->changedConstraintOids,
9418+
conoid);
9419+
tab->changedConstraintDefs=lappend(tab->changedConstraintDefs,
9420+
defstring);
9421+
}
9422+
}
9423+
}
9424+
9425+
/*
9426+
* Subroutine for ATExecAlterColumnType: remember that an index needs
9427+
* to be rebuilt (which we might already know).
9428+
*/
9429+
staticvoid
9430+
RememberIndexForRebuilding(Oidindoid,AlteredTableInfo*tab)
9431+
{
9432+
/*
9433+
* This de-duplication check is critical for two independent reasons: we
9434+
* mustn't try to recreate the same index twice, and if an index depends
9435+
* on more than one column whose type is to be altered, we must capture
9436+
* its definition string before applying any of the column type changes.
9437+
* ruleutils.c will get confused if we ask again later.
9438+
*/
9439+
if (!list_member_oid(tab->changedIndexOids,indoid))
9440+
{
9441+
/*
9442+
* Before adding it as an index-to-rebuild, we'd better see if it
9443+
* belongs to a constraint, and if so rebuild the constraint instead.
9444+
* Typically this check fails, because constraint indexes normally
9445+
* have only dependencies on their constraint. But it's possible for
9446+
* such an index to also have direct dependencies on table columns,
9447+
* for example with a partial exclusion constraint.
9448+
*/
9449+
Oidconoid=get_index_constraint(indoid);
9450+
9451+
if (OidIsValid(conoid))
9452+
{
9453+
/* index dependencies on columns should generally be AUTO */
9454+
RememberConstraintForRebuilding(conoid,tab,DEPENDENCY_AUTO);
9455+
}
9456+
else
9457+
{
9458+
/* OK, capture the index's existing definition string */
9459+
char*defstring=pg_get_indexdef_string(indoid);
9460+
9461+
tab->changedIndexOids=lappend_oid(tab->changedIndexOids,
9462+
indoid);
9463+
tab->changedIndexDefs=lappend(tab->changedIndexDefs,
9464+
defstring);
9465+
}
9466+
}
9467+
}
9468+
94639469
/*
94649470
* Returns the address of the modified column
94659471
*/

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

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

19361936
drop table anothertab;
1937-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1938-
create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
1937+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1938+
create table anothertab(f1 int primary key, f2 int unique,
1939+
f3 int, f4 int, f5 int);
19391940
alter table anothertab
19401941
add exclude using btree (f3 with =);
19411942
alter table anothertab
19421943
add exclude using btree (f4 with =) where (f4 is not null);
1944+
alter table anothertab
1945+
add exclude using btree (f4 with =) where (f5 > 0);
1946+
alter table anothertab
1947+
add unique(f1,f4);
1948+
create index on anothertab(f2,f3);
1949+
create unique index on anothertab(f4);
19431950
\d anothertab
19441951
Table "public.anothertab"
19451952
Column | Type | Collation | Nullable | Default
@@ -1948,17 +1955,23 @@ alter table anothertab
19481955
f2 | integer | | |
19491956
f3 | integer | | |
19501957
f4 | integer | | |
1958+
f5 | integer | | |
19511959
Indexes:
19521960
"anothertab_pkey" PRIMARY KEY, btree (f1)
1961+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
19531962
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1963+
"anothertab_f4_idx" UNIQUE, btree (f4)
1964+
"anothertab_f2_f3_idx" btree (f2, f3)
19541965
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
19551966
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1967+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
19561968

19571969
alter table anothertab alter column f1 type bigint;
19581970
alter table anothertab
19591971
alter column f2 type bigint,
19601972
alter column f3 type bigint,
19611973
alter column f4 type bigint;
1974+
alter table anothertab alter column f5 type bigint;
19621975
\d anothertab
19631976
Table "public.anothertab"
19641977
Column | Type | Collation | Nullable | Default
@@ -1967,11 +1980,16 @@ alter table anothertab
19671980
f2 | bigint | | |
19681981
f3 | bigint | | |
19691982
f4 | bigint | | |
1983+
f5 | bigint | | |
19701984
Indexes:
19711985
"anothertab_pkey" PRIMARY KEY, btree (f1)
1986+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
19721987
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1988+
"anothertab_f4_idx" UNIQUE, btree (f4)
1989+
"anothertab_f2_f3_idx" btree (f2, f3)
19731990
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
19741991
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1992+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
19751993

19761994
drop table anothertab;
19771995
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
@@ -1307,19 +1307,27 @@ select * from anothertab;
13071307

13081308
droptable anothertab;
13091309

1310-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1311-
createtableanothertab(f1intprimary key, f2int unique, f3int, f4int);
1310+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1311+
createtableanothertab(f1intprimary key, f2int unique,
1312+
f3int, f4int, f5int);
13121313
altertable anothertab
13131314
add exclude using btree (f3 with=);
13141315
altertable anothertab
13151316
add exclude using btree (f4 with=)where (f4is not null);
1317+
altertable anothertab
1318+
add exclude using btree (f4 with=)where (f5>0);
1319+
altertable anothertab
1320+
add unique(f1,f4);
1321+
createindexon anothertab(f2,f3);
1322+
createunique indexon anothertab(f4);
13161323

13171324
\d anothertab
13181325
altertable anothertab alter column f1 typebigint;
13191326
altertable anothertab
13201327
alter column f2 typebigint,
13211328
alter column f3 typebigint,
13221329
alter column f4 typebigint;
1330+
altertable anothertab alter column f5 typebigint;
13231331
\d anothertab
13241332

13251333
droptable anothertab;

0 commit comments

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