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

Commitddfb1b2

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 parent2854e2a commitddfb1b2

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
@@ -372,6 +372,9 @@ static void ATPrepAlterColumnType(List **wqueue,
372372
staticboolATColumnChangeRequiresRewrite(Node*expr,AttrNumbervarattno);
373373
staticvoidATExecAlterColumnType(AlteredTableInfo*tab,Relationrel,
374374
AlterTableCmd*cmd,LOCKMODElockmode);
375+
staticvoidRememberConstraintForRebuilding(Oidconoid,AlteredTableInfo*tab,
376+
DependencyTypedeptype);
377+
staticvoidRememberIndexForRebuilding(Oidindoid,AlteredTableInfo*tab);
375378
staticvoidATExecAlterColumnGenericOptions(Relationrel,constchar*colName,
376379
List*options,LOCKMODElockmode);
377380
staticvoidATPostAlterTypeCleanup(List**wqueue,AlteredTableInfo*tab,LOCKMODElockmode);
@@ -7787,9 +7790,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
77877790
ScanKeyDatakey[3];
77887791
SysScanDescscan;
77897792
HeapTupledepTup;
7790-
ListCell*lc;
7791-
ListCell*prev;
7792-
ListCell*next;
77937793

77947794
attrelation=heap_open(AttributeRelationId,RowExclusiveLock);
77957795

@@ -7858,11 +7858,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
78587858
* performed all the individual ALTER TYPE operations. We have to save
78597859
* the info before executing ALTER TYPE, though, else the deparser will
78607860
* get confused.
7861-
*
7862-
* There could be multiple entries for the same object, so we must check
7863-
* to ensure we process each one only once. Note: we assume that an index
7864-
* that implements a constraint will not show a direct dependency on the
7865-
* column.
78667861
*/
78677862
depRel=heap_open(DependRelationId,RowExclusiveLock);
78687863

@@ -7903,20 +7898,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
79037898

79047899
if (relKind==RELKIND_INDEX)
79057900
{
7906-
/*
7907-
* Indexes that are directly dependent on the table
7908-
* might be regular indexes or constraint indexes.
7909-
* Constraint indexes typically have only indirect
7910-
* dependencies; but there are exceptions, notably
7911-
* partial exclusion constraints. Hence we must check
7912-
* whether the index depends on any constraint that's
7913-
* due to be rebuilt, which we'll do below after we've
7914-
* found all such constraints.
7915-
*/
79167901
Assert(foundObject.objectSubId==0);
7917-
tab->changedIndexOids=
7918-
list_append_unique_oid(tab->changedIndexOids,
7919-
foundObject.objectId);
7902+
RememberIndexForRebuilding(foundObject.objectId,tab);
79207903
}
79217904
elseif (relKind==RELKIND_SEQUENCE)
79227905
{
@@ -7937,39 +7920,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
79377920

79387921
caseOCLASS_CONSTRAINT:
79397922
Assert(foundObject.objectSubId==0);
7940-
if (!list_member_oid(tab->changedConstraintOids,
7941-
foundObject.objectId))
7942-
{
7943-
char*defstring=pg_get_constraintdef_string(foundObject.objectId);
7944-
7945-
/*
7946-
* Put NORMAL dependencies at the front of the list and
7947-
* AUTO dependencies at the back. This makes sure that
7948-
* foreign-key constraints depending on this column will
7949-
* be dropped before unique or primary-key constraints of
7950-
* the column; which we must have because the FK
7951-
* constraints depend on the indexes belonging to the
7952-
* unique constraints.
7953-
*/
7954-
if (foundDep->deptype==DEPENDENCY_NORMAL)
7955-
{
7956-
tab->changedConstraintOids=
7957-
lcons_oid(foundObject.objectId,
7958-
tab->changedConstraintOids);
7959-
tab->changedConstraintDefs=
7960-
lcons(defstring,
7961-
tab->changedConstraintDefs);
7962-
}
7963-
else
7964-
{
7965-
tab->changedConstraintOids=
7966-
lappend_oid(tab->changedConstraintOids,
7967-
foundObject.objectId);
7968-
tab->changedConstraintDefs=
7969-
lappend(tab->changedConstraintDefs,
7970-
defstring);
7971-
}
7972-
}
7923+
RememberConstraintForRebuilding(foundObject.objectId,tab,
7924+
foundDep->deptype);
79737925
break;
79747926

79757927
caseOCLASS_REWRITE:
@@ -8052,41 +8004,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
80528004

80538005
systable_endscan(scan);
80548006

8055-
/*
8056-
* Check the collected index OIDs to see which ones belong to the
8057-
* constraint(s) of the table, and drop those from the list of indexes
8058-
* that we need to process; rebuilding the constraints will handle them.
8059-
*/
8060-
prev=NULL;
8061-
for (lc=list_head(tab->changedIndexOids);lc;lc=next)
8062-
{
8063-
Oidindexoid=lfirst_oid(lc);
8064-
Oidconoid;
8065-
8066-
next=lnext(lc);
8067-
8068-
conoid=get_index_constraint(indexoid);
8069-
if (OidIsValid(conoid)&&
8070-
list_member_oid(tab->changedConstraintOids,conoid))
8071-
tab->changedIndexOids=list_delete_cell(tab->changedIndexOids,
8072-
lc,prev);
8073-
else
8074-
prev=lc;
8075-
}
8076-
8077-
/*
8078-
* Now collect the definitions of the indexes that must be rebuilt. (We
8079-
* could merge this into the previous loop, but it'd be more complicated
8080-
* for little gain.)
8081-
*/
8082-
foreach(lc,tab->changedIndexOids)
8083-
{
8084-
Oidindexoid=lfirst_oid(lc);
8085-
8086-
tab->changedIndexDefs=lappend(tab->changedIndexDefs,
8087-
pg_get_indexdef_string(indexoid));
8088-
}
8089-
80908007
/*
80918008
* Now scan for dependencies of this column on other things. The only
80928009
* thing we should find is the dependency on the column datatype, which we
@@ -8188,6 +8105,95 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
81888105
heap_freetuple(heapTup);
81898106
}
81908107

8108+
/*
8109+
* Subroutine for ATExecAlterColumnType: remember that a constraint needs
8110+
* to be rebuilt (which we might already know).
8111+
*/
8112+
staticvoid
8113+
RememberConstraintForRebuilding(Oidconoid,AlteredTableInfo*tab,
8114+
DependencyTypedeptype)
8115+
{
8116+
/*
8117+
* This de-duplication check is critical for two independent reasons: we
8118+
* mustn't try to recreate the same constraint twice, and if a constraint
8119+
* depends on more than one column whose type is to be altered, we must
8120+
* capture its definition string before applying any of the column type
8121+
* changes. ruleutils.c will get confused if we ask again later.
8122+
*/
8123+
if (!list_member_oid(tab->changedConstraintOids,conoid))
8124+
{
8125+
/* OK, capture the constraint's existing definition string */
8126+
char*defstring=pg_get_constraintdef_string(conoid);
8127+
8128+
/*
8129+
* Put NORMAL dependencies at the front of the list and AUTO
8130+
* dependencies at the back. This makes sure that foreign-key
8131+
* constraints depending on this column will be dropped before unique
8132+
* or primary-key constraints of the column; which we must have
8133+
* because the FK constraints depend on the indexes belonging to the
8134+
* unique constraints.
8135+
*/
8136+
if (deptype==DEPENDENCY_NORMAL)
8137+
{
8138+
tab->changedConstraintOids=lcons_oid(conoid,
8139+
tab->changedConstraintOids);
8140+
tab->changedConstraintDefs=lcons(defstring,
8141+
tab->changedConstraintDefs);
8142+
}
8143+
else
8144+
{
8145+
tab->changedConstraintOids=lappend_oid(tab->changedConstraintOids,
8146+
conoid);
8147+
tab->changedConstraintDefs=lappend(tab->changedConstraintDefs,
8148+
defstring);
8149+
}
8150+
}
8151+
}
8152+
8153+
/*
8154+
* Subroutine for ATExecAlterColumnType: remember that an index needs
8155+
* to be rebuilt (which we might already know).
8156+
*/
8157+
staticvoid
8158+
RememberIndexForRebuilding(Oidindoid,AlteredTableInfo*tab)
8159+
{
8160+
/*
8161+
* This de-duplication check is critical for two independent reasons: we
8162+
* mustn't try to recreate the same index twice, and if an index depends
8163+
* on more than one column whose type is to be altered, we must capture
8164+
* its definition string before applying any of the column type changes.
8165+
* ruleutils.c will get confused if we ask again later.
8166+
*/
8167+
if (!list_member_oid(tab->changedIndexOids,indoid))
8168+
{
8169+
/*
8170+
* Before adding it as an index-to-rebuild, we'd better see if it
8171+
* belongs to a constraint, and if so rebuild the constraint instead.
8172+
* Typically this check fails, because constraint indexes normally
8173+
* have only dependencies on their constraint. But it's possible for
8174+
* such an index to also have direct dependencies on table columns,
8175+
* for example with a partial exclusion constraint.
8176+
*/
8177+
Oidconoid=get_index_constraint(indoid);
8178+
8179+
if (OidIsValid(conoid))
8180+
{
8181+
/* index dependencies on columns should generally be AUTO */
8182+
RememberConstraintForRebuilding(conoid,tab,DEPENDENCY_AUTO);
8183+
}
8184+
else
8185+
{
8186+
/* OK, capture the index's existing definition string */
8187+
char*defstring=pg_get_indexdef_string(indoid);
8188+
8189+
tab->changedIndexOids=lappend_oid(tab->changedIndexOids,
8190+
indoid);
8191+
tab->changedIndexDefs=lappend(tab->changedIndexDefs,
8192+
defstring);
8193+
}
8194+
}
8195+
}
8196+
81918197
staticvoid
81928198
ATExecAlterColumnGenericOptions(Relationrel,
81938199
constchar*colName,

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

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

19011901
drop table anothertab;
1902-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1903-
create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
1902+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1903+
create table anothertab(f1 int primary key, f2 int unique,
1904+
f3 int, f4 int, f5 int);
19041905
alter table anothertab
19051906
add exclude using btree (f3 with =);
19061907
alter table anothertab
19071908
add exclude using btree (f4 with =) where (f4 is not null);
1909+
alter table anothertab
1910+
add exclude using btree (f4 with =) where (f5 > 0);
1911+
alter table anothertab
1912+
add unique(f1,f4);
1913+
create index on anothertab(f2,f3);
1914+
create unique index on anothertab(f4);
19081915
\d anothertab
19091916
Table "public.anothertab"
19101917
Column | Type | Modifiers
@@ -1913,17 +1920,23 @@ alter table anothertab
19131920
f2 | integer |
19141921
f3 | integer |
19151922
f4 | integer |
1923+
f5 | integer |
19161924
Indexes:
19171925
"anothertab_pkey" PRIMARY KEY, btree (f1)
1926+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
19181927
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1928+
"anothertab_f4_idx" UNIQUE, btree (f4)
1929+
"anothertab_f2_f3_idx" btree (f2, f3)
19191930
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
19201931
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1932+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
19211933

19221934
alter table anothertab alter column f1 type bigint;
19231935
alter table anothertab
19241936
alter column f2 type bigint,
19251937
alter column f3 type bigint,
19261938
alter column f4 type bigint;
1939+
alter table anothertab alter column f5 type bigint;
19271940
\d anothertab
19281941
Table "public.anothertab"
19291942
Column | Type | Modifiers
@@ -1932,11 +1945,16 @@ alter table anothertab
19321945
f2 | bigint |
19331946
f3 | bigint |
19341947
f4 | bigint |
1948+
f5 | bigint |
19351949
Indexes:
19361950
"anothertab_pkey" PRIMARY KEY, btree (f1)
1951+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
19371952
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
1953+
"anothertab_f4_idx" UNIQUE, btree (f4)
1954+
"anothertab_f2_f3_idx" btree (f2, f3)
19381955
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
19391956
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
1957+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
19401958

19411959
drop table anothertab;
19421960
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
@@ -1292,19 +1292,27 @@ select * from anothertab;
12921292

12931293
droptable anothertab;
12941294

1295-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1296-
createtableanothertab(f1intprimary key, f2int unique, f3int, f4int);
1295+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1296+
createtableanothertab(f1intprimary key, f2int unique,
1297+
f3int, f4int, f5int);
12971298
altertable anothertab
12981299
add exclude using btree (f3 with=);
12991300
altertable anothertab
13001301
add exclude using btree (f4 with=)where (f4is not null);
1302+
altertable anothertab
1303+
add exclude using btree (f4 with=)where (f5>0);
1304+
altertable anothertab
1305+
add unique(f1,f4);
1306+
createindexon anothertab(f2,f3);
1307+
createunique indexon anothertab(f4);
13011308

13021309
\d anothertab
13031310
altertable anothertab alter column f1 typebigint;
13041311
altertable anothertab
13051312
alter column f2 typebigint,
13061313
alter column f3 typebigint,
13071314
alter column f4 typebigint;
1315+
altertable anothertab alter column f5 typebigint;
13081316
\d anothertab
13091317

13101318
droptable anothertab;

0 commit comments

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