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

Commit1281a5c

Browse files
committed
Restructure ALTER TABLE execution to fix assorted bugs.
We've had numerous bug reports about how (1) IF NOT EXISTS clauses inALTER TABLE don't behave as-expected, and (2) combining certain actionsinto one ALTER TABLE doesn't work, though executing the same actions asseparate statements does. This patch cleans up all of the cases so farreported from the field, though there are still some oddities associatedwith identity columns.The core problem behind all of these bugs is that we do parse analysisof ALTER TABLE subcommands too soon, before starting execution of thestatement. The root of the bugs in group (1) is that parse analysisschedules derived commands (such as a CREATE SEQUENCE for a serialcolumn) before it's known whether the IF NOT EXISTS clause should causea subcommand to be skipped. The root of the bugs in group (2) is thatearlier subcommands may change the catalog state that later subcommandsneed to be parsed against.Hence, postpone parse analysis of ALTER TABLE's subcommands, and dothat one subcommand at a time, during "phase 2" of ALTER TABLE whichis the phase that does catalog rewrites. Thus the catalog effectsof earlier subcommands are already visible when we analyze later ones.(The sole exception is that we do parse analysis for ALTER COLUMN TYPEsubcommands during phase 1, so that their USING expressions can beparsed against the table's original state, which is what we need.Arguably, these bugs stem from falsely concluding that because ALTERCOLUMN TYPE must do early parse analysis, every other command subtypecan too.)This means that ALTER TABLE itself must deal with execution of anynon-ALTER-TABLE derived statements that are generated by parse analysis.Add a suitable entry point to utility.c to accept those recursivecalls, and create a struct to pass through the information needed bythe recursive call, rather than making the argument lists ofAlterTable() and friends even longer.Getting this to work correctly required a little bit of fiddlingwith the subcommand pass structure, in particular breaking upAT_PASS_ADD_CONSTR into multiple passes. But otherwise it's mostlya pretty straightforward application of the above ideas.Fixing the residual issues for identity columns requires refactoring ofwhere the dependency link from an identity column to its sequence getsset up. So that seems like suitable material for a separate patch,especially since this one is pretty big already.Discussion:https://postgr.es/m/10365.1558909428@sss.pgh.pa.us
1 parenta166d40 commit1281a5c

File tree

13 files changed

+827
-267
lines changed

13 files changed

+827
-267
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 387 additions & 107 deletions
Large diffs are not rendered by default.

‎src/backend/commands/view.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace,
145145
* Note that we must do this before updating the query for the view,
146146
* since the rules system requires that the correct view columns be in
147147
* place when defining the new rules.
148+
*
149+
* Also note that ALTER TABLE doesn't run parse transformation on
150+
* AT_AddColumnToView commands. The ColumnDef we supply must be ready
151+
* to execute as-is.
148152
*/
149153
if (list_length(attrList)>rel->rd_att->natts)
150154
{

‎src/backend/parser/parse_utilcmd.c

Lines changed: 77 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,8 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
347347
*/
348348
staticvoid
349349
generateSerialExtraStmts(CreateStmtContext*cxt,ColumnDef*column,
350-
Oidseqtypid,List*seqoptions,boolfor_identity,
350+
Oidseqtypid,List*seqoptions,
351+
boolfor_identity,boolcol_exists,
351352
char**snamespace_p,char**sname_p)
352353
{
353354
ListCell*option;
@@ -472,8 +473,12 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
472473

473474
/*
474475
* Build an ALTER SEQUENCE ... OWNED BY command to mark the sequence as
475-
* owned by this column, and add it to the list of things to be done after
476-
* this CREATE/ALTER TABLE.
476+
* owned by this column, and add it to the appropriate list of things to
477+
* be done along with this CREATE/ALTER TABLE. In a CREATE or ALTER ADD
478+
* COLUMN, it must be done after the statement because we don't know the
479+
* column's attnum yet. But if we do have the attnum (in AT_AddIdentity),
480+
* we can do the marking immediately, which improves some ALTER TABLE
481+
* behaviors.
477482
*/
478483
altseqstmt=makeNode(AlterSeqStmt);
479484
altseqstmt->sequence=makeRangeVar(snamespace,sname,-1);
@@ -484,7 +489,10 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
484489
(Node*)attnamelist,-1));
485490
altseqstmt->for_identity=for_identity;
486491

487-
cxt->alist=lappend(cxt->alist,altseqstmt);
492+
if (col_exists)
493+
cxt->blist=lappend(cxt->blist,altseqstmt);
494+
else
495+
cxt->alist=lappend(cxt->alist,altseqstmt);
488496

489497
if (snamespace_p)
490498
*snamespace_p=snamespace;
@@ -568,7 +576,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
568576
Constraint*constraint;
569577

570578
generateSerialExtraStmts(cxt,column,
571-
column->typeName->typeOid,NIL, false,
579+
column->typeName->typeOid,NIL,
580+
false, false,
572581
&snamespace,&sname);
573582

574583
/*
@@ -684,7 +693,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
684693
constraint->location)));
685694

686695
generateSerialExtraStmts(cxt,column,
687-
typeOid,constraint->options, true,
696+
typeOid,constraint->options,
697+
true, false,
688698
NULL,NULL);
689699

690700
column->identity=constraint->generated_when;
@@ -1086,7 +1096,8 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
10861096
seq_relid=getIdentitySequence(RelationGetRelid(relation),attribute->attnum, false);
10871097
seq_options=sequence_options(seq_relid);
10881098
generateSerialExtraStmts(cxt,def,
1089-
InvalidOid,seq_options, true,
1099+
InvalidOid,seq_options,
1100+
true, false,
10901101
NULL,NULL);
10911102
def->identity=attribute->attidentity;
10921103
}
@@ -2572,7 +2583,7 @@ transformFKConstraints(CreateStmtContext *cxt,
25722583
Constraint*constraint= (Constraint*)lfirst(fkclist);
25732584
AlterTableCmd*altercmd=makeNode(AlterTableCmd);
25742585

2575-
altercmd->subtype=AT_ProcessedConstraint;
2586+
altercmd->subtype=AT_AddConstraint;
25762587
altercmd->name=NULL;
25772588
altercmd->def= (Node*)constraint;
25782589
alterstmt->cmds=lappend(alterstmt->cmds,altercmd);
@@ -3004,23 +3015,23 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
30043015
* transformAlterTableStmt -
30053016
*parse analysis for ALTER TABLE
30063017
*
3007-
* Returnsa List of utility commands to bedone in sequence. One of these
3008-
*will bethe transformed AlterTableStmt, but there may be additional actions
3009-
*to be done before and after the actual AlterTable() call.
3018+
* Returnsthe transformed AlterTableStmt. There may beadditional actions
3019+
*to bedone before and after the transformed statement, which are returned
3020+
*in *beforeStmts and *afterStmts as lists of utility command parsetrees.
30103021
*
30113022
* To avoid race conditions, it's important that this function rely only on
30123023
* the passed-in relid (and not on stmt->relation) to determine the target
30133024
* relation.
30143025
*/
3015-
List*
3026+
AlterTableStmt*
30163027
transformAlterTableStmt(Oidrelid,AlterTableStmt*stmt,
3017-
constchar*queryString)
3028+
constchar*queryString,
3029+
List**beforeStmts,List**afterStmts)
30183030
{
30193031
Relationrel;
30203032
TupleDesctupdesc;
30213033
ParseState*pstate;
30223034
CreateStmtContextcxt;
3023-
List*result;
30243035
List*save_alist;
30253036
ListCell*lcmd,
30263037
*l;
@@ -3052,7 +3063,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
30523063

30533064
/* Set up CreateStmtContext */
30543065
cxt.pstate=pstate;
3055-
if (stmt->relkind==OBJECT_FOREIGN_TABLE)
3066+
if (rel->rd_rel->relkind==RELKIND_FOREIGN_TABLE)
30563067
{
30573068
cxt.stmtType="ALTER FOREIGN TABLE";
30583069
cxt.isforeign= true;
@@ -3080,9 +3091,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
30803091
cxt.ofType= false;
30813092

30823093
/*
3083-
* The only subtypes that currently require parse transformation handling
3084-
* are ADD COLUMN, ADD CONSTRAINT and SET DATA TYPE. These largely re-use
3085-
* code from CREATE TABLE.
3094+
* Transform ALTER subcommands that need it (most don't). These largely
3095+
* re-use code from CREATE TABLE.
30863096
*/
30873097
foreach(lcmd,stmt->cmds)
30883098
{
@@ -3091,7 +3101,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
30913101
switch (cmd->subtype)
30923102
{
30933103
caseAT_AddColumn:
3094-
caseAT_AddColumnToView:
3104+
caseAT_AddColumnRecurse:
30953105
{
30963106
ColumnDef*def=castNode(ColumnDef,cmd->def);
30973107

@@ -3115,6 +3125,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
31153125
}
31163126

31173127
caseAT_AddConstraint:
3128+
caseAT_AddConstraintRecurse:
31183129

31193130
/*
31203131
* The original AddConstraint cmd node doesn't go to newcmds
@@ -3130,19 +3141,9 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
31303141
(int)nodeTag(cmd->def));
31313142
break;
31323143

3133-
caseAT_ProcessedConstraint:
3134-
3135-
/*
3136-
* Already-transformed ADD CONSTRAINT, so just make it look
3137-
* like the standard case.
3138-
*/
3139-
cmd->subtype=AT_AddConstraint;
3140-
newcmds=lappend(newcmds,cmd);
3141-
break;
3142-
31433144
caseAT_AlterColumnType:
31443145
{
3145-
ColumnDef*def= (ColumnDef*)cmd->def;
3146+
ColumnDef*def=castNode(ColumnDef,cmd->def);
31463147
AttrNumberattnum;
31473148

31483149
/*
@@ -3161,13 +3162,13 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
31613162
* change the data type of the sequence.
31623163
*/
31633164
attnum=get_attnum(relid,cmd->name);
3165+
if (attnum==InvalidAttrNumber)
3166+
ereport(ERROR,
3167+
(errcode(ERRCODE_UNDEFINED_COLUMN),
3168+
errmsg("column \"%s\" of relation \"%s\" does not exist",
3169+
cmd->name,RelationGetRelationName(rel))));
31643170

3165-
/*
3166-
* if attribute not found, something will error about it
3167-
* later
3168-
*/
3169-
if (attnum!=InvalidAttrNumber&&
3170-
TupleDescAttr(tupdesc,attnum-1)->attidentity)
3171+
if (TupleDescAttr(tupdesc,attnum-1)->attidentity)
31713172
{
31723173
Oidseq_relid=getIdentitySequence(relid,attnum, false);
31733174
OidtypeOid=typenameTypeId(pstate,def->typeName);
@@ -3196,16 +3197,16 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
31963197
cmd->def= (Node*)newdef;
31973198

31983199
attnum=get_attnum(relid,cmd->name);
3200+
if (attnum==InvalidAttrNumber)
3201+
ereport(ERROR,
3202+
(errcode(ERRCODE_UNDEFINED_COLUMN),
3203+
errmsg("column \"%s\" of relation \"%s\" does not exist",
3204+
cmd->name,RelationGetRelationName(rel))));
31993205

3200-
/*
3201-
* if attribute not found, something will error about it
3202-
* later
3203-
*/
3204-
if (attnum!=InvalidAttrNumber)
3205-
generateSerialExtraStmts(&cxt,newdef,
3206-
get_atttype(relid,attnum),
3207-
def->options, true,
3208-
NULL,NULL);
3206+
generateSerialExtraStmts(&cxt,newdef,
3207+
get_atttype(relid,attnum),
3208+
def->options, true, true,
3209+
NULL,NULL);
32093210

32103211
newcmds=lappend(newcmds,cmd);
32113212
break;
@@ -3221,6 +3222,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
32213222
List*newseqopts=NIL;
32223223
List*newdef=NIL;
32233224
AttrNumberattnum;
3225+
Oidseq_relid;
32243226

32253227
/*
32263228
* Split options into those handled by ALTER SEQUENCE and
@@ -3237,29 +3239,34 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
32373239
}
32383240

32393241
attnum=get_attnum(relid,cmd->name);
3242+
if (attnum==InvalidAttrNumber)
3243+
ereport(ERROR,
3244+
(errcode(ERRCODE_UNDEFINED_COLUMN),
3245+
errmsg("column \"%s\" of relation \"%s\" does not exist",
3246+
cmd->name,RelationGetRelationName(rel))));
32403247

3241-
if (attnum)
3242-
{
3243-
Oidseq_relid=getIdentitySequence(relid,attnum, true);
3248+
seq_relid=getIdentitySequence(relid,attnum, true);
32443249

3245-
if (seq_relid)
3246-
{
3247-
AlterSeqStmt*seqstmt;
3250+
if (seq_relid)
3251+
{
3252+
AlterSeqStmt*seqstmt;
32483253

3249-
seqstmt=makeNode(AlterSeqStmt);
3250-
seqstmt->sequence=makeRangeVar(get_namespace_name(get_rel_namespace(seq_relid)),
3251-
get_rel_name(seq_relid),-1);
3252-
seqstmt->options=newseqopts;
3253-
seqstmt->for_identity= true;
3254-
seqstmt->missing_ok= false;
3254+
seqstmt=makeNode(AlterSeqStmt);
3255+
seqstmt->sequence=makeRangeVar(get_namespace_name(get_rel_namespace(seq_relid)),
3256+
get_rel_name(seq_relid),-1);
3257+
seqstmt->options=newseqopts;
3258+
seqstmt->for_identity= true;
3259+
seqstmt->missing_ok= false;
32553260

3256-
cxt.alist=lappend(cxt.alist,seqstmt);
3257-
}
3261+
cxt.blist=lappend(cxt.blist,seqstmt);
32583262
}
32593263

32603264
/*
3261-
* If column was not found or was not an identity column,
3262-
* we just let the ALTER TABLE command error out later.
3265+
* If column was not an identity column, we just let the
3266+
* ALTER TABLE command error out later. (There are cases
3267+
* this fails to cover, but we'll need to restructure
3268+
* where creation of the sequence dependency linkage
3269+
* happens before we can fix it.)
32633270
*/
32643271

32653272
cmd->def= (Node*)newdef;
@@ -3281,6 +3288,12 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
32813288
break;
32823289

32833290
default:
3291+
3292+
/*
3293+
* Currently, we shouldn't actually get here for subcommand
3294+
* types that don't require transformation; but if we do, just
3295+
* emit them unchanged.
3296+
*/
32843297
newcmds=lappend(newcmds,cmd);
32853298
break;
32863299
}
@@ -3361,11 +3374,10 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
33613374
*/
33623375
stmt->cmds=newcmds;
33633376

3364-
result=lappend(cxt.blist,stmt);
3365-
result=list_concat(result,cxt.alist);
3366-
result=list_concat(result,save_alist);
3377+
*beforeStmts=cxt.blist;
3378+
*afterStmts=list_concat(cxt.alist,save_alist);
33673379

3368-
returnresult;
3380+
returnstmt;
33693381
}
33703382

33713383

0 commit comments

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