Restructure ALTER TABLE execution to fix assorted bugs.

We've had numerous bug reports about how (1) IF NOT EXISTS clauses in
ALTER TABLE don't behave as-expected, and (2) combining certain actions
into one ALTER TABLE doesn't work, though executing the same actions as
separate statements does.  This patch cleans up all of the cases so far
reported from the field, though there are still some oddities associated
with identity columns.

The core problem behind all of these bugs is that we do parse analysis
of ALTER TABLE subcommands too soon, before starting execution of the
statement.  The root of the bugs in group (1) is that parse analysis
schedules derived commands (such as a CREATE SEQUENCE for a serial
column) before it's known whether the IF NOT EXISTS clause should cause
a subcommand to be skipped.  The root of the bugs in group (2) is that
earlier subcommands may change the catalog state that later subcommands
need to be parsed against.

Hence, postpone parse analysis of ALTER TABLE's subcommands, and do
that one subcommand at a time, during "phase 2" of ALTER TABLE which
is the phase that does catalog rewrites.  Thus the catalog effects
of earlier subcommands are already visible when we analyze later ones.
(The sole exception is that we do parse analysis for ALTER COLUMN TYPE
subcommands during phase 1, so that their USING expressions can be
parsed against the table's original state, which is what we need.
Arguably, these bugs stem from falsely concluding that because ALTER
COLUMN TYPE must do early parse analysis, every other command subtype
can too.)

This means that ALTER TABLE itself must deal with execution of any
non-ALTER-TABLE derived statements that are generated by parse analysis.
Add a suitable entry point to utility.c to accept those recursive
calls, and create a struct to pass through the information needed by
the recursive call, rather than making the argument lists of
AlterTable() and friends even longer.

Getting this to work correctly required a little bit of fiddling
with the subcommand pass structure, in particular breaking up
AT_PASS_ADD_CONSTR into multiple passes.  But otherwise it's mostly
a pretty straightforward application of the above ideas.

Fixing the residual issues for identity columns requires refactoring of
where the dependency link from an identity column to its sequence gets
set 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

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)
 		{

src/backend/parser/parse_utilcmd.c

@@ -347,7 +347,8 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
  */
 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, ColumnDef *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 after
-	 * 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 = makeNode(AlterSeqStmt);
 	altseqstmt->sequence = makeRangeVar(snamespace, sname, -1);
@@ -484,6 +489,9 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
 												 (Node *) attnamelist, -1));
 	altseqstmt->for_identity = for_identity;
 
+	if (col_exists)
+		cxt->blist = lappend(cxt->blist, altseqstmt);
+	else
 		cxt->alist = lappend(cxt->alist, altseqstmt);
 
 	if (snamespace_p)
@@ -568,7 +576,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *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, ColumnDef *column)
 													constraint->location)));
 
 					generateSerialExtraStmts(cxt, column,
-											 typeOid, constraint->options, true,
+											 typeOid, constraint->options,
+											 true, false,
 											 NULL, NULL);
 
 					column->identity = constraint->generated_when;
@@ -1086,7 +1096,8 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 			seq_relid = getIdentitySequence(RelationGetRelid(relation), attribute->attnum, false);
 			seq_options = sequence_options(seq_relid);
 			generateSerialExtraStmts(cxt, def,
-									 InvalidOid, seq_options, true,
+									 InvalidOid, seq_options,
+									 true, false,
 									 NULL, NULL);
 			def->identity = attribute->attidentity;
 		}
@@ -2572,7 +2583,7 @@ transformFKConstraints(CreateStmtContext *cxt,
 			Constraint *constraint = (Constraint *) lfirst(fkclist);
 			AlterTableCmd *altercmd = makeNode(AlterTableCmd);
 
-			altercmd->subtype = AT_ProcessedConstraint;
+			altercmd->subtype = AT_AddConstraint;
 			altercmd->name = NULL;
 			altercmd->def = (Node *) constraint;
 			alterstmt->cmds = lappend(alterstmt->cmds, altercmd);
@@ -3004,23 +3015,23 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
  * transformAlterTableStmt -
  *		parse analysis for ALTER TABLE
  *
- * Returns a List of utility commands to be done in sequence.  One of these
- * will be the transformed AlterTableStmt, but there may be additional actions
- * to be done before and after the actual AlterTable() call.
+ * Returns the transformed AlterTableStmt.  There may be additional actions
+ * to be done before and after the transformed statement, which are returned
+ * 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 target
  * 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 *stmt,
 
 	/* Set up CreateStmtContext */
 	cxt.pstate = pstate;
-	if (stmt->relkind == OBJECT_FOREIGN_TABLE)
+	if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
 	{
 		cxt.stmtType = "ALTER FOREIGN TABLE";
 		cxt.isforeign = true;
@@ -3080,9 +3091,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 	cxt.ofType = false;
 
 	/*
-	 * The only subtypes that currently require parse transformation handling
-	 * are ADD COLUMN, ADD CONSTRAINT and SET DATA TYPE.  These largely re-use
-	 * 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 *stmt,
 		switch (cmd->subtype)
 		{
 			case AT_AddColumn:
-			case AT_AddColumnToView:
+			case AT_AddColumnRecurse:
 				{
 					ColumnDef  *def = castNode(ColumnDef, cmd->def);
 
@@ -3115,6 +3125,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 				}
 
 			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 = AT_AddConstraint;
-				newcmds = lappend(newcmds, cmd);
-				break;
-
 			case AT_AlterColumnType:
 				{
-					ColumnDef  *def = (ColumnDef *) cmd->def;
+					ColumnDef  *def = castNode(ColumnDef, cmd->def);
 					AttrNumber	attnum;
 
 					/*
@@ -3161,13 +3162,13 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 					 * change the data type of the sequence.
 					 */
 					attnum = get_attnum(relid, cmd->name);
+					if (attnum == 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 != InvalidAttrNumber &&
-						TupleDescAttr(tupdesc, attnum - 1)->attidentity)
+					if (TupleDescAttr(tupdesc, attnum - 1)->attidentity)
 					{
 						Oid			seq_relid = getIdentitySequence(relid, attnum, false);
 						Oid			typeOid = typenameTypeId(pstate, def->typeName);
@@ -3196,15 +3197,15 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 					cmd->def = (Node *) newdef;
 
 					attnum = get_attnum(relid, cmd->name);
+					if (attnum == 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 != InvalidAttrNumber)
 					generateSerialExtraStmts(&cxt, newdef,
 											 get_atttype(relid, attnum),
-												 def->options, true,
+											 def->options, true, true,
 											 NULL, NULL);
 
 					newcmds = lappend(newcmds, cmd);
@@ -3221,6 +3222,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 					List	   *newseqopts = NIL;
 					List	   *newdef = NIL;
 					AttrNumber	attnum;
+					Oid			seq_relid;
 
 					/*
 					 * Split options into those handled by ALTER SEQUENCE and
@@ -3237,10 +3239,13 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 					}
 
 					attnum = get_attnum(relid, cmd->name);
+					if (attnum == 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 = getIdentitySequence(relid, attnum, true);
+					seq_relid = getIdentitySequence(relid, attnum, true);
 
 					if (seq_relid)
 					{
@@ -3253,13 +3258,15 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 						seqstmt->for_identity = true;
 						seqstmt->missing_ok = false;
 
-							cxt.alist = lappend(cxt.alist, seqstmt);
-						}
+						cxt.blist = 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 = (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 = lappend(newcmds, cmd);
 				break;
 		}
@@ -3361,11 +3374,10 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 	 */
 	stmt->cmds = newcmds;
 
-	result = lappend(cxt.blist, stmt);
-	result = list_concat(result, cxt.alist);
-	result = list_concat(result, save_alist);
+	*beforeStmts = cxt.blist;
+	*afterStmts = list_concat(cxt.alist, save_alist);
 
-	return result;
+	return stmt;
 }
 
 

src/backend/tcop/utility.c

@@ -1093,8 +1093,6 @@ ProcessUtilitySlow(ParseState *pstate,
 				{
 					AlterTableStmt *atstmt = (AlterTableStmt *) parsetree;
 					Oid			relid;
-					List	   *stmts;
-					ListCell   *l;
 					LOCKMODE	lockmode;
 
 					/*
@@ -1108,59 +1106,21 @@ ProcessUtilitySlow(ParseState *pstate,
 
 					if (OidIsValid(relid))
 					{
-						/* Run parse analysis ... */
-						stmts = transformAlterTableStmt(relid, atstmt,
-														queryString);
+						AlterTableUtilityContext atcontext;
+
+						/* Set up info needed for recursive callbacks ... */
+						atcontext.pstmt = pstmt;
+						atcontext.queryString = queryString;
+						atcontext.relid = relid;
+						atcontext.params = params;
+						atcontext.queryEnv = queryEnv;
 
 						/* ... ensure we have an event trigger context ... */
 						EventTriggerAlterTableStart(parsetree);
 						EventTriggerAlterTableRelid(relid);
 
 						/* ... and do it */
-						foreach(l, stmts)
-						{
-							Node	   *stmt = (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 = makeNode(PlannedStmt);
-								wrapper->commandType = CMD_UTILITY;
-								wrapper->canSetTag = false;
-								wrapper->utilityStmt = stmt;
-								wrapper->stmt_location = pstmt->stmt_location;
-								wrapper->stmt_len = 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) != NULL)
-								CommandCounterIncrement();
-						}
+						AlterTable(atstmt, lockmode, &atcontext);
 
 						/* done */
 						EventTriggerAlterTableEnd();
@@ -1717,6 +1677,52 @@ ProcessUtilitySlow(ParseState *pstate,
 	PG_END_TRY();
 }
 
+/*
+ * 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 = makeNode(PlannedStmt);
+	wrapper->commandType = CMD_UTILITY;
+	wrapper->canSetTag = false;
+	wrapper->utilityStmt = stmt;
+	wrapper->stmt_location = context->pstmt->stmt_location;
+	wrapper->stmt_len = 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
  */
@@ -2394,9 +2400,10 @@ CreateCommandTag(Node *parsetree)
 			break;
 
 		case T_RenameStmt:
+
 			/*
-			 * When the column is renamed, the command tag is created
-			 * from its relation type
+			 * When the column is renamed, the command tag is created from its
+			 * relation type
 			 */
 			tag = AlterObjectTypeCommandTag(
 											((RenameStmt *) parsetree)->renameType == OBJECT_COLUMN ?

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 ownerId,
 									ObjectAddress *typaddress, const char *queryString);
@@ -29,7 +31,8 @@ extern void RemoveRelations(DropStmt *drop);
 
 extern Oid	AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode);
 
-extern void AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *stmt);
+extern void AlterTable(AlterTableStmt *stmt, LOCKMODE lockmode,
+					   struct AlterTableUtilityContext *context);
 
 extern LOCKMODE AlterTableGetLockLevel(List *cmds);
 

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 */

src/include/parser/parse_utilcmd.h

@@ -20,8 +20,10 @@ struct AttrMap;					/* avoid including attmap.h here */
 
 
 extern List *transformCreateStmt(CreateStmt *stmt, const char *queryString);
-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,

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);

src/test/modules/test_ddl_deparse/test_ddl_deparse.c

@@ -162,9 +162,6 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
 			case AT_ValidateConstraintRecurse:
 				strtype = "VALIDATE CONSTRAINT (and recurse)";
 				break;
-			case AT_ProcessedConstraint:
-				strtype = "ADD (processed) CONSTRAINT";
-				break;
 			case AT_AddIndexConstraint:
 				strtype = "ADD CONSTRAINT (using index)";
 				break;

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
+--------------------+----
+ 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" already 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" already exists
 
 ALTER TABLE test_add_column
 	ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
-	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, skipping
 \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 exists
-	ADD COLUMN IF NOT EXISTS c3 integer; -- skipping because c3 already exists
+	ADD COLUMN IF NOT EXISTS c3 integer primary key; -- skipping because c3 already exists
 NOTICE:  column "c2" of relation "test_add_column" already exists, skipping
 NOTICE:  column "c3" of relation "test_add_column" already exists, skipping
 \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 exists
 	ADD COLUMN IF NOT EXISTS c3 integer, -- skipping because c3 already exists
-	ADD COLUMN c4 integer;
+	ADD COLUMN c4 integer REFERENCES test_add_column;
 NOTICE:  column "c2" of relation "test_add_column" already exists, skipping
 NOTICE:  column "c3" of relation "test_add_column" already exists, skipping
 \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, skipping
+\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_seq'::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, skipping
+\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_seq'::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 =);
+\d ataddindex
+           Table "public.ataddindex"
+ Column | Type | Collation | Nullable | Default 
+--------+------+-----------+----------+---------
+ f1     | text |           |          | 
+Indexes:
+    "ataddindex_expr_excl" EXCLUDE USING btree ((f1 ~~ 'a'::text) WITH =)
+
+DROP TABLE ataddindex;
 -- unsupported constraint types for partitioned tables
 CREATE TABLE partitioned (
 	a int,

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 identity | 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? | Cache 
+--------+-------+---------+---------------------+-----------+---------+-------
+ 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 IDENTITY); -- error

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 exists
-	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 exists
-	ADD COLUMN IF NOT EXISTS c3 integer; -- skipping because c3 already exists
+	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 exists
 	ADD COLUMN IF NOT EXISTS c3 integer, -- skipping because c3 already exists
-	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 =);
+\d ataddindex
+DROP TABLE ataddindex;
 
 -- unsupported constraint types for partitioned tables
 CREATE TABLE partitioned (

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)