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

Commit6c57239

Browse files
committed
Rearrange "add column" logic to merge columns at exec time.
The previous coding set attinhcount too high in some cases, resulting inan undumpable, undroppable column. Per bug #5856, reported by NaoyaAnzai. See also commit31b6fc0, whichfixes a similar bug in ALTER TABLE .. ADD CONSTRAINT.Patch by Noah Misch.
1 parentcabf5d8 commit6c57239

File tree

4 files changed

+119
-73
lines changed

4 files changed

+119
-73
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 88 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -285,16 +285,15 @@ static void ATSimplePermissions(Relation rel, int allowed_targets);
285285
staticvoidATWrongRelkindError(Relationrel,intallowed_targets);
286286
staticvoidATSimpleRecursion(List**wqueue,Relationrel,
287287
AlterTableCmd*cmd,boolrecurse,LOCKMODElockmode);
288-
staticvoidATOneLevelRecursion(List**wqueue,Relationrel,
289-
AlterTableCmd*cmd,LOCKMODElockmode);
290288
staticvoidATTypedTableRecursion(List**wqueue,Relationrel,AlterTableCmd*cmd,
291289
LOCKMODElockmode);
292290
staticList*find_typed_table_dependencies(OidtypeOid,constchar*typeName,
293291
DropBehaviorbehavior);
294292
staticvoidATPrepAddColumn(List**wqueue,Relationrel,boolrecurse,boolrecursing,
295293
AlterTableCmd*cmd,LOCKMODElockmode);
296-
staticvoidATExecAddColumn(AlteredTableInfo*tab,Relationrel,
297-
ColumnDef*colDef,boolisOid,LOCKMODElockmode);
294+
staticvoidATExecAddColumn(List**wqueue,AlteredTableInfo*tab,Relationrel,
295+
ColumnDef*colDef,boolisOid,
296+
boolrecurse,boolrecursing,LOCKMODElockmode);
298297
staticvoidadd_column_datatype_dependency(Oidrelid,int32attnum,Oidtypid,Oidcollid);
299298
staticvoidATPrepAddOids(List**wqueue,Relationrel,boolrecurse,
300299
AlterTableCmd*cmd,LOCKMODElockmode);
@@ -2775,15 +2774,15 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
27752774
caseAT_AddColumn:/* ADD COLUMN */
27762775
ATSimplePermissions(rel,
27772776
ATT_TABLE|ATT_COMPOSITE_TYPE|ATT_FOREIGN_TABLE);
2778-
/* Performs own recursion */
27792777
ATPrepAddColumn(wqueue,rel,recurse,recursing,cmd,lockmode);
2778+
/* Recursion occurs during execution phase */
27802779
pass=AT_PASS_ADD_COL;
27812780
break;
27822781
caseAT_AddColumnToView:/* add column via CREATE OR REPLACE
27832782
* VIEW */
27842783
ATSimplePermissions(rel,ATT_VIEW);
2785-
/* Performs own recursion */
27862784
ATPrepAddColumn(wqueue,rel,recurse,recursing,cmd,lockmode);
2785+
/* Recursion occurs during execution phase */
27872786
pass=AT_PASS_ADD_COL;
27882787
break;
27892788
caseAT_ColumnDefault:/* ALTER COLUMN DEFAULT */
@@ -2885,9 +2884,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
28852884
break;
28862885
caseAT_AddOids:/* SET WITH OIDS */
28872886
ATSimplePermissions(rel,ATT_TABLE);
2888-
/* Performs own recursion */
28892887
if (!rel->rd_rel->relhasoids||recursing)
28902888
ATPrepAddOids(wqueue,rel,recurse,cmd,lockmode);
2889+
/* Recursion occurs during execution phase */
28912890
pass=AT_PASS_ADD_COL;
28922891
break;
28932892
caseAT_DropOids:/* SET WITHOUT OIDS */
@@ -3043,7 +3042,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
30433042
caseAT_AddColumn:/* ADD COLUMN */
30443043
caseAT_AddColumnToView:/* add column via CREATE OR REPLACE
30453044
* VIEW */
3046-
ATExecAddColumn(tab,rel, (ColumnDef*)cmd->def, false,lockmode);
3045+
ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3046+
false, false, false,lockmode);
3047+
break;
3048+
caseAT_AddColumnRecurse:
3049+
ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3050+
false, true, false,lockmode);
30473051
break;
30483052
caseAT_ColumnDefault:/* ALTER COLUMN DEFAULT */
30493053
ATExecColumnDefault(rel,cmd->name,cmd->def,lockmode);
@@ -3121,7 +3125,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
31213125
caseAT_AddOids:/* SET WITH OIDS */
31223126
/* Use the ADD COLUMN code, unless prep decided to do nothing */
31233127
if (cmd->def!=NULL)
3124-
ATExecAddColumn(tab,rel, (ColumnDef*)cmd->def, true,lockmode);
3128+
ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3129+
true, false, false,lockmode);
3130+
break;
3131+
caseAT_AddOidsRecurse:/* SET WITH OIDS */
3132+
/* Use the ADD COLUMN code, unless prep decided to do nothing */
3133+
if (cmd->def!=NULL)
3134+
ATExecAddColumn(wqueue,tab,rel, (ColumnDef*)cmd->def,
3135+
true, true, false,lockmode);
31253136
break;
31263137
caseAT_DropOids:/* SET WITHOUT OIDS */
31273138

@@ -3842,37 +3853,6 @@ ATSimpleRecursion(List **wqueue, Relation rel,
38423853
}
38433854
}
38443855

3845-
/*
3846-
* ATOneLevelRecursion
3847-
*
3848-
* Here, we visit only direct inheritance children. It is expected that
3849-
* the command's prep routine will recurse again to find indirect children.
3850-
* When using this technique, a multiply-inheriting child will be visited
3851-
* multiple times.
3852-
*/
3853-
staticvoid
3854-
ATOneLevelRecursion(List**wqueue,Relationrel,
3855-
AlterTableCmd*cmd,LOCKMODElockmode)
3856-
{
3857-
Oidrelid=RelationGetRelid(rel);
3858-
ListCell*child;
3859-
List*children;
3860-
3861-
children=find_inheritance_children(relid,lockmode);
3862-
3863-
foreach(child,children)
3864-
{
3865-
Oidchildrelid=lfirst_oid(child);
3866-
Relationchildrel;
3867-
3868-
/* find_inheritance_children already got lock */
3869-
childrel=relation_open(childrelid,NoLock);
3870-
CheckTableNotInUse(childrel,"ALTER TABLE");
3871-
ATPrepCmd(wqueue,childrel,cmd, true, true,lockmode);
3872-
relation_close(childrel,NoLock);
3873-
}
3874-
}
3875-
38763856
/*
38773857
* ATTypedTableRecursion
38783858
*
@@ -4060,6 +4040,12 @@ find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior be
40604040
* CHECK, NOT NULL, and FOREIGN KEY constraints will be removed from the
40614041
* AT_AddColumn AlterTableCmd by parse_utilcmd.c and added as independent
40624042
* AlterTableCmd's.
4043+
*
4044+
* ADD COLUMN cannot use the normal ALTER TABLE recursion mechanism, because we
4045+
* have to decide at runtime whether to recurse or not depending on whether we
4046+
* actually add a column or merely merge with an existing column. (We can't
4047+
* check this in a static pre-pass because it won't handle multiple inheritance
4048+
* situations correctly.)
40634049
*/
40644050
staticvoid
40654051
ATPrepAddColumn(List**wqueue,Relationrel,boolrecurse,boolrecursing,
@@ -4070,43 +4056,17 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
40704056
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
40714057
errmsg("cannot add column to typed table")));
40724058

4073-
/*
4074-
* Recurse to add the column to child classes, if requested.
4075-
*
4076-
* We must recurse one level at a time, so that multiply-inheriting
4077-
* children are visited the right number of times and end up with the
4078-
* right attinhcount.
4079-
*/
4080-
if (recurse)
4081-
{
4082-
AlterTableCmd*childCmd=copyObject(cmd);
4083-
ColumnDef*colDefChild= (ColumnDef*)childCmd->def;
4084-
4085-
/* Child should see column as singly inherited */
4086-
colDefChild->inhcount=1;
4087-
colDefChild->is_local= false;
4088-
4089-
ATOneLevelRecursion(wqueue,rel,childCmd,lockmode);
4090-
}
4091-
else
4092-
{
4093-
/*
4094-
* If we are told not to recurse, there had better not be any child
4095-
* tables; else the addition would put them out of step.
4096-
*/
4097-
if (find_inheritance_children(RelationGetRelid(rel),NoLock)!=NIL)
4098-
ereport(ERROR,
4099-
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
4100-
errmsg("column must be added to child tables too")));
4101-
}
4102-
41034059
if (rel->rd_rel->relkind==RELKIND_COMPOSITE_TYPE)
41044060
ATTypedTableRecursion(wqueue,rel,cmd,lockmode);
4061+
4062+
if (recurse)
4063+
cmd->subtype=AT_AddColumnRecurse;
41054064
}
41064065

41074066
staticvoid
4108-
ATExecAddColumn(AlteredTableInfo*tab,Relationrel,
4109-
ColumnDef*colDef,boolisOid,LOCKMODElockmode)
4067+
ATExecAddColumn(List**wqueue,AlteredTableInfo*tab,Relationrel,
4068+
ColumnDef*colDef,boolisOid,
4069+
boolrecurse,boolrecursing,LOCKMODElockmode)
41104070
{
41114071
Oidmyrelid=RelationGetRelid(rel);
41124072
Relationpgclass,
@@ -4121,12 +4081,20 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
41214081
OidcollOid;
41224082
Form_pg_typetform;
41234083
Expr*defval;
4084+
List*children;
4085+
ListCell*child;
4086+
4087+
/* At top level, permission check was done in ATPrepCmd, else do it */
4088+
if (recursing)
4089+
ATSimplePermissions(rel,ATT_TABLE);
41244090

41254091
attrdesc=heap_open(AttributeRelationId,RowExclusiveLock);
41264092

41274093
/*
41284094
* Are we adding the column to a recursion child? If so, check whether to
4129-
* merge with an existing definition for the column.
4095+
* merge with an existing definition for the column. If we do merge,
4096+
* we must not recurse. Children will already have the column, and
4097+
* recursing into them would mess up attinhcount.
41304098
*/
41314099
if (colDef->inhcount>0)
41324100
{
@@ -4389,6 +4357,50 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
43894357
* Add needed dependency entries for the new column.
43904358
*/
43914359
add_column_datatype_dependency(myrelid,newattnum,attribute.atttypid,attribute.attcollation);
4360+
4361+
/*
4362+
* Propagate to children as appropriate. Unlike most other ALTER
4363+
* routines, we have to do this one level of recursion at a time; we can't
4364+
* use find_all_inheritors to do it in one pass.
4365+
*/
4366+
children=find_inheritance_children(RelationGetRelid(rel),lockmode);
4367+
4368+
/*
4369+
* If we are told not to recurse, there had better not be any child
4370+
* tables; else the addition would put them out of step.
4371+
*/
4372+
if (children&& !recurse)
4373+
ereport(ERROR,
4374+
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
4375+
errmsg("column must be added to child tables too")));
4376+
4377+
/* Children should see column as singly inherited */
4378+
if (!recursing)
4379+
{
4380+
colDef=copyObject(colDef);
4381+
colDef->inhcount=1;
4382+
colDef->is_local= false;
4383+
}
4384+
4385+
foreach(child,children)
4386+
{
4387+
Oidchildrelid=lfirst_oid(child);
4388+
Relationchildrel;
4389+
AlteredTableInfo*childtab;
4390+
4391+
/* find_inheritance_children already got lock */
4392+
childrel=heap_open(childrelid,NoLock);
4393+
CheckTableNotInUse(childrel,"ALTER TABLE");
4394+
4395+
/* Find or create work queue entry for this table */
4396+
childtab=ATGetQueueEntry(wqueue,childrel);
4397+
4398+
/* Recurse to child */
4399+
ATExecAddColumn(wqueue,childtab,childrel,
4400+
colDef,isOid,recurse, true,lockmode);
4401+
4402+
heap_close(childrel,NoLock);
4403+
}
43924404
}
43934405

43944406
/*
@@ -4440,6 +4452,9 @@ ATPrepAddOids(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd, LOC
44404452
cmd->def= (Node*)cdef;
44414453
}
44424454
ATPrepAddColumn(wqueue,rel,recurse, false,cmd,lockmode);
4455+
4456+
if (recurse)
4457+
cmd->subtype=AT_AddOidsRecurse;
44434458
}
44444459

44454460
/*

‎src/include/nodes/parsenodes.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,7 @@ typedef struct AlterTableStmt
11731173
typedefenumAlterTableType
11741174
{
11751175
AT_AddColumn,/* add column */
1176+
AT_AddColumnRecurse,/* internal to commands/tablecmds.c */
11761177
AT_AddColumnToView,/* implicitly via CREATE OR REPLACE VIEW */
11771178
AT_ColumnDefault,/* alter column default */
11781179
AT_DropNotNull,/* alter column drop not null */
@@ -1198,6 +1199,7 @@ typedef enum AlterTableType
11981199
AT_ClusterOn,/* CLUSTER ON */
11991200
AT_DropCluster,/* SET WITHOUT CLUSTER */
12001201
AT_AddOids,/* SET WITH OIDS */
1202+
AT_AddOidsRecurse,/* internal to commands/tablecmds.c */
12011203
AT_DropOids,/* SET WITHOUT OIDS */
12021204
AT_SetTableSpace,/* SET TABLESPACE */
12031205
AT_SetRelOptions,/* SET (...) -- AM specific parameters */

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,23 @@ drop table p1, p2 cascade;
11981198
NOTICE: drop cascades to 2 other objects
11991199
DETAIL: drop cascades to table c1
12001200
drop cascades to table gc1
1201+
-- test attinhcount tracking with merged columns
1202+
create table depth0();
1203+
create table depth1(c text) inherits (depth0);
1204+
create table depth2() inherits (depth1);
1205+
alter table depth0 add c text;
1206+
NOTICE: merging definition of column "c" for child "depth1"
1207+
select attrelid::regclass, attname, attinhcount, attislocal
1208+
from pg_attribute
1209+
where attnum > 0 and attrelid::regclass in ('depth0', 'depth1', 'depth2')
1210+
order by attrelid::regclass::text, attnum;
1211+
attrelid | attname | attinhcount | attislocal
1212+
----------+---------+-------------+------------
1213+
depth0 | c | 0 | t
1214+
depth1 | c | 1 | t
1215+
depth2 | c | 1 | f
1216+
(3 rows)
1217+
12011218
--
12021219
-- Test the ALTER TABLE SET WITH/WITHOUT OIDS command
12031220
--

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,18 @@ order by relname, attnum;
944944

945945
droptable p1, p2 cascade;
946946

947+
-- test attinhcount tracking with merged columns
948+
949+
createtabledepth0();
950+
createtabledepth1(ctext) inherits (depth0);
951+
createtabledepth2() inherits (depth1);
952+
altertable depth0 add ctext;
953+
954+
select attrelid::regclass, attname, attinhcount, attislocal
955+
from pg_attribute
956+
where attnum>0and attrelid::regclassin ('depth0','depth1','depth2')
957+
order by attrelid::regclass::text, attnum;
958+
947959
--
948960
-- Test the ALTER TABLE SET WITH/WITHOUT OIDS command
949961
--

0 commit comments

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