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

1 files changed, 13 insertions, 0 deletions

diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index 40551c45af2..f62bfc44173 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);