Re: Rearranging ALTER TABLE to avoid multi-operations bugs
Tom Lane <[email protected]> Tue, 14 Jan 2020 17:27:01 -0500
| Newsgroups | gmane.comp.db.postgresql.devel.general |
|---|---|
| Message-ID | <[email protected]> |
------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <[email protected]> I wrote: > [ fix-alter-table-order-of-operations-3.patch ] Rebased again, fixing a minor conflict with f595117e2. > I'd kind of like to get this cleared out of my queue soon. > Does anyone intend to review it further? If I don't hear objections pretty darn quick, I'm going to go ahead and push this. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="fix-alter-table-order-of-operations-4.patch"; charset="us-ascii" Content-ID: <[email protected]> Content-Description: fix-alter-table-order-of-operations-4.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/table= cmds.c index 2ec3fc5..4e44ebb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -85,6 +85,7 @@ #include "storage/lock.h" #include "storage/predicate.h" #include "storage/smgr.h" +#include "tcop/utility.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" @@ -142,11 +143,13 @@ static List *on_commits =3D NIL; #define AT_PASS_OLD_CONSTR 3 /* re-add existing constraints */ /* We could support a RENAME COLUMN pass here, but not currently used */ #define AT_PASS_ADD_COL 4 /* ADD COLUMN */ -#define AT_PASS_COL_ATTRS 5 /* set other column attributes */ -#define AT_PASS_ADD_INDEX 6 /* ADD indexes */ -#define AT_PASS_ADD_CONSTR 7 /* ADD constraints, defaults */ -#define AT_PASS_MISC 8 /* other stuff */ -#define AT_NUM_PASSES 9 +#define AT_PASS_ADD_CONSTR 5 /* ADD constraints (initial examination) */ +#define AT_PASS_COL_ATTRS 6 /* set column attributes, eg NOT NULL */ +#define AT_PASS_ADD_INDEXCONSTR 7 /* ADD index-based constraints */ +#define AT_PASS_ADD_INDEX 8 /* ADD indexes */ +#define AT_PASS_ADD_OTHERCONSTR 9 /* ADD other constraints, defaults */ +#define AT_PASS_MISC 10 /* other stuff */ +#define AT_NUM_PASSES 11 = typedef struct AlteredTableInfo { @@ -159,6 +162,7 @@ typedef struct AlteredTableInfo /* Information saved by Phases 1/2 for Phase 3: */ List *constraints; /* List of NewConstraint */ List *newvals; /* List of NewColumnValue */ + List *afterStmts; /* List of utility command parsetrees */ bool verify_new_notnull; /* T if we should recheck NOT NULL */ int rewrite; /* Reason for forced rewrite, if any */ Oid newTableSpace; /* new tablespace; 0 means no change */ @@ -340,31 +344,45 @@ static void validateForeignKeyConstraint(char *conna= me, Relation rel, Relation pkrel, Oid pkindOid, Oid constraintOid); static void ATController(AlterTableStmt *parsetree, - Relation rel, List *cmds, bool recurse, LOCKMODE lockmode); + Relation rel, List *cmds, bool recurse, LOCKMODE lockmode, + AlterTableUtilityContext *context); static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, - bool recurse, bool recursing, LOCKMODE lockmode); -static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode); + bool recurse, bool recursing, LOCKMODE lockmode, + AlterTableUtilityContext *context); +static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, + AlterTableUtilityContext *context); static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, - AlterTableCmd *cmd, LOCKMODE lockmode); + AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass, + AlterTableUtilityContext *context); +static AlterTableCmd *ATParseTransformCmd(List **wqueue, AlteredTableInfo= *tab, + Relation rel, AlterTableCmd *cmd, + bool recurse, LOCKMODE lockmode, + int cur_pass, + AlterTableUtilityContext *context); static void ATRewriteTables(AlterTableStmt *parsetree, - List **wqueue, LOCKMODE lockmode); + List **wqueue, LOCKMODE lockmode, + AlterTableUtilityContext *context); static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMOD= E lockmode); static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel); static void ATSimplePermissions(Relation rel, int allowed_targets); static void ATWrongRelkindError(Relation rel, int allowed_targets); static void ATSimpleRecursion(List **wqueue, Relation rel, - AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode); + AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode, + AlterTableUtilityContext *context); static void ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode); static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTable= Cmd *cmd, - LOCKMODE lockmode); + LOCKMODE lockmode, + AlterTableUtilityContext *context); static List *find_typed_table_dependencies(Oid typeOid, const char *typeN= ame, DropBehavior behavior); static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bo= ol recursing, - bool is_view, AlterTableCmd *cmd, LOCKMODE lockmode); + bool is_view, AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context); static ObjectAddress ATExecAddColumn(List **wqueue, AlteredTableInfo *tab= , - Relation rel, ColumnDef *colDef, + Relation rel, AlterTableCmd **cmd, bool recurse, bool recursing, - bool if_not_exists, LOCKMODE lockmode); + LOCKMODE lockmode, int cur_pass, + AlterTableUtilityContext *context); static bool check_for_column_name_collision(Relation rel, const char *col= name, bool if_not_exists); static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid t= ypid); @@ -373,7 +391,8 @@ static void ATPrepDropNotNull(Relation rel, bool recur= se, bool recursing); static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName,= LOCKMODE lockmode); static void ATPrepSetNotNull(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, - LOCKMODE lockmode); + LOCKMODE lockmode, + AlterTableUtilityContext *context); static ObjectAddress ATExecSetNotNull(AlteredTableInfo *tab, Relation rel= , const char *colName, LOCKMODE lockmode); static void ATExecCheckNotNull(AlteredTableInfo *tab, Relation rel, @@ -397,7 +416,8 @@ static ObjectAddress ATExecSetOptions(Relation rel, co= nst char *colName, static ObjectAddress ATExecSetStorage(Relation rel, const char *colName, Node *newValue, LOCKMODE lockmode); static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, b= ool recursing, - AlterTableCmd *cmd, LOCKMODE lockmode); + AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context); static ObjectAddress ATExecDropColumn(List **wqueue, Relation rel, const = char *colName, DropBehavior behavior, bool recurse, bool recursing, @@ -454,7 +474,8 @@ static void ATExecDropConstraint(Relation rel, const c= har *constrName, static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, - AlterTableCmd *cmd, LOCKMODE lockmode); + AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context); static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno= ); static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relatio= n rel, AlterTableCmd *cmd, LOCKMODE lockmode); @@ -3463,7 +3484,7 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKM= ODE lockmode) * * ALTER TABLE is performed in three phases: * 1. Examine subcommands and perform pre-transformation checking. - * 2. Update system catalogs. + * 2. Validate and transform subcommands, and update system catalogs. * 3. Scan table(s) to check new constraints, and optionally recopy * the data into new table(s). * Phase 3 is not performed unless one or more of the subcommands require= s @@ -3474,9 +3495,10 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCK= MODE lockmode) * ATPrepCmd performs phase 1. A "work queue" entry is created for * each table to be affected (there may be multiple affected tables if th= e * commands traverse a table inheritance hierarchy). Also we do prelimin= ary - * validation of the subcommands, including parse transformation of those - * expressions that need to be evaluated with respect to the old table - * schema. + * validation of the subcommands. Because earlier subcommands may change + * the catalog state seen by later commands, there are limits to what can + * be done in this phase. Generally, this phase acquires table locks, + * checks permissions and relkind, and recurses to find child tables. * * ATRewriteCatalogs performs phase 2 for each affected table. (Note tha= t * phases 2 and 3 normally do no explicit recursion, since phase 1 alread= y @@ -3498,18 +3520,23 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOC= KMODE lockmode) * lock level we want as we recurse might well be higher than required fo= r * that specific subcommand. So we pass down the overall lock requirement= , * rather than reassess it at lower levels. + * + * The caller also provides a "context" which is to be passed back to + * utility.c when we need to execute a subcommand such as CREATE INDEX. + * Some of the fields therein, such as the relid, are used here as well. */ void -AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *stmt) +AlterTable(AlterTableStmt *stmt, LOCKMODE lockmode, + AlterTableUtilityContext *context) { Relation rel; = /* Caller is required to provide an adequate lock. */ - rel =3D relation_open(relid, NoLock); + rel =3D relation_open(context->relid, NoLock); = CheckTableNotInUse(rel, "ALTER TABLE"); = - ATController(stmt, rel, stmt->cmds, stmt->relation->inh, lockmode); + ATController(stmt, rel, stmt->cmds, stmt->relation->inh, lockmode, conte= xt); } = /* @@ -3522,6 +3549,10 @@ AlterTable(Oid relid, LOCKMODE lockmode, AlterTable= Stmt *stmt) * is unsafe to use this entry point for alterations that could break * existing query plans. On the assumption it's not used for such, we * don't have to reject pending AFTER triggers, either. + * + * Also, since we don't have an AlterTableUtilityContext, this cannot be + * used for any subcommand types that require parse transformation or + * could generate subcommands that have to be passed to ProcessUtility. */ void AlterTableInternal(Oid relid, List *cmds, bool recurse) @@ -3533,7 +3564,7 @@ AlterTableInternal(Oid relid, List *cmds, bool recur= se) = EventTriggerAlterTableRelid(relid); = - ATController(NULL, rel, cmds, recurse, lockmode); + ATController(NULL, rel, cmds, recurse, lockmode, NULL); } = /* @@ -3679,7 +3710,6 @@ AlterTableGetLockLevel(List *cmds) break; = case AT_AddConstraint: - case AT_ProcessedConstraint: /* becomes AT_AddConstraint */ case AT_AddConstraintRecurse: /* becomes AT_AddConstraint */ case AT_ReAddConstraint: /* becomes AT_AddConstraint */ case AT_ReAddDomainConstraint: /* becomes AT_AddConstraint */ @@ -3832,7 +3862,8 @@ AlterTableGetLockLevel(List *cmds) */ static void ATController(AlterTableStmt *parsetree, - Relation rel, List *cmds, bool recurse, LOCKMODE lockmode) + Relation rel, List *cmds, bool recurse, LOCKMODE lockmode, + AlterTableUtilityContext *context) { List *wqueue =3D NIL; ListCell *lcmd; @@ -3842,17 +3873,17 @@ ATController(AlterTableStmt *parsetree, { AlterTableCmd *cmd =3D (AlterTableCmd *) lfirst(lcmd); = - ATPrepCmd(&wqueue, rel, cmd, recurse, false, lockmode); + ATPrepCmd(&wqueue, rel, cmd, recurse, false, lockmode, context); } = /* Close the relation, but keep lock until commit */ relation_close(rel, NoLock); = /* Phase 2: update system catalogs */ - ATRewriteCatalogs(&wqueue, lockmode); + ATRewriteCatalogs(&wqueue, lockmode, context); = - /* Phase 3: scan/rewrite tables as needed */ - ATRewriteTables(parsetree, &wqueue, lockmode); + /* Phase 3: scan/rewrite tables as needed, and run afterStmts */ + ATRewriteTables(parsetree, &wqueue, lockmode, context); } = /* @@ -3866,7 +3897,8 @@ ATController(AlterTableStmt *parsetree, */ static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, - bool recurse, bool recursing, LOCKMODE lockmode) + bool recurse, bool recursing, LOCKMODE lockmode, + AlterTableUtilityContext *context) { AlteredTableInfo *tab; int pass =3D AT_PASS_UNSET; @@ -3878,13 +3910,17 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableC= md *cmd, * Copy the original subcommand for each table. This avoids conflicts * when different child tables need to make different parse * transformations (for example, the same column may have different colu= mn - * numbers in different children). + * numbers in different children). It also ensures that we don't corrup= t + * the original parse tree, in case it is saved in plancache. */ cmd =3D copyObject(cmd); = /* - * Do permissions checking, recursion to child tables if needed, and any - * additional phase-1 processing needed. + * Do permissions and relkind checking, recursion to child tables if + * needed, and any additional phase-1 processing needed. (But beware of + * adding any processing that looks at table details that another + * subcommand could change. In some cases we reject multiple subcommand= s + * that could try to change the same state in contrary ways.) */ switch (cmd->subtype) { @@ -3892,14 +3928,14 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableC= md *cmd, ATSimplePermissions(rel, ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE); ATPrepAddColumn(wqueue, rel, recurse, recursing, false, cmd, - lockmode); + lockmode, context); /* Recursion occurs during execution phase */ pass =3D AT_PASS_ADD_COL; break; case AT_AddColumnToView: /* add column via CREATE OR REPLACE VIEW */ ATSimplePermissions(rel, ATT_VIEW); ATPrepAddColumn(wqueue, rel, recurse, recursing, true, cmd, - lockmode); + lockmode, context); /* Recursion occurs during execution phase */ pass =3D AT_PASS_ADD_COL; break; @@ -3912,19 +3948,20 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableC= md *cmd, * rules. */ ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); /* No command-specific prep needed */ - pass =3D cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP; + pass =3D cmd->def ? AT_PASS_ADD_OTHERCONSTR : AT_PASS_DROP; break; case AT_AddIdentity: ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE); /* This command never recurses */ - pass =3D AT_PASS_ADD_CONSTR; + pass =3D AT_PASS_ADD_OTHERCONSTR; break; case AT_SetIdentity: ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE); /* This command never recurses */ - pass =3D AT_PASS_COL_ATTRS; + /* This should run after AddIdentity, so do it in MISC pass */ + pass =3D AT_PASS_MISC; break; case AT_DropIdentity: ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE); @@ -3934,30 +3971,31 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableC= md *cmd, case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); ATPrepDropNotNull(rel, recurse, recursing); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); pass =3D AT_PASS_DROP; break; case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */ ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); /* Need command-specific recursion decision */ - ATPrepSetNotNull(wqueue, rel, cmd, recurse, recursing, lockmode); + ATPrepSetNotNull(wqueue, rel, cmd, recurse, recursing, + lockmode, context); pass =3D AT_PASS_COL_ATTRS; break; case AT_CheckNotNull: /* check column is already marked NOT NULL */ ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); /* No command-specific prep needed */ pass =3D AT_PASS_COL_ATTRS; break; case AT_DropExpression: /* ALTER COLUMN DROP EXPRESSION */ ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); ATPrepDropExpression(rel, cmd, recursing); pass =3D AT_PASS_DROP; break; case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PAR= TITIONED_INDEX | ATT_FOREIGN_TABLE); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); /* No command-specific prep needed */ pass =3D AT_PASS_MISC; break; @@ -3969,14 +4007,15 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableC= md *cmd, break; case AT_SetStorage: /* ALTER COLUMN SET STORAGE */ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE); - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); /* No command-specific prep needed */ pass =3D AT_PASS_MISC; break; case AT_DropColumn: /* DROP COLUMN */ ATSimplePermissions(rel, ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE); - ATPrepDropColumn(wqueue, rel, recurse, recursing, cmd, lockmode); + ATPrepDropColumn(wqueue, rel, recurse, recursing, cmd, + lockmode, context); /* Recursion occurs during execution phase */ pass =3D AT_PASS_DROP; break; @@ -3998,7 +4037,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd= *cmd, ATSimplePermissions(rel, ATT_TABLE); /* This command never recurses */ /* No command-specific prep needed */ - pass =3D AT_PASS_ADD_CONSTR; + pass =3D AT_PASS_ADD_INDEXCONSTR; break; case AT_DropConstraint: /* DROP CONSTRAINT */ ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); @@ -4012,8 +4051,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCm= d *cmd, case AT_AlterColumnType: /* ALTER COLUMN TYPE */ ATSimplePermissions(rel, ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE); + /* See comments for ATPrepAlterColumnType */ + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, recurse, lockmode, + AT_PASS_UNSET, context); + Assert(cmd !=3D NULL); /* Performs own recursion */ - ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd, lockm= ode); + ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd, + lockmode, context); pass =3D AT_PASS_ALTER_TYPE; break; case AT_AlterColumnGenericOptions: @@ -4036,6 +4080,10 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCm= d *cmd, break; case AT_SetLogged: /* SET LOGGED */ ATSimplePermissions(rel, ATT_TABLE); + if (tab->chgPersistence) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change persistence setting twice"))); tab->chgPersistence =3D ATPrepChangePersistence(rel, true); /* force rewrite if necessary; see comment in ATRewriteTables */ if (tab->chgPersistence) @@ -4047,6 +4095,10 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCm= d *cmd, break; case AT_SetUnLogged: /* SET UNLOGGED */ ATSimplePermissions(rel, ATT_TABLE); + if (tab->chgPersistence) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot change persistence setting twice"))); tab->chgPersistence =3D ATPrepChangePersistence(rel, false); /* force rewrite if necessary; see comment in ATRewriteTables */ if (tab->chgPersistence) @@ -4166,7 +4218,8 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd= *cmd, * conflicts). */ static void -ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode) +ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, + AlterTableUtilityContext *context) { int pass; ListCell *ltab; @@ -4199,7 +4252,7 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode) foreach(lcmd, subcmds) ATExecCmd(wqueue, tab, rel, castNode(AlterTableCmd, lfirst(lcmd)), - lockmode); + lockmode, pass, context); = /* * After the ALTER TYPE pass, do cleanup work (this is not done in @@ -4236,7 +4289,8 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode) */ static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, - AlterTableCmd *cmd, LOCKMODE lockmode) + AlterTableCmd *cmd, LOCKMODE lockmode, int cur_pass, + AlterTableUtilityContext *context) { ObjectAddress address =3D InvalidObjectAddress; = @@ -4244,22 +4298,28 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Re= lation rel, { case AT_AddColumn: /* ADD COLUMN */ case AT_AddColumnToView: /* add column via CREATE OR REPLACE VIEW */ - address =3D ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def, + address =3D ATExecAddColumn(wqueue, tab, rel, &cmd, false, false, - cmd->missing_ok, lockmode); + lockmode, cur_pass, context); break; case AT_AddColumnRecurse: - address =3D ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def, + address =3D ATExecAddColumn(wqueue, tab, rel, &cmd, true, false, - cmd->missing_ok, lockmode); + lockmode, cur_pass, context); break; case AT_ColumnDefault: /* ALTER COLUMN DEFAULT */ address =3D ATExecColumnDefault(rel, cmd->name, cmd->def, lockmode); break; case AT_AddIdentity: + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, + cur_pass, context); + Assert(cmd !=3D NULL); address =3D ATExecAddIdentity(rel, cmd->name, cmd->def, lockmode); break; case AT_SetIdentity: + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, + cur_pass, context); + Assert(cmd !=3D NULL); address =3D ATExecSetIdentity(rel, cmd->name, cmd->def, lockmode); break; case AT_DropIdentity: @@ -4310,14 +4370,24 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Re= lation rel, lockmode); break; case AT_AddConstraint: /* ADD CONSTRAINT */ - address =3D - ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, - false, false, lockmode); + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, + cur_pass, context); + /* Might not have gotten AddConstraint back from parse transform */ + if (cmd !=3D NULL) + address =3D + ATExecAddConstraint(wqueue, tab, rel, + (Constraint *) cmd->def, + false, false, lockmode); break; case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */ - address =3D - ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, - true, false, lockmode); + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, true, lockmode, + cur_pass, context); + /* Might not have gotten AddConstraint back from parse transform */ + if (cmd !=3D NULL) + address =3D + ATExecAddConstraint(wqueue, tab, rel, + (Constraint *) cmd->def, + true, false, lockmode); break; case AT_ReAddConstraint: /* Re-add pre-existing check constraint */ address =3D @@ -4361,6 +4431,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Rela= tion rel, cmd->missing_ok, lockmode); break; case AT_AlterColumnType: /* ALTER COLUMN TYPE */ + /* parse transformation was done earlier */ address =3D ATExecAlterColumnType(tab, rel, cmd, lockmode); break; case AT_AlterColumnGenericOptions: /* ALTER COLUMN OPTIONS */ @@ -4483,6 +4554,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Rela= tion rel, ATExecGenericOptions(rel, (List *) cmd->def); break; case AT_AttachPartition: + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, + cur_pass, context); + Assert(cmd !=3D NULL); if (rel->rd_rel->relkind =3D=3D RELKIND_PARTITIONED_TABLE) ATExecAttachPartition(wqueue, rel, (PartitionCmd *) cmd->def); else @@ -4490,6 +4564,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Rela= tion rel, ((PartitionCmd *) cmd->def)->name); break; case AT_DetachPartition: + cmd =3D ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode, + cur_pass, context); + Assert(cmd !=3D NULL); /* ATPrepCmd ensures it must be a table */ Assert(rel->rd_rel->relkind =3D=3D RELKIND_PARTITIONED_TABLE); ATExecDetachPartition(rel, ((PartitionCmd *) cmd->def)->name); @@ -4503,7 +4580,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Rela= tion rel, /* * Report the subcommand to interested event triggers. */ - EventTriggerCollectAlterTableSubcmd((Node *) cmd, address); + if (cmd) + EventTriggerCollectAlterTableSubcmd((Node *) cmd, address); = /* * Bump the command counter to ensure the next subcommand in the sequenc= e @@ -4513,10 +4591,143 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, R= elation rel, } = /* + * ATParseTransformCmd: perform parse transformation for one subcommand + * + * Returns the transformed subcommand tree, if there is one, else NULL. + * + * The parser may hand back additional AlterTableCmd(s) and/or other + * utility statements, either before or after the original subcommand. + * Other AlterTableCmds are scheduled into the appropriate slot of the + * AlteredTableInfo (they had better be for later passes than the current= one). + * Utility statements that are supposed to happen before the AlterTableCm= d + * are executed immediately. Those that are supposed to happen afterward= s + * are added to the tab->afterStmts list to be done at the very end. + */ +static AlterTableCmd * +ATParseTransformCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, + AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode, + int cur_pass, AlterTableUtilityContext *context) +{ + AlterTableCmd *newcmd =3D NULL; + AlterTableStmt *atstmt =3D makeNode(AlterTableStmt); + List *beforeStmts; + List *afterStmts; + ListCell *lc; + + /* Gin up an AlterTableStmt with just this subcommand and this table */ + atstmt->relation =3D + makeRangeVar(get_namespace_name(RelationGetNamespace(rel)), + pstrdup(RelationGetRelationName(rel)), + -1); + atstmt->relation->inh =3D recurse; + atstmt->cmds =3D list_make1(cmd); + atstmt->relkind =3D OBJECT_TABLE; /* needn't be picky here */ + atstmt->missing_ok =3D false; + + /* Transform the AlterTableStmt */ + atstmt =3D transformAlterTableStmt(RelationGetRelid(rel), + atstmt, + context->queryString, + &beforeStmts, + &afterStmts); + + /* Execute any statements that should happen before these subcommand(s) = */ + foreach(lc, beforeStmts) + { + Node *stmt =3D (Node *) lfirst(lc); + + ProcessUtilityForAlterTable(stmt, context); + CommandCounterIncrement(); + } + + /* Examine the transformed subcommands and schedule them appropriately *= / + foreach(lc, atstmt->cmds) + { + AlterTableCmd *cmd2 =3D lfirst_node(AlterTableCmd, lc); + + if (newcmd =3D=3D NULL && + (cmd->subtype =3D=3D cmd2->subtype || + (cmd->subtype =3D=3D AT_AddConstraintRecurse && + cmd2->subtype =3D=3D AT_AddConstraint))) + { + /* Found the transformed version of our subcommand */ + cmd2->subtype =3D cmd->subtype; /* copy recursion flag */ + newcmd =3D cmd2; + } + else + { + int pass; + + /* + * Schedule added subcommand appropriately. We assume we needn't + * do any phase-1 checks for it. This switch only has to cover + * the subcommand types that can be added by parse_utilcmd.c. + */ + switch (cmd2->subtype) + { + case AT_SetNotNull: + /* Need command-specific recursion decision */ + ATPrepSetNotNull(wqueue, rel, cmd2, + recurse, false, + lockmode, context); + pass =3D AT_PASS_COL_ATTRS; + break; + case AT_AddIndex: + /* This command never recurses */ + /* No command-specific prep needed */ + pass =3D AT_PASS_ADD_INDEX; + break; + case AT_AddIndexConstraint: + /* This command never recurses */ + /* No command-specific prep needed */ + pass =3D AT_PASS_ADD_INDEXCONSTR; + break; + case AT_AddConstraint: + /* Recursion occurs during execution phase */ + if (recurse) + cmd2->subtype =3D AT_AddConstraintRecurse; + switch (castNode(Constraint, cmd2->def)->contype) + { + case CONSTR_PRIMARY: + case CONSTR_UNIQUE: + case CONSTR_EXCLUSION: + pass =3D AT_PASS_ADD_INDEXCONSTR; + break; + default: + pass =3D AT_PASS_ADD_OTHERCONSTR; + break; + } + break; + case AT_AlterColumnGenericOptions: + /* This command never recurses */ + /* No command-specific prep needed */ + pass =3D AT_PASS_MISC; + break; + default: + elog(ERROR, "unexpected AlterTableType: %d", + (int) cmd2->subtype); + pass =3D AT_PASS_UNSET; + break; + } + /* Must be for a later pass than we're currently doing */ + if (pass <=3D cur_pass) + elog(ERROR, "ALTER TABLE scheduling failure"); + tab->subcmds[pass] =3D lappend(tab->subcmds[pass], cmd2); + } + } + + /* Queue up any after-statements to happen at the end */ + tab->afterStmts =3D list_concat(tab->afterStmts, afterStmts); + + return newcmd; +} + +/* * ATRewriteTables: ALTER TABLE phase 3 */ static void -ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmo= de) +ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmo= de, + AlterTableUtilityContext *context) { ListCell *ltab; = @@ -4743,6 +4954,21 @@ ATRewriteTables(AlterTableStmt *parsetree, List **w= queue, LOCKMODE lockmode) if (rel) table_close(rel, NoLock); } + + /* Finally, run any afterStmts that were queued up */ + foreach(ltab, *wqueue) + { + AlteredTableInfo *tab =3D (AlteredTableInfo *) lfirst(ltab); + ListCell *lc; + + foreach(lc, tab->afterStmts) + { + Node *stmt =3D (Node *) lfirst(lc); + + ProcessUtilityForAlterTable(stmt, context); + CommandCounterIncrement(); + } + } } = /* @@ -5279,7 +5505,8 @@ ATWrongRelkindError(Relation rel, int allowed_target= s) */ static void ATSimpleRecursion(List **wqueue, Relation rel, - AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode) + AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode, + AlterTableUtilityContext *context) { /* * Propagate to children if desired. Only plain tables, foreign tables @@ -5312,7 +5539,7 @@ ATSimpleRecursion(List **wqueue, Relation rel, /* find_all_inheritors already got lock */ childrel =3D relation_open(childrelid, NoLock); CheckTableNotInUse(childrel, "ALTER TABLE"); - ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode); + ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode, context); relation_close(childrel, NoLock); } } @@ -5357,7 +5584,7 @@ ATCheckPartitionsNotInUse(Relation rel, LOCKMODE loc= kmode) */ static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, - LOCKMODE lockmode) + LOCKMODE lockmode, AlterTableUtilityContext *context) { ListCell *child; List *children; @@ -5375,7 +5602,7 @@ ATTypedTableRecursion(List **wqueue, Relation rel, A= lterTableCmd *cmd, = childrel =3D relation_open(childrelid, lockmode); CheckTableNotInUse(childrel, "ALTER TABLE"); - ATPrepCmd(wqueue, childrel, cmd, true, true, lockmode); + ATPrepCmd(wqueue, childrel, cmd, true, true, lockmode, context); relation_close(childrel, NoLock); } } @@ -5612,7 +5839,8 @@ check_of_type(HeapTuple typetuple) */ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing= , - bool is_view, AlterTableCmd *cmd, LOCKMODE lockmode) + bool is_view, AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context) { if (rel->rd_rel->reloftype && !recursing) ereport(ERROR, @@ -5620,7 +5848,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool re= curse, bool recursing, errmsg("cannot add column to typed table"))); = if (rel->rd_rel->relkind =3D=3D RELKIND_COMPOSITE_TYPE) - ATTypedTableRecursion(wqueue, rel, cmd, lockmode); + ATTypedTableRecursion(wqueue, rel, cmd, lockmode, context); = if (recurse && !is_view) cmd->subtype =3D AT_AddColumnRecurse; @@ -5629,14 +5857,20 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool = recurse, bool recursing, /* * Add a column to a table. The return value is the address of the * new column in the parent relation. + * + * cmd is pass-by-ref so that we can replace it with the parse-transforme= d + * copy (but that happens only after we check for IF NOT EXISTS). */ static ObjectAddress ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, - ColumnDef *colDef, + AlterTableCmd **cmd, bool recurse, bool recursing, - bool if_not_exists, LOCKMODE lockmode) + LOCKMODE lockmode, int cur_pass, + AlterTableUtilityContext *context) { Oid myrelid =3D RelationGetRelid(rel); + ColumnDef *colDef =3D castNode(ColumnDef, (*cmd)->def); + bool if_not_exists =3D (*cmd)->missing_ok; Relation pgclass, attrdesc; HeapTuple reltup; @@ -5651,6 +5885,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab= , Relation rel, Expr *defval; List *children; ListCell *child; + AlterTableCmd *childcmd; AclResult aclresult; ObjectAddress address; = @@ -5718,12 +5953,31 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *t= ab, Relation rel, } } = - pgclass =3D table_open(RelationRelationId, RowExclusiveLock); + /* skip if the name already exists and if_not_exists is true */ + if (!check_for_column_name_collision(rel, colDef->colname, if_not_exists= )) + { + table_close(attrdesc, RowExclusiveLock); + return InvalidObjectAddress; + } = - reltup =3D SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid)); - if (!HeapTupleIsValid(reltup)) - elog(ERROR, "cache lookup failed for relation %u", myrelid); - relkind =3D ((Form_pg_class) GETSTRUCT(reltup))->relkind; + /* + * Okay, we need to add the column, so go ahead and do parse + * transformation. This can result in queueing up, or even immediately + * executing, subsidiary operations (such as creation of unique indexes)= ; + * so we mustn't do it until we have made the if_not_exists check. + * + * When recursing, the command was already transformed and we needn't do + * so again. Also, if context isn't given we can't transform. (That + * currently happens only for AT_AddColumnToView; we expect that view.c + * passed us a ColumnDef that doesn't need work.) + */ + if (context !=3D NULL && !recursing) + { + *cmd =3D ATParseTransformCmd(wqueue, tab, rel, *cmd, recurse, lockmode, + cur_pass, context); + Assert(*cmd !=3D NULL); + colDef =3D castNode(ColumnDef, (*cmd)->def); + } = /* * Cannot add identity column if table has children, because identity do= es @@ -5736,14 +5990,12 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *t= ab, Relation rel, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot recursively add identity column to table that has chi= ld tables"))); = - /* skip if the name already exists and if_not_exists is true */ - if (!check_for_column_name_collision(rel, colDef->colname, if_not_exists= )) - { - table_close(attrdesc, RowExclusiveLock); - heap_freetuple(reltup); - table_close(pgclass, RowExclusiveLock); - return InvalidObjectAddress; - } + pgclass =3D table_open(RelationRelationId, RowExclusiveLock); + + reltup =3D SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid)); + if (!HeapTupleIsValid(reltup)) + elog(ERROR, "cache lookup failed for relation %u", myrelid); + relkind =3D ((Form_pg_class) GETSTRUCT(reltup))->relkind; = /* Determine the new attribute's number */ newattnum =3D ((Form_pg_class) GETSTRUCT(reltup))->relnatts + 1; @@ -5974,10 +6226,13 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *t= ab, Relation rel, /* Children should see column as singly inherited */ if (!recursing) { - colDef =3D copyObject(colDef); + childcmd =3D copyObject(*cmd); + colDef =3D castNode(ColumnDef, childcmd->def); colDef->inhcount =3D 1; colDef->is_local =3D false; } + else + childcmd =3D *cmd; /* no need to copy again */ = foreach(child, children) { @@ -5994,8 +6249,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab= , Relation rel, = /* Recurse to child; return value is ignored */ ATExecAddColumn(wqueue, childtab, childrel, - colDef, recurse, true, - if_not_exists, lockmode); + &childcmd, recurse, true, + lockmode, cur_pass, context); = table_close(childrel, NoLock); } @@ -6254,7 +6509,7 @@ ATExecDropNotNull(Relation rel, const char *colName,= LOCKMODE lockmode) static void ATPrepSetNotNull(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, - LOCKMODE lockmode) + LOCKMODE lockmode, AlterTableUtilityContext *context) { /* * If we're already recursing, there's nothing to do; the topmost @@ -6275,10 +6530,10 @@ ATPrepSetNotNull(List **wqueue, Relation rel, = newcmd->subtype =3D AT_CheckNotNull; newcmd->name =3D pstrdup(cmd->name); - ATSimpleRecursion(wqueue, rel, newcmd, true, lockmode); + ATSimpleRecursion(wqueue, rel, newcmd, true, lockmode, context); } else - ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); + ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context); } = /* @@ -7165,7 +7420,8 @@ ATExecSetStorage(Relation rel, const char *colName, = Node *newValue, LOCKMODE loc */ static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursin= g, - AlterTableCmd *cmd, LOCKMODE lockmode) + AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context) { if (rel->rd_rel->reloftype && !recursing) ereport(ERROR, @@ -7173,7 +7429,7 @@ ATPrepDropColumn(List **wqueue, Relation rel, bool r= ecurse, bool recursing, errmsg("cannot drop column from typed table"))); = if (rel->rd_rel->relkind =3D=3D RELKIND_COMPOSITE_TYPE) - ATTypedTableRecursion(wqueue, rel, cmd, lockmode); + ATTypedTableRecursion(wqueue, rel, cmd, lockmode, context); = if (recurse) cmd->subtype =3D AT_DropColumnRecurse; @@ -10426,12 +10682,27 @@ ATExecDropConstraint(Relation rel, const char *c= onstrName, = /* * ALTER COLUMN TYPE + * + * Unlike other subcommand types, we do parse transformation for ALTER CO= LUMN + * TYPE during phase 1 --- the AlterTableCmd passed in here is already + * transformed (and must be, because we rely on some transformed fields). + * + * The point of this is that the execution of all ALTER COLUMN TYPEs for = a + * table will be done "in parallel" during phase 3, so all the USING + * expressions should be parsed assuming the original column types. Also= , + * this allows a USING expression to refer to a field that will be droppe= d. + * + * To make this work safely, AT_PASS_DROP then AT_PASS_ALTER_TYPE must be + * the first two execution steps in phase 2; they must not see the effect= s + * of any other subcommand types, since the USING expressions are parsed + * against the unmodified table's state. */ static void ATPrepAlterColumnType(List **wqueue, AlteredTableInfo *tab, Relation rel, bool recurse, bool recursing, - AlterTableCmd *cmd, LOCKMODE lockmode) + AlterTableCmd *cmd, LOCKMODE lockmode, + AlterTableUtilityContext *context) { char *colName =3D cmd->name; ColumnDef *def =3D (ColumnDef *) cmd->def; @@ -10678,7 +10949,7 @@ ATPrepAlterColumnType(List **wqueue, errdetail("USING expression contains a whole-row table reference.= "))); pfree(attmap); } - ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode); + ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode, context); relation_close(childrel, NoLock); } } @@ -10690,7 +10961,7 @@ ATPrepAlterColumnType(List **wqueue, colName))); = if (tab->relkind =3D=3D RELKIND_COMPOSITE_TYPE) - ATTypedTableRecursion(wqueue, rel, cmd, lockmode); + ATTypedTableRecursion(wqueue, rel, cmd, lockmode, context); } = /* @@ -11469,10 +11740,19 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oi= d refRelId, char *cmd, (IndexStmt *) stmt, cmd)); else if (IsA(stmt, AlterTableStmt)) - querytree_list =3D list_concat(querytree_list, - transformAlterTableStmt(oldRelId, - (AlterTableStmt *) stmt, - cmd)); + { + List *beforeStmts; + List *afterStmts; + + stmt =3D (Node *) transformAlterTableStmt(oldRelId, + (AlterTableStmt *) stmt, + cmd, + &beforeStmts, + &afterStmts); + querytree_list =3D list_concat(querytree_list, beforeStmts); + querytree_list =3D lappend(querytree_list, stmt); + querytree_list =3D list_concat(querytree_list, afterStmts); + } else querytree_list =3D lappend(querytree_list, stmt); } diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 06bc2b7..6e65103 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -145,6 +145,10 @@ DefineVirtualRelation(RangeVar *relation, List *tlist= , bool replace, * Note that we must do this before updating the query for the view, * since the rules system requires that the correct view columns be in * place when defining the new rules. + * + * Also note that ALTER TABLE doesn't run parse transformation on + * AT_AddColumnToView commands. The ColumnDef we supply must be ready + * to execute as-is. */ if (list_length(attrList) > rel->rd_att->natts) { diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse= _utilcmd.c index 42095ab..ee2d2b5 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -347,7 +347,8 @@ transformCreateStmt(CreateStmt *stmt, const char *quer= yString) */ static void generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column, - Oid seqtypid, List *seqoptions, bool for_identity, + Oid seqtypid, List *seqoptions, + bool for_identity, bool col_exists, char **snamespace_p, char **sname_p) { ListCell *option; @@ -472,8 +473,12 @@ generateSerialExtraStmts(CreateStmtContext *cxt, Colu= mnDef *column, = /* * Build an ALTER SEQUENCE ... OWNED BY command to mark the sequence as - * owned by this column, and add it to the list of things to be done aft= er - * this CREATE/ALTER TABLE. + * owned by this column, and add it to the appropriate list of things to + * be done along with this CREATE/ALTER TABLE. In a CREATE or ALTER ADD + * COLUMN, it must be done after the statement because we don't know the + * column's attnum yet. But if we do have the attnum (in AT_AddIdentity= ), + * we can do the marking immediately, which improves some ALTER TABLE + * behaviors. */ altseqstmt =3D makeNode(AlterSeqStmt); altseqstmt->sequence =3D makeRangeVar(snamespace, sname, -1); @@ -484,7 +489,10 @@ generateSerialExtraStmts(CreateStmtContext *cxt, Colu= mnDef *column, (Node *) attnamelist, -1)); altseqstmt->for_identity =3D for_identity; = - cxt->alist =3D lappend(cxt->alist, altseqstmt); + if (col_exists) + cxt->blist =3D lappend(cxt->blist, altseqstmt); + else + cxt->alist =3D lappend(cxt->alist, altseqstmt); = if (snamespace_p) *snamespace_p =3D snamespace; @@ -568,7 +576,8 @@ transformColumnDefinition(CreateStmtContext *cxt, Colu= mnDef *column) Constraint *constraint; = generateSerialExtraStmts(cxt, column, - column->typeName->typeOid, NIL, false, + column->typeName->typeOid, NIL, + false, false, &snamespace, &sname); = /* @@ -684,7 +693,8 @@ transformColumnDefinition(CreateStmtContext *cxt, Colu= mnDef *column) constraint->location))); = generateSerialExtraStmts(cxt, column, - typeOid, constraint->options, true, + typeOid, constraint->options, + true, false, NULL, NULL); = column->identity =3D constraint->generated_when; @@ -1086,7 +1096,8 @@ transformTableLikeClause(CreateStmtContext *cxt, Tab= leLikeClause *table_like_cla seq_relid =3D getIdentitySequence(RelationGetRelid(relation), attribut= e->attnum, false); seq_options =3D sequence_options(seq_relid); generateSerialExtraStmts(cxt, def, - InvalidOid, seq_options, true, + InvalidOid, seq_options, + true, false, NULL, NULL); def->identity =3D attribute->attidentity; } @@ -2572,7 +2583,7 @@ transformFKConstraints(CreateStmtContext *cxt, Constraint *constraint =3D (Constraint *) lfirst(fkclist); AlterTableCmd *altercmd =3D makeNode(AlterTableCmd); = - altercmd->subtype =3D AT_ProcessedConstraint; + altercmd->subtype =3D AT_AddConstraint; altercmd->name =3D NULL; altercmd->def =3D (Node *) constraint; alterstmt->cmds =3D lappend(alterstmt->cmds, altercmd); @@ -3004,23 +3015,23 @@ transformRuleStmt(RuleStmt *stmt, const char *quer= yString, * transformAlterTableStmt - * parse analysis for ALTER TABLE * - * Returns a List of utility commands to be done in sequence. One of the= se - * will be the transformed AlterTableStmt, but there may be additional ac= tions - * to be done before and after the actual AlterTable() call. + * Returns the transformed AlterTableStmt. There may be additional actio= ns + * to be done before and after the transformed statement, which are retur= ned + * in *beforeStmts and *afterStmts as lists of utility command parsetrees= . * * To avoid race conditions, it's important that this function rely only = on * the passed-in relid (and not on stmt->relation) to determine the targe= t * relation. */ -List * +AlterTableStmt * transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, - const char *queryString) + const char *queryString, + List **beforeStmts, List **afterStmts) { Relation rel; TupleDesc tupdesc; ParseState *pstate; CreateStmtContext cxt; - List *result; List *save_alist; ListCell *lcmd, *l; @@ -3052,7 +3063,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, = /* Set up CreateStmtContext */ cxt.pstate =3D pstate; - if (stmt->relkind =3D=3D OBJECT_FOREIGN_TABLE) + if (rel->rd_rel->relkind =3D=3D RELKIND_FOREIGN_TABLE) { cxt.stmtType =3D "ALTER FOREIGN TABLE"; cxt.isforeign =3D true; @@ -3080,9 +3091,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, cxt.ofType =3D false; = /* - * The only subtypes that currently require parse transformation handlin= g - * are ADD COLUMN, ADD CONSTRAINT and SET DATA TYPE. These largely re-u= se - * code from CREATE TABLE. + * Transform ALTER subcommands that need it (most don't). These largely + * re-use code from CREATE TABLE. */ foreach(lcmd, stmt->cmds) { @@ -3091,7 +3101,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, switch (cmd->subtype) { case AT_AddColumn: - case AT_AddColumnToView: + case AT_AddColumnRecurse: { ColumnDef *def =3D castNode(ColumnDef, cmd->def); = @@ -3115,6 +3125,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, } = case AT_AddConstraint: + case AT_AddConstraintRecurse: = /* * The original AddConstraint cmd node doesn't go to newcmds @@ -3130,19 +3141,9 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *= stmt, (int) nodeTag(cmd->def)); break; = - case AT_ProcessedConstraint: - - /* - * Already-transformed ADD CONSTRAINT, so just make it look - * like the standard case. - */ - cmd->subtype =3D AT_AddConstraint; - newcmds =3D lappend(newcmds, cmd); - break; - case AT_AlterColumnType: { - ColumnDef *def =3D (ColumnDef *) cmd->def; + ColumnDef *def =3D castNode(ColumnDef, cmd->def); AttrNumber attnum; = /* @@ -3161,13 +3162,13 @@ transformAlterTableStmt(Oid relid, AlterTableStmt = *stmt, * change the data type of the sequence. */ attnum =3D get_attnum(relid, cmd->name); + if (attnum =3D=3D InvalidAttrNumber) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + cmd->name, RelationGetRelationName(rel)))); = - /* - * if attribute not found, something will error about it - * later - */ - if (attnum !=3D InvalidAttrNumber && - TupleDescAttr(tupdesc, attnum - 1)->attidentity) + if (TupleDescAttr(tupdesc, attnum - 1)->attidentity) { Oid seq_relid =3D getIdentitySequence(relid, attnum, false); Oid typeOid =3D typenameTypeId(pstate, def->typeName); @@ -3196,16 +3197,16 @@ transformAlterTableStmt(Oid relid, AlterTableStmt = *stmt, cmd->def =3D (Node *) newdef; = attnum =3D get_attnum(relid, cmd->name); + if (attnum =3D=3D InvalidAttrNumber) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + cmd->name, RelationGetRelationName(rel)))); = - /* - * if attribute not found, something will error about it - * later - */ - if (attnum !=3D InvalidAttrNumber) - generateSerialExtraStmts(&cxt, newdef, - get_atttype(relid, attnum), - def->options, true, - NULL, NULL); + generateSerialExtraStmts(&cxt, newdef, + get_atttype(relid, attnum), + def->options, true, true, + NULL, NULL); = newcmds =3D lappend(newcmds, cmd); break; @@ -3221,6 +3222,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *s= tmt, List *newseqopts =3D NIL; List *newdef =3D NIL; AttrNumber attnum; + Oid seq_relid; = /* * Split options into those handled by ALTER SEQUENCE and @@ -3237,29 +3239,34 @@ transformAlterTableStmt(Oid relid, AlterTableStmt = *stmt, } = attnum =3D get_attnum(relid, cmd->name); + if (attnum =3D=3D InvalidAttrNumber) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_COLUMN), + errmsg("column \"%s\" of relation \"%s\" does not exist", + cmd->name, RelationGetRelationName(rel)))); = - if (attnum) - { - Oid seq_relid =3D getIdentitySequence(relid, attnum, true); + seq_relid =3D getIdentitySequence(relid, attnum, true); = - if (seq_relid) - { - AlterSeqStmt *seqstmt; + if (seq_relid) + { + AlterSeqStmt *seqstmt; = - seqstmt =3D makeNode(AlterSeqStmt); - seqstmt->sequence =3D makeRangeVar(get_namespace_name(get_rel_name= space(seq_relid)), - get_rel_name(seq_relid), -1); - seqstmt->options =3D newseqopts; - seqstmt->for_identity =3D true; - seqstmt->missing_ok =3D false; + seqstmt =3D makeNode(AlterSeqStmt); + seqstmt->sequence =3D makeRangeVar(get_namespace_name(get_rel_names= pace(seq_relid)), + get_rel_name(seq_relid), -1); + seqstmt->options =3D newseqopts; + seqstmt->for_identity =3D true; + seqstmt->missing_ok =3D false; = - cxt.alist =3D lappend(cxt.alist, seqstmt); - } + cxt.blist =3D lappend(cxt.blist, seqstmt); } = /* - * If column was not found or was not an identity column, - * we just let the ALTER TABLE command error out later. + * If column was not an identity column, we just let the + * ALTER TABLE command error out later. (There are cases + * this fails to cover, but we'll need to restructure + * where creation of the sequence dependency linkage + * happens before we can fix it.) */ = cmd->def =3D (Node *) newdef; @@ -3281,6 +3288,12 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *= stmt, break; = default: + + /* + * Currently, we shouldn't actually get here for subcommand + * types that don't require transformation; but if we do, just + * emit them unchanged. + */ newcmds =3D lappend(newcmds, cmd); break; } @@ -3361,11 +3374,10 @@ transformAlterTableStmt(Oid relid, AlterTableStmt = *stmt, */ stmt->cmds =3D newcmds; = - result =3D lappend(cxt.blist, stmt); - result =3D list_concat(result, cxt.alist); - result =3D list_concat(result, save_alist); + *beforeStmts =3D cxt.blist; + *afterStmts =3D list_concat(cxt.alist, save_alist); = - return result; + return stmt; } = = diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index b2c58bf..696c92f 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1093,8 +1093,6 @@ ProcessUtilitySlow(ParseState *pstate, { AlterTableStmt *atstmt =3D (AlterTableStmt *) parsetree; Oid relid; - List *stmts; - ListCell *l; LOCKMODE lockmode; = /* @@ -1108,59 +1106,21 @@ ProcessUtilitySlow(ParseState *pstate, = if (OidIsValid(relid)) { - /* Run parse analysis ... */ - stmts =3D transformAlterTableStmt(relid, atstmt, - queryString); + AlterTableUtilityContext atcontext; + + /* Set up info needed for recursive callbacks ... */ + atcontext.pstmt =3D pstmt; + atcontext.queryString =3D queryString; + atcontext.relid =3D relid; + atcontext.params =3D params; + atcontext.queryEnv =3D queryEnv; = /* ... ensure we have an event trigger context ... */ EventTriggerAlterTableStart(parsetree); EventTriggerAlterTableRelid(relid); = /* ... and do it */ - foreach(l, stmts) - { - Node *stmt =3D (Node *) lfirst(l); - - if (IsA(stmt, AlterTableStmt)) - { - /* Do the table alteration proper */ - AlterTable(relid, lockmode, - (AlterTableStmt *) stmt); - } - else - { - /* - * Recurse for anything else. If we need to - * do so, "close" the current complex-command - * set, and start a new one at the bottom; - * this is needed to ensure the ordering of - * queued commands is consistent with the way - * they are executed here. - */ - PlannedStmt *wrapper; - - EventTriggerAlterTableEnd(); - wrapper =3D makeNode(PlannedStmt); - wrapper->commandType =3D CMD_UTILITY; - wrapper->canSetTag =3D false; - wrapper->utilityStmt =3D stmt; - wrapper->stmt_location =3D pstmt->stmt_location; - wrapper->stmt_len =3D pstmt->stmt_len; - ProcessUtility(wrapper, - queryString, - PROCESS_UTILITY_SUBCOMMAND, - params, - NULL, - None_Receiver, - NULL); - EventTriggerAlterTableStart(parsetree); - EventTriggerAlterTableRelid(relid); - } - - /* Need CCI between commands */ - if (lnext(stmts, l) !=3D NULL) - CommandCounterIncrement(); - } + AlterTable(atstmt, lockmode, &atcontext); = /* done */ EventTriggerAlterTableEnd(); @@ -1718,6 +1678,52 @@ ProcessUtilitySlow(ParseState *pstate, } = /* + * ProcessUtilityForAlterTable + * Recursive entry from ALTER TABLE + * + * ALTER TABLE sometimes generates subcommands such as CREATE INDEX. + * It calls this, not the main entry point ProcessUtility, to execute + * such subcommands. + * + * stmt: the utility command to execute + * context: opaque passthrough struct with the info we need + * + * It's caller's responsibility to do CommandCounterIncrement after + * calling this, if needed. + */ +void +ProcessUtilityForAlterTable(Node *stmt, AlterTableUtilityContext *context= ) +{ + PlannedStmt *wrapper; + + /* + * For event triggers, we must "close" the current complex-command set, + * and start a new one afterwards; this is needed to ensure the ordering + * of command events is consistent with the way they were executed. + */ + EventTriggerAlterTableEnd(); + + /* Create a suitable wrapper */ + wrapper =3D makeNode(PlannedStmt); + wrapper->commandType =3D CMD_UTILITY; + wrapper->canSetTag =3D false; + wrapper->utilityStmt =3D stmt; + wrapper->stmt_location =3D context->pstmt->stmt_location; + wrapper->stmt_len =3D context->pstmt->stmt_len; + + ProcessUtility(wrapper, + context->queryString, + PROCESS_UTILITY_SUBCOMMAND, + context->params, + context->queryEnv, + None_Receiver, + NULL); + + EventTriggerAlterTableStart(context->pstmt->utilityStmt); + EventTriggerAlterTableRelid(context->relid); +} + +/* * Dispatch function for DropStmt */ static void diff --git a/src/include/commands/tablecmds.h b/src/include/commands/table= cmds.h index 2e04b82..c1581ad 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -21,6 +21,8 @@ #include "storage/lock.h" #include "utils/relcache.h" = +struct AlterTableUtilityContext; /* avoid including tcop/utility.h here *= / + = extern ObjectAddress DefineRelation(CreateStmt *stmt, char relkind, Oid o= wnerId, ObjectAddress *typaddress, const char *queryString); @@ -29,7 +31,8 @@ extern void RemoveRelations(DropStmt *drop); = extern Oid AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmo= de); = -extern void AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *stmt= ); +extern void AlterTable(AlterTableStmt *stmt, LOCKMODE lockmode, + struct AlterTableUtilityContext *context); = extern LOCKMODE AlterTableGetLockLevel(List *cmds); = diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes= .h index 28d837b..da0706a 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1802,8 +1802,6 @@ typedef enum AlterTableType AT_AlterConstraint, /* alter constraint */ AT_ValidateConstraint, /* validate constraint */ AT_ValidateConstraintRecurse, /* internal to commands/tablecmds.c */ - AT_ProcessedConstraint, /* pre-processed add constraint (local in - * parser/parse_utilcmd.c) */ AT_AddIndexConstraint, /* add constraint using existing index */ AT_DropConstraint, /* drop constraint */ AT_DropConstraintRecurse, /* internal to commands/tablecmds.c */ diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse= _utilcmd.h index eb73acd..1a5e0b8 100644 --- a/src/include/parser/parse_utilcmd.h +++ b/src/include/parser/parse_utilcmd.h @@ -20,8 +20,10 @@ struct AttrMap; /* avoid including attmap.h here */ = = extern List *transformCreateStmt(CreateStmt *stmt, const char *queryStrin= g); -extern List *transformAlterTableStmt(Oid relid, AlterTableStmt *stmt, - const char *queryString); +extern AlterTableStmt *transformAlterTableStmt(Oid relid, AlterTableStmt = *stmt, + const char *queryString, + List **beforeStmts, + List **afterStmts); extern IndexStmt *transformIndexStmt(Oid relid, IndexStmt *stmt, const char *queryString); extern void transformRuleStmt(RuleStmt *stmt, const char *queryString, diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h index 40551c4..f62bfc4 100644 --- a/src/include/tcop/utility.h +++ b/src/include/tcop/utility.h @@ -25,6 +25,16 @@ typedef enum PROCESS_UTILITY_SUBCOMMAND /* a portion of a query */ } ProcessUtilityContext; = +/* Info needed when recursing from ALTER TABLE */ +typedef struct AlterTableUtilityContext +{ + PlannedStmt *pstmt; /* PlannedStmt for outer ALTER TABLE command */ + const char *queryString; /* its query string */ + Oid relid; /* OID of ALTER's target table */ + ParamListInfo params; /* any parameters available to ALTER TABLE */ + QueryEnvironment *queryEnv; /* execution environment for ALTER TABLE */ +} AlterTableUtilityContext; + /* Hook for plugins to get control in ProcessUtility() */ typedef void (*ProcessUtility_hook_type) (PlannedStmt *pstmt, const char *queryString, ProcessUtilityContext context, @@ -42,6 +52,9 @@ extern void standard_ProcessUtility(PlannedStmt *pstmt, = const char *queryString, QueryEnvironment *queryEnv, DestReceiver *dest, char *completionTag); = +extern void ProcessUtilityForAlterTable(Node *stmt, + AlterTableUtilityContext *context); + extern bool UtilityReturnsTuples(Node *parsetree); = extern TupleDesc UtilityTupleDescriptor(Node *parsetree); diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/te= st/modules/test_ddl_deparse/test_ddl_deparse.c index 5ef04b7..e1629ec 100644 --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c @@ -162,9 +162,6 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS) case AT_ValidateConstraintRecurse: strtype =3D "VALIDATE CONSTRAINT (and recurse)"; break; - case AT_ProcessedConstraint: - strtype =3D "ADD (processed) CONSTRAINT"; - break; case AT_AddIndexConstraint: strtype =3D "ADD CONSTRAINT (using index)"; break; diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/= expected/alter_table.out index b492c60..4dd3507 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1958,27 +1958,29 @@ Indexes: "anothertab_f4_idx" UNIQUE, btree (f4) = drop table anothertab; -create table another (f1 int, f2 text); -insert into another values(1, 'one'); -insert into another values(2, 'two'); -insert into another values(3, 'three'); +-- test that USING expressions are parsed before column alter type / drop= steps +create table another (f1 int, f2 text, f3 text); +insert into another values(1, 'one', 'uno'); +insert into another values(2, 'two', 'due'); +insert into another values(3, 'three', 'tre'); select * from another; - f1 | f2 = -----+------- - 1 | one - 2 | two - 3 | three + f1 | f2 | f3 = +----+-------+----- + 1 | one | uno + 2 | two | due + 3 | three | tre (3 rows) = alter table another - alter f1 type text using f2 || ' more', - alter f2 type bigint using f1 * 10; + alter f1 type text using f2 || ' and ' || f3 || ' more', + alter f2 type bigint using f1 * 10, + drop column f3; select * from another; - f1 | f2 = -------------+---- - one more | 10 - two more | 20 - three more | 30 + f1 | f2 = +--------------------+---- + one and uno more | 10 + two and due more | 20 + three and tre more | 30 (3 rows) = drop table another; @@ -3469,7 +3471,7 @@ NOTICE: column "c2" of relation "test_add_column" a= lready exists, skipping = ALTER TABLE test_add_column ADD COLUMN c2 integer, -- fail because c2 already exists - ADD COLUMN c3 integer; + ADD COLUMN c3 integer primary key; ERROR: column "c2" of relation "test_add_column" already exists \d test_add_column Table "public.test_add_column" @@ -3480,7 +3482,7 @@ ERROR: column "c2" of relation "test_add_column" al= ready exists = ALTER TABLE test_add_column ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exis= ts - ADD COLUMN c3 integer; -- fail because c3 already exists + ADD COLUMN c3 integer primary key; NOTICE: column "c2" of relation "test_add_column" already exists, skippi= ng \d test_add_column Table "public.test_add_column" @@ -3488,11 +3490,13 @@ NOTICE: column "c2" of relation "test_add_column"= already exists, skipping --------+---------+-----------+----------+--------- c1 | integer | | | = c2 | integer | | | = - c3 | integer | | | = + c3 | integer | | not null | = +Indexes: + "test_add_column_pkey" PRIMARY KEY, btree (c3) = ALTER TABLE test_add_column ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exis= ts - ADD COLUMN IF NOT EXISTS c3 integer; -- skipping because c3 already exis= ts + ADD COLUMN IF NOT EXISTS c3 integer primary key; -- skipping because c3 = already exists NOTICE: column "c2" of relation "test_add_column" already exists, skippi= ng NOTICE: column "c3" of relation "test_add_column" already exists, skippi= ng \d test_add_column @@ -3501,12 +3505,14 @@ NOTICE: column "c3" of relation "test_add_column"= already exists, skipping --------+---------+-----------+----------+--------- c1 | integer | | | = c2 | integer | | | = - c3 | integer | | | = + c3 | integer | | not null | = +Indexes: + "test_add_column_pkey" PRIMARY KEY, btree (c3) = ALTER TABLE test_add_column ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exis= ts ADD COLUMN IF NOT EXISTS c3 integer, -- skipping because c3 already exis= ts - ADD COLUMN c4 integer; + ADD COLUMN c4 integer REFERENCES test_add_column; NOTICE: column "c2" of relation "test_add_column" already exists, skippi= ng NOTICE: column "c3" of relation "test_add_column" already exists, skippi= ng \d test_add_column @@ -3515,10 +3521,118 @@ NOTICE: column "c3" of relation "test_add_column= " already exists, skipping --------+---------+-----------+----------+--------- c1 | integer | | | = c2 | integer | | | = - c3 | integer | | | = + c3 | integer | | not null | = + c4 | integer | | | = +Indexes: + "test_add_column_pkey" PRIMARY KEY, btree (c3) +Foreign-key constraints: + "test_add_column_c4_fkey" FOREIGN KEY (c4) REFERENCES test_add_column= (c3) +Referenced by: + TABLE "test_add_column" CONSTRAINT "test_add_column_c4_fkey" FOREIGN = KEY (c4) REFERENCES test_add_column(c3) + +ALTER TABLE test_add_column + ADD COLUMN IF NOT EXISTS c4 integer REFERENCES test_add_column; +NOTICE: column "c4" of relation "test_add_column" already exists, skippi= ng +\d test_add_column + Table "public.test_add_column" + Column | Type | Collation | Nullable | Default = +--------+---------+-----------+----------+--------- + c1 | integer | | | = + c2 | integer | | | = + c3 | integer | | not null | = + c4 | integer | | | = +Indexes: + "test_add_column_pkey" PRIMARY KEY, btree (c3) +Foreign-key constraints: + "test_add_column_c4_fkey" FOREIGN KEY (c4) REFERENCES test_add_column= (c3) +Referenced by: + TABLE "test_add_column" CONSTRAINT "test_add_column_c4_fkey" FOREIGN = KEY (c4) REFERENCES test_add_column(c3) + +ALTER TABLE test_add_column + ADD COLUMN IF NOT EXISTS c5 SERIAL CHECK (c5 > 8); +\d test_add_column + Table "public.test_add_column" + Column | Type | Collation | Nullable | Default = = +--------+---------+-----------+----------+-------------------------------= -------------- + c1 | integer | | | = + c2 | integer | | | = + c3 | integer | | not null | = + c4 | integer | | | = + c5 | integer | | not null | nextval('test_add_column_c5_se= q'::regclass) +Indexes: + "test_add_column_pkey" PRIMARY KEY, btree (c3) +Check constraints: + "test_add_column_c5_check" CHECK (c5 > 8) +Foreign-key constraints: + "test_add_column_c4_fkey" FOREIGN KEY (c4) REFERENCES test_add_column= (c3) +Referenced by: + TABLE "test_add_column" CONSTRAINT "test_add_column_c4_fkey" FOREIGN = KEY (c4) REFERENCES test_add_column(c3) + +ALTER TABLE test_add_column + ADD COLUMN IF NOT EXISTS c5 SERIAL CHECK (c5 > 10); +NOTICE: column "c5" of relation "test_add_column" already exists, skippi= ng +\d test_add_column* + Table "public.test_add_column" + Column | Type | Collation | Nullable | Default = = +--------+---------+-----------+----------+-------------------------------= -------------- + c1 | integer | | | = + c2 | integer | | | = + c3 | integer | | not null | = c4 | integer | | | = + c5 | integer | | not null | nextval('test_add_column_c5_se= q'::regclass) +Indexes: + "test_add_column_pkey" PRIMARY KEY, btree (c3) +Check constraints: + "test_add_column_c5_check" CHECK (c5 > 8) +Foreign-key constraints: + "test_add_column_c4_fkey" FOREIGN KEY (c4) REFERENCES test_add_column= (c3) +Referenced by: + TABLE "test_add_column" CONSTRAINT "test_add_column_c4_fkey" FOREIGN = KEY (c4) REFERENCES test_add_column(c3) + + Sequence "public.test_add_column_c5_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache = +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Owned by: public.test_add_column.c5 + + Index "public.test_add_column_pkey" + Column | Type | Key? | Definition = +--------+---------+------+------------ + c3 | integer | yes | c3 +primary key, btree, for table "public.test_add_column" = DROP TABLE test_add_column; +\d test_add_column* +-- assorted cases with multiple ALTER TABLE steps +CREATE TABLE ataddindex(f1 INT); +INSERT INTO ataddindex VALUES (42), (43); +CREATE UNIQUE INDEX ataddindexi0 ON ataddindex(f1); +ALTER TABLE ataddindex + ADD PRIMARY KEY USING INDEX ataddindexi0, + ALTER f1 TYPE BIGINT; +\d ataddindex + Table "public.ataddindex" + Column | Type | Collation | Nullable | Default = +--------+--------+-----------+----------+--------- + f1 | bigint | | not null | = +Indexes: + "ataddindexi0" PRIMARY KEY, btree (f1) + +DROP TABLE ataddindex; +CREATE TABLE ataddindex(f1 VARCHAR(10)); +INSERT INTO ataddindex(f1) VALUES ('foo'), ('a'); +ALTER TABLE ataddindex + ALTER f1 SET DATA TYPE TEXT, + ADD EXCLUDE ((f1 LIKE 'a') WITH =3D); +\d ataddindex + Table "public.ataddindex" + Column | Type | Collation | Nullable | Default = +--------+------+-----------+----------+--------- + f1 | text | | | = +Indexes: + "ataddindex_expr_excl" EXCLUDE USING btree ((f1 ~~ 'a'::text) WITH =3D= ) + +DROP TABLE ataddindex; -- unsupported constraint types for partitioned tables CREATE TABLE partitioned ( a int, diff --git a/src/test/regress/expected/identity.out b/src/test/regress/exp= ected/identity.out index 36a2393..7cf4696 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -387,6 +387,68 @@ SELECT * FROM itest8; RESET ROLE; DROP TABLE itest8; DROP USER regress_identity_user1; +-- multiple steps in ALTER TABLE +CREATE TABLE itest8 (f1 int); +ALTER TABLE itest8 + ADD COLUMN f2 int NOT NULL, + ALTER COLUMN f2 ADD GENERATED ALWAYS AS IDENTITY; +ALTER TABLE itest8 + ADD COLUMN f3 int NOT NULL, + ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY, + ALTER COLUMN f3 SET GENERATED BY DEFAULT SET INCREMENT 10; +ALTER TABLE itest8 + ADD COLUMN f4 int; +ALTER TABLE itest8 + ALTER COLUMN f4 SET NOT NULL, + ALTER COLUMN f4 ADD GENERATED ALWAYS AS IDENTITY, + ALTER COLUMN f4 SET DATA TYPE bigint; +ALTER TABLE itest8 + ADD COLUMN f5 int GENERATED ALWAYS AS IDENTITY; +ALTER TABLE itest8 + ALTER COLUMN f5 DROP IDENTITY, + ALTER COLUMN f5 DROP NOT NULL, + ALTER COLUMN f5 SET DATA TYPE bigint; +INSERT INTO itest8 VALUES(0), (1); +TABLE itest8; + f1 | f2 | f3 | f4 | f5 = +----+----+----+----+---- + 0 | 1 | 1 | 1 | = + 1 | 2 | 11 | 2 | = +(2 rows) + +\d+ itest8 + Table "public.itest8" + Column | Type | Collation | Nullable | Default = | Storage | Stats target | Description = +--------+---------+-----------+----------+-------------------------------= ---+---------+--------------+------------- + f1 | integer | | | = | plain | | = + f2 | integer | | not null | generated always as identity = | plain | | = + f3 | integer | | not null | generated by default as identi= ty | plain | | = + f4 | bigint | | not null | generated always as identity = | plain | | = + f5 | bigint | | | = | plain | | = + +\d itest8_f2_seq + Sequence "public.itest8_f2_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache = +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 1 | no | 1 +Sequence for identity column: public.itest8.f2 + +\d itest8_f3_seq + Sequence "public.itest8_f3_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | Cache = +---------+-------+---------+------------+-----------+---------+------- + integer | 1 | 1 | 2147483647 | 10 | no | 1 +Sequence for identity column: public.itest8.f3 + +\d itest8_f4_seq + Sequence "public.itest8_f4_seq" + Type | Start | Minimum | Maximum | Increment | Cycles? | C= ache = +--------+-------+---------+---------------------+-----------+---------+--= ----- + bigint | 1 | 1 | 9223372036854775807 | 1 | no | = 1 +Sequence for identity column: public.itest8.f4 + +\d itest8_f5_seq +DROP TABLE itest8; -- typed tables (currently not supported) CREATE TYPE itest_type AS (f1 integer, f2 text, f3 bigint); CREATE TABLE itest12 OF itest_type (f1 WITH OPTIONS GENERATED ALWAYS AS I= DENTITY); -- error diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/a= lter_table.sql index abe7be3..a16e4c9 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1342,17 +1342,19 @@ alter table anothertab alter column f5 type bigint= ; = drop table anothertab; = -create table another (f1 int, f2 text); +-- test that USING expressions are parsed before column alter type / drop= steps +create table another (f1 int, f2 text, f3 text); = -insert into another values(1, 'one'); -insert into another values(2, 'two'); -insert into another values(3, 'three'); +insert into another values(1, 'one', 'uno'); +insert into another values(2, 'two', 'due'); +insert into another values(3, 'three', 'tre'); = select * from another; = alter table another - alter f1 type text using f2 || ' more', - alter f2 type bigint using f1 * 10; + alter f1 type text using f2 || ' and ' || f3 || ' more', + alter f2 type bigint using f1 * 10, + drop column f3; = select * from another; = @@ -2170,22 +2172,50 @@ ALTER TABLE ONLY test_add_column \d test_add_column ALTER TABLE test_add_column ADD COLUMN c2 integer, -- fail because c2 already exists - ADD COLUMN c3 integer; + ADD COLUMN c3 integer primary key; \d test_add_column ALTER TABLE test_add_column ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exis= ts - ADD COLUMN c3 integer; -- fail because c3 already exists + ADD COLUMN c3 integer primary key; \d test_add_column ALTER TABLE test_add_column ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exis= ts - ADD COLUMN IF NOT EXISTS c3 integer; -- skipping because c3 already exis= ts + ADD COLUMN IF NOT EXISTS c3 integer primary key; -- skipping because c3 = already exists \d test_add_column ALTER TABLE test_add_column ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exis= ts ADD COLUMN IF NOT EXISTS c3 integer, -- skipping because c3 already exis= ts - ADD COLUMN c4 integer; + ADD COLUMN c4 integer REFERENCES test_add_column; \d test_add_column +ALTER TABLE test_add_column + ADD COLUMN IF NOT EXISTS c4 integer REFERENCES test_add_column; +\d test_add_column +ALTER TABLE test_add_column + ADD COLUMN IF NOT EXISTS c5 SERIAL CHECK (c5 > 8); +\d test_add_column +ALTER TABLE test_add_column + ADD COLUMN IF NOT EXISTS c5 SERIAL CHECK (c5 > 10); +\d test_add_column* DROP TABLE test_add_column; +\d test_add_column* + +-- assorted cases with multiple ALTER TABLE steps +CREATE TABLE ataddindex(f1 INT); +INSERT INTO ataddindex VALUES (42), (43); +CREATE UNIQUE INDEX ataddindexi0 ON ataddindex(f1); +ALTER TABLE ataddindex + ADD PRIMARY KEY USING INDEX ataddindexi0, + ALTER f1 TYPE BIGINT; +\d ataddindex +DROP TABLE ataddindex; + +CREATE TABLE ataddindex(f1 VARCHAR(10)); +INSERT INTO ataddindex(f1) VALUES ('foo'), ('a'); +ALTER TABLE ataddindex + ALTER f1 SET DATA TYPE TEXT, + ADD EXCLUDE ((f1 LIKE 'a') WITH =3D); +\d ataddindex +DROP TABLE ataddindex; = -- unsupported constraint types for partitioned tables CREATE TABLE partitioned ( diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/iden= tity.sql index 4b03d24..685607c 100644 --- a/src/test/regress/sql/identity.sql +++ b/src/test/regress/sql/identity.sql @@ -239,6 +239,44 @@ RESET ROLE; DROP TABLE itest8; DROP USER regress_identity_user1; = +-- multiple steps in ALTER TABLE +CREATE TABLE itest8 (f1 int); + +ALTER TABLE itest8 + ADD COLUMN f2 int NOT NULL, + ALTER COLUMN f2 ADD GENERATED ALWAYS AS IDENTITY; + +ALTER TABLE itest8 + ADD COLUMN f3 int NOT NULL, + ALTER COLUMN f3 ADD GENERATED ALWAYS AS IDENTITY, + ALTER COLUMN f3 SET GENERATED BY DEFAULT SET INCREMENT 10; + +ALTER TABLE itest8 + ADD COLUMN f4 int; + +ALTER TABLE itest8 + ALTER COLUMN f4 SET NOT NULL, + ALTER COLUMN f4 ADD GENERATED ALWAYS AS IDENTITY, + ALTER COLUMN f4 SET DATA TYPE bigint; + +ALTER TABLE itest8 + ADD COLUMN f5 int GENERATED ALWAYS AS IDENTITY; + +ALTER TABLE itest8 + ALTER COLUMN f5 DROP IDENTITY, + ALTER COLUMN f5 DROP NOT NULL, + ALTER COLUMN f5 SET DATA TYPE bigint; + +INSERT INTO itest8 VALUES(0), (1); + +TABLE itest8; +\d+ itest8 +\d itest8_f2_seq +\d itest8_f3_seq +\d itest8_f4_seq +\d itest8_f5_seq +DROP TABLE itest8; + = -- typed tables (currently not supported) = ------- =_aaaaaaaaaa0--