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

Commitafaf48a

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 parent40dde82 commitafaf48a

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
@@ -425,6 +425,8 @@ static void ATPrepAlterColumnType(List **wqueue,
425425
staticboolATColumnChangeRequiresRewrite(Node*expr,AttrNumbervarattno);
426426
staticObjectAddressATExecAlterColumnType(AlteredTableInfo*tab,Relationrel,
427427
AlterTableCmd*cmd,LOCKMODElockmode);
428+
staticvoidRememberConstraintForRebuilding(Oidconoid,AlteredTableInfo*tab);
429+
staticvoidRememberIndexForRebuilding(Oidindoid,AlteredTableInfo*tab);
428430
staticObjectAddressATExecAlterColumnGenericOptions(Relationrel,constchar*colName,
429431
List*options,LOCKMODElockmode);
430432
staticvoidATPostAlterTypeCleanup(List**wqueue,AlteredTableInfo*tab,
@@ -9785,9 +9787,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
97859787
SysScanDescscan;
97869788
HeapTupledepTup;
97879789
ObjectAddressaddress;
9788-
ListCell*lc;
9789-
ListCell*prev;
9790-
ListCell*next;
97919790

97929791
/*
97939792
* Clear all the missing values if we're rewriting the table, since this
@@ -9872,11 +9871,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
98729871
* performed all the individual ALTER TYPE operations. We have to save
98739872
* the info before executing ALTER TYPE, though, else the deparser will
98749873
* get confused.
9875-
*
9876-
* There could be multiple entries for the same object, so we must check
9877-
* to ensure we process each one only once. Note: we assume that an index
9878-
* that implements a constraint will not show a direct dependency on the
9879-
* column.
98809874
*/
98819875
depRel=heap_open(DependRelationId,RowExclusiveLock);
98829876

@@ -9918,20 +9912,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
99189912
if (relKind==RELKIND_INDEX||
99199913
relKind==RELKIND_PARTITIONED_INDEX)
99209914
{
9921-
/*
9922-
* Indexes that are directly dependent on the table
9923-
* might be regular indexes or constraint indexes.
9924-
* Constraint indexes typically have only indirect
9925-
* dependencies; but there are exceptions, notably
9926-
* partial exclusion constraints. Hence we must check
9927-
* whether the index depends on any constraint that's
9928-
* due to be rebuilt, which we'll do below after we've
9929-
* found all such constraints.
9930-
*/
99319915
Assert(foundObject.objectSubId==0);
9932-
tab->changedIndexOids=
9933-
list_append_unique_oid(tab->changedIndexOids,
9934-
foundObject.objectId);
9916+
RememberIndexForRebuilding(foundObject.objectId,tab);
99359917
}
99369918
elseif (relKind==RELKIND_SEQUENCE)
99379919
{
@@ -9952,18 +9934,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
99529934

99539935
caseOCLASS_CONSTRAINT:
99549936
Assert(foundObject.objectSubId==0);
9955-
if (!list_member_oid(tab->changedConstraintOids,
9956-
foundObject.objectId))
9957-
{
9958-
char*defstring=pg_get_constraintdef_command(foundObject.objectId);
9959-
9960-
tab->changedConstraintOids=
9961-
lappend_oid(tab->changedConstraintOids,
9962-
foundObject.objectId);
9963-
tab->changedConstraintDefs=
9964-
lappend(tab->changedConstraintDefs,
9965-
defstring);
9966-
}
9937+
RememberConstraintForRebuilding(foundObject.objectId,tab);
99679938
break;
99689939

99699940
caseOCLASS_REWRITE:
@@ -10082,41 +10053,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1008210053

1008310054
systable_endscan(scan);
1008410055

10085-
/*
10086-
* Check the collected index OIDs to see which ones belong to the
10087-
* constraint(s) of the table, and drop those from the list of indexes
10088-
* that we need to process; rebuilding the constraints will handle them.
10089-
*/
10090-
prev=NULL;
10091-
for (lc=list_head(tab->changedIndexOids);lc;lc=next)
10092-
{
10093-
Oidindexoid=lfirst_oid(lc);
10094-
Oidconoid;
10095-
10096-
next=lnext(lc);
10097-
10098-
conoid=get_index_constraint(indexoid);
10099-
if (OidIsValid(conoid)&&
10100-
list_member_oid(tab->changedConstraintOids,conoid))
10101-
tab->changedIndexOids=list_delete_cell(tab->changedIndexOids,
10102-
lc,prev);
10103-
else
10104-
prev=lc;
10105-
}
10106-
10107-
/*
10108-
* Now collect the definitions of the indexes that must be rebuilt. (We
10109-
* could merge this into the previous loop, but it'd be more complicated
10110-
* for little gain.)
10111-
*/
10112-
foreach(lc,tab->changedIndexOids)
10113-
{
10114-
Oidindexoid=lfirst_oid(lc);
10115-
10116-
tab->changedIndexDefs=lappend(tab->changedIndexDefs,
10117-
pg_get_indexdef_string(indexoid));
10118-
}
10119-
1012010056
/*
1012110057
* Now scan for dependencies of this column on other things. The only
1012210058
* thing we should find is the dependency on the column datatype, which we
@@ -10285,6 +10221,75 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
1028510221
returnaddress;
1028610222
}
1028710223

10224+
/*
10225+
* Subroutine for ATExecAlterColumnType: remember that a constraint needs
10226+
* to be rebuilt (which we might already know).
10227+
*/
10228+
staticvoid
10229+
RememberConstraintForRebuilding(Oidconoid,AlteredTableInfo*tab)
10230+
{
10231+
/*
10232+
* This de-duplication check is critical for two independent reasons: we
10233+
* mustn't try to recreate the same constraint twice, and if a constraint
10234+
* depends on more than one column whose type is to be altered, we must
10235+
* capture its definition string before applying any of the column type
10236+
* changes. ruleutils.c will get confused if we ask again later.
10237+
*/
10238+
if (!list_member_oid(tab->changedConstraintOids,conoid))
10239+
{
10240+
/* OK, capture the constraint's existing definition string */
10241+
char*defstring=pg_get_constraintdef_command(conoid);
10242+
10243+
tab->changedConstraintOids=lappend_oid(tab->changedConstraintOids,
10244+
conoid);
10245+
tab->changedConstraintDefs=lappend(tab->changedConstraintDefs,
10246+
defstring);
10247+
}
10248+
}
10249+
10250+
/*
10251+
* Subroutine for ATExecAlterColumnType: remember that an index needs
10252+
* to be rebuilt (which we might already know).
10253+
*/
10254+
staticvoid
10255+
RememberIndexForRebuilding(Oidindoid,AlteredTableInfo*tab)
10256+
{
10257+
/*
10258+
* This de-duplication check is critical for two independent reasons: we
10259+
* mustn't try to recreate the same index twice, and if an index depends
10260+
* on more than one column whose type is to be altered, we must capture
10261+
* its definition string before applying any of the column type changes.
10262+
* ruleutils.c will get confused if we ask again later.
10263+
*/
10264+
if (!list_member_oid(tab->changedIndexOids,indoid))
10265+
{
10266+
/*
10267+
* Before adding it as an index-to-rebuild, we'd better see if it
10268+
* belongs to a constraint, and if so rebuild the constraint instead.
10269+
* Typically this check fails, because constraint indexes normally
10270+
* have only dependencies on their constraint. But it's possible for
10271+
* such an index to also have direct dependencies on table columns,
10272+
* for example with a partial exclusion constraint.
10273+
*/
10274+
Oidconoid=get_index_constraint(indoid);
10275+
10276+
if (OidIsValid(conoid))
10277+
{
10278+
RememberConstraintForRebuilding(conoid,tab);
10279+
}
10280+
else
10281+
{
10282+
/* OK, capture the index's existing definition string */
10283+
char*defstring=pg_get_indexdef_string(indoid);
10284+
10285+
tab->changedIndexOids=lappend_oid(tab->changedIndexOids,
10286+
indoid);
10287+
tab->changedIndexDefs=lappend(tab->changedIndexDefs,
10288+
defstring);
10289+
}
10290+
}
10291+
}
10292+
1028810293
/*
1028910294
* Returns the address of the modified column
1029010295
*/

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

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

19931993
drop table anothertab;
1994-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1995-
create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int);
1994+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1995+
create table anothertab(f1 int primary key, f2 int unique,
1996+
f3 int, f4 int, f5 int);
19961997
alter table anothertab
19971998
add exclude using btree (f3 with =);
19981999
alter table anothertab
19992000
add exclude using btree (f4 with =) where (f4 is not null);
2001+
alter table anothertab
2002+
add exclude using btree (f4 with =) where (f5 > 0);
2003+
alter table anothertab
2004+
add unique(f1,f4);
2005+
create index on anothertab(f2,f3);
2006+
create unique index on anothertab(f4);
20002007
\d anothertab
20012008
Table "public.anothertab"
20022009
Column | Type | Collation | Nullable | Default
@@ -2005,17 +2012,23 @@ alter table anothertab
20052012
f2 | integer | | |
20062013
f3 | integer | | |
20072014
f4 | integer | | |
2015+
f5 | integer | | |
20082016
Indexes:
20092017
"anothertab_pkey" PRIMARY KEY, btree (f1)
2018+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
20102019
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
2020+
"anothertab_f4_idx" UNIQUE, btree (f4)
2021+
"anothertab_f2_f3_idx" btree (f2, f3)
20112022
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
20122023
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
2024+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
20132025

20142026
alter table anothertab alter column f1 type bigint;
20152027
alter table anothertab
20162028
alter column f2 type bigint,
20172029
alter column f3 type bigint,
20182030
alter column f4 type bigint;
2031+
alter table anothertab alter column f5 type bigint;
20192032
\d anothertab
20202033
Table "public.anothertab"
20212034
Column | Type | Collation | Nullable | Default
@@ -2024,11 +2037,16 @@ alter table anothertab
20242037
f2 | bigint | | |
20252038
f3 | bigint | | |
20262039
f4 | bigint | | |
2040+
f5 | bigint | | |
20272041
Indexes:
20282042
"anothertab_pkey" PRIMARY KEY, btree (f1)
2043+
"anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
20292044
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
2045+
"anothertab_f4_idx" UNIQUE, btree (f4)
2046+
"anothertab_f2_f3_idx" btree (f2, f3)
20302047
"anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
20312048
"anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
2049+
"anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
20322050

20332051
drop table anothertab;
20342052
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
@@ -1357,19 +1357,27 @@ select * from anothertab;
13571357

13581358
droptable anothertab;
13591359

1360-
-- Test alter table column type with constraint indexes (cf. bug #15835)
1361-
createtableanothertab(f1intprimary key, f2int unique, f3int, f4int);
1360+
-- Test index handling in alter table column type (cf. bugs #15835, #15865)
1361+
createtableanothertab(f1intprimary key, f2int unique,
1362+
f3int, f4int, f5int);
13621363
altertable anothertab
13631364
add exclude using btree (f3 with=);
13641365
altertable anothertab
13651366
add exclude using btree (f4 with=)where (f4is not null);
1367+
altertable anothertab
1368+
add exclude using btree (f4 with=)where (f5>0);
1369+
altertable anothertab
1370+
add unique(f1,f4);
1371+
createindexon anothertab(f2,f3);
1372+
createunique indexon anothertab(f4);
13661373

13671374
\d anothertab
13681375
altertable anothertab alter column f1 typebigint;
13691376
altertable anothertab
13701377
alter column f2 typebigint,
13711378
alter column f3 typebigint,
13721379
alter column f4 typebigint;
1380+
altertable anothertab alter column f5 typebigint;
13731381
\d anothertab
13741382

13751383
droptable anothertab;

0 commit comments

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