Re: Rearranging ALTER TABLE to avoid multi-operations bugs - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: Rearranging ALTER TABLE to avoid multi-operations bugs |
| Date | |
| Msg-id | 19348.1576267346@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: Rearranging ALTER TABLE to avoid multi-operations bugs (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: Rearranging ALTER TABLE to avoid multi-operations bugs
|
| List | pgsql-hackers |
I wrote:
> [ fix-alter-table-order-of-operations-1.patch ]
The cfbot noticed that this failed to apply over a recent commit,
so here's v2. No substantive changes.
regards, tom lane
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index daa80ec..b04ef36 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 = 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 */
@@ -338,31 +342,45 @@ static void validateForeignKeyConstraint(char *conname,
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, LOCKMODE 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, AlterTableCmd *cmd,
- LOCKMODE lockmode);
+ LOCKMODE lockmode,
+ AlterTableUtilityContext *context);
static List *find_typed_table_dependencies(Oid typeOid, const char *typeName,
DropBehavior behavior);
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);
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 *colname,
bool if_not_exists);
static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
@@ -371,7 +389,8 @@ static void ATPrepDropNotNull(Relation rel, bool recurse, 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,
@@ -393,7 +412,8 @@ static ObjectAddress ATExecSetOptions(Relation rel, const char *colName,
static ObjectAddress ATExecSetStorage(Relation rel, const char *colName,
Node *newValue, LOCKMODE lockmode);
static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool 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,
@@ -450,7 +470,8 @@ static void ATExecDropConstraint(Relation rel, const char *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, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
@@ -3460,7 +3481,7 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE 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 requires
@@ -3471,9 +3492,10 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode)
* ATPrepCmd performs phase 1. A "work queue" entry is created for
* each table to be affected (there may be multiple affected tables if the
* commands traverse a table inheritance hierarchy). Also we do preliminary
- * 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 that
* phases 2 and 3 normally do no explicit recursion, since phase 1 already
@@ -3495,18 +3517,23 @@ AlterTableLookupRelation(AlterTableStmt *stmt, LOCKMODE lockmode)
* lock level we want as we recurse might well be higher than required for
* 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 = relation_open(relid, NoLock);
+ rel = 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, context);
}
/*
@@ -3519,6 +3546,10 @@ AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *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)
@@ -3530,7 +3561,7 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse)
EventTriggerAlterTableRelid(relid);
- ATController(NULL, rel, cmds, recurse, lockmode);
+ ATController(NULL, rel, cmds, recurse, lockmode, NULL);
}
/*
@@ -3675,7 +3706,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 */
@@ -3828,7 +3858,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 = NIL;
ListCell *lcmd;
@@ -3838,17 +3869,17 @@ ATController(AlterTableStmt *parsetree,
{
AlterTableCmd *cmd = (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);
}
/*
@@ -3862,7 +3893,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 = AT_PASS_UNSET;
@@ -3874,13 +3906,17 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 column
- * numbers in different children).
+ * numbers in different children). It also ensures that we don't corrupt
+ * the original parse tree, in case it is saved in plancache.
*/
cmd = 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 subcommands
+ * that could try to change the same state in contrary ways.)
*/
switch (cmd->subtype)
{
@@ -3888,14 +3924,14 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 = 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 = AT_PASS_ADD_COL;
break;
@@ -3908,19 +3944,20 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP;
+ pass = 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 = AT_PASS_ADD_CONSTR;
+ pass = AT_PASS_ADD_OTHERCONSTR;
break;
case AT_SetIdentity:
ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
/* This command never recurses */
- pass = AT_PASS_COL_ATTRS;
+ /* This should run after AddIdentity, so do it in MISC pass */
+ pass = AT_PASS_MISC;
break;
case AT_DropIdentity:
ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
@@ -3930,24 +3967,25 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 = 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 = 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 = AT_PASS_COL_ATTRS;
break;
case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */
ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE);
- ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
+ ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
@@ -3959,14 +3997,15 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 = 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 = AT_PASS_DROP;
break;
@@ -3988,7 +4027,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
ATSimplePermissions(rel, ATT_TABLE);
/* This command never recurses */
/* No command-specific prep needed */
- pass = AT_PASS_ADD_CONSTR;
+ pass = AT_PASS_ADD_INDEXCONSTR;
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
@@ -4002,8 +4041,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
ATSimplePermissions(rel,
ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE);
+ /* See comments for ATPrepAlterColumnType */
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, recurse, lockmode,
+ AT_PASS_UNSET, context);
+ Assert(cmd != NULL);
/* Performs own recursion */
- ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd, lockmode);
+ ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd,
+ lockmode, context);
pass = AT_PASS_ALTER_TYPE;
break;
case AT_AlterColumnGenericOptions:
@@ -4026,6 +4070,10 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 = ATPrepChangePersistence(rel, true);
/* force rewrite if necessary; see comment in ATRewriteTables */
if (tab->chgPersistence)
@@ -4037,6 +4085,10 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *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 = ATPrepChangePersistence(rel, false);
/* force rewrite if necessary; see comment in ATRewriteTables */
if (tab->chgPersistence)
@@ -4156,7 +4208,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;
@@ -4189,7 +4242,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
@@ -4226,7 +4279,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 = InvalidObjectAddress;
@@ -4234,22 +4288,28 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
{
case AT_AddColumn: /* ADD COLUMN */
case AT_AddColumnToView: /* add column via CREATE OR REPLACE VIEW */
- address = ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
+ address = ATExecAddColumn(wqueue, tab, rel, &cmd,
false, false,
- cmd->missing_ok, lockmode);
+ lockmode, cur_pass, context);
break;
case AT_AddColumnRecurse:
- address = ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
+ address = ATExecAddColumn(wqueue, tab, rel, &cmd,
true, false,
- cmd->missing_ok, lockmode);
+ lockmode, cur_pass, context);
break;
case AT_ColumnDefault: /* ALTER COLUMN DEFAULT */
address = ATExecColumnDefault(rel, cmd->name, cmd->def, lockmode);
break;
case AT_AddIdentity:
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
+ cur_pass, context);
+ Assert(cmd != NULL);
address = ATExecAddIdentity(rel, cmd->name, cmd->def, lockmode);
break;
case AT_SetIdentity:
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
+ cur_pass, context);
+ Assert(cmd != NULL);
address = ATExecSetIdentity(rel, cmd->name, cmd->def, lockmode);
break;
case AT_DropIdentity:
@@ -4297,14 +4357,24 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
lockmode);
break;
case AT_AddConstraint: /* ADD CONSTRAINT */
- address =
- ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
- false, false, lockmode);
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
+ cur_pass, context);
+ /* Might not have gotten AddConstraint back from parse transform */
+ if (cmd != NULL)
+ address =
+ ATExecAddConstraint(wqueue, tab, rel,
+ (Constraint *) cmd->def,
+ false, false, lockmode);
break;
case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */
- address =
- ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def,
- true, false, lockmode);
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, true, lockmode,
+ cur_pass, context);
+ /* Might not have gotten AddConstraint back from parse transform */
+ if (cmd != NULL)
+ address =
+ ATExecAddConstraint(wqueue, tab, rel,
+ (Constraint *) cmd->def,
+ true, false, lockmode);
break;
case AT_ReAddConstraint: /* Re-add pre-existing check constraint */
address =
@@ -4348,6 +4418,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
cmd->missing_ok, lockmode);
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
+ /* parse transformation was done earlier */
address = ATExecAlterColumnType(tab, rel, cmd, lockmode);
break;
case AT_AlterColumnGenericOptions: /* ALTER COLUMN OPTIONS */
@@ -4470,6 +4541,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
ATExecGenericOptions(rel, (List *) cmd->def);
break;
case AT_AttachPartition:
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
+ cur_pass, context);
+ Assert(cmd != NULL);
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
ATExecAttachPartition(wqueue, rel, (PartitionCmd *) cmd->def);
else
@@ -4477,6 +4551,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
((PartitionCmd *) cmd->def)->name);
break;
case AT_DetachPartition:
+ cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, false, lockmode,
+ cur_pass, context);
+ Assert(cmd != NULL);
/* ATPrepCmd ensures it must be a table */
Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
ATExecDetachPartition(rel, ((PartitionCmd *) cmd->def)->name);
@@ -4490,7 +4567,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation 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 sequence
@@ -4500,10 +4578,143 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation 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 AlterTableCmd
+ * are executed immediately. Those that are supposed to happen afterwards
+ * 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 = NULL;
+ AlterTableStmt *atstmt = makeNode(AlterTableStmt);
+ List *beforeStmts;
+ List *afterStmts;
+ ListCell *lc;
+
+ /* Gin up an AlterTableStmt with just this subcommand and this table */
+ atstmt->relation =
+ makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
+ pstrdup(RelationGetRelationName(rel)),
+ -1);
+ atstmt->relation->inh = recurse;
+ atstmt->cmds = list_make1(cmd);
+ atstmt->relkind = OBJECT_TABLE; /* needn't be picky here */
+ atstmt->missing_ok = false;
+
+ /* Transform the AlterTableStmt */
+ atstmt = transformAlterTableStmt(RelationGetRelid(rel),
+ atstmt,
+ context->queryString,
+ &beforeStmts,
+ &afterStmts);
+
+ /* Execute any statements that should happen before these subcommand(s) */
+ foreach(lc, beforeStmts)
+ {
+ Node *stmt = (Node *) lfirst(lc);
+
+ ProcessUtilityForAlterTable(stmt, context);
+ CommandCounterIncrement();
+ }
+
+ /* Examine the transformed subcommands and schedule them appropriately */
+ foreach(lc, atstmt->cmds)
+ {
+ AlterTableCmd *cmd2 = lfirst_node(AlterTableCmd, lc);
+
+ if (newcmd == NULL &&
+ (cmd->subtype == cmd2->subtype ||
+ (cmd->subtype == AT_AddConstraintRecurse &&
+ cmd2->subtype == AT_AddConstraint)))
+ {
+ /* Found the transformed version of our subcommand */
+ cmd2->subtype = cmd->subtype; /* copy recursion flag */
+ newcmd = 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 = AT_PASS_COL_ATTRS;
+ break;
+ case AT_AddIndex:
+ /* This command never recurses */
+ /* No command-specific prep needed */
+ pass = AT_PASS_ADD_INDEX;
+ break;
+ case AT_AddIndexConstraint:
+ /* This command never recurses */
+ /* No command-specific prep needed */
+ pass = AT_PASS_ADD_INDEXCONSTR;
+ break;
+ case AT_AddConstraint:
+ /* Recursion occurs during execution phase */
+ if (recurse)
+ cmd2->subtype = AT_AddConstraintRecurse;
+ switch (castNode(Constraint, cmd2->def)->contype)
+ {
+ case CONSTR_PRIMARY:
+ case CONSTR_UNIQUE:
+ case CONSTR_EXCLUSION:
+ pass = AT_PASS_ADD_INDEXCONSTR;
+ break;
+ default:
+ pass = AT_PASS_ADD_OTHERCONSTR;
+ break;
+ }
+ break;
+ case AT_AlterColumnGenericOptions:
+ /* This command never recurses */
+ /* No command-specific prep needed */
+ pass = AT_PASS_MISC;
+ break;
+ default:
+ elog(ERROR, "unexpected AlterTableType: %d",
+ (int) cmd2->subtype);
+ pass = AT_PASS_UNSET;
+ break;
+ }
+ /* Must be for a later pass than we're currently doing */
+ if (pass <= cur_pass)
+ elog(ERROR, "ALTER TABLE scheduling failure");
+ tab->subcmds[pass] = lappend(tab->subcmds[pass], cmd2);
+ }
+ }
+
+ /* Queue up any after-statements to happen at the end */
+ tab->afterStmts = list_concat(tab->afterStmts, afterStmts);
+
+ return newcmd;
+}
+
+/*
* ATRewriteTables: ALTER TABLE phase 3
*/
static void
-ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
+ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode,
+ AlterTableUtilityContext *context)
{
ListCell *ltab;
@@ -4730,6 +4941,21 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
if (rel)
table_close(rel, NoLock);
}
+
+ /* Finally, run any afterStmts that were queued up */
+ foreach(ltab, *wqueue)
+ {
+ AlteredTableInfo *tab = (AlteredTableInfo *) lfirst(ltab);
+ ListCell *lc;
+
+ foreach(lc, tab->afterStmts)
+ {
+ Node *stmt = (Node *) lfirst(lc);
+
+ ProcessUtilityForAlterTable(stmt, context);
+ CommandCounterIncrement();
+ }
+ }
}
/*
@@ -4901,8 +5127,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
/*
* Set all columns in the new slot to NULL initially, to ensure
- * columns added as part of the rewrite are initialized to
- * NULL. That is necessary as tab->newvals will not contain an
+ * columns added as part of the rewrite are initialized to NULL.
+ * That is necessary as tab->newvals will not contain an
* expression for columns with a NULL default, e.g. when adding a
* column without a default together with a column with a default
* requiring an actual rewrite.
@@ -5241,7 +5467,8 @@ 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)
{
/*
* Propagate to children if desired. Only plain tables, foreign tables
@@ -5274,7 +5501,7 @@ ATSimpleRecursion(List **wqueue, Relation rel,
/* find_all_inheritors already got lock */
childrel = 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);
}
}
@@ -5319,7 +5546,7 @@ ATCheckPartitionsNotInUse(Relation rel, LOCKMODE lockmode)
*/
static void
ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
- LOCKMODE lockmode)
+ LOCKMODE lockmode, AlterTableUtilityContext *context)
{
ListCell *child;
List *children;
@@ -5337,7 +5564,7 @@ ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
childrel = 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);
}
}
@@ -5574,7 +5801,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,
@@ -5582,7 +5810,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
errmsg("cannot add column to typed table")));
if (rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
+ ATTypedTableRecursion(wqueue, rel, cmd, lockmode, context);
if (recurse && !is_view)
cmd->subtype = AT_AddColumnRecurse;
@@ -5591,14 +5819,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-transformed
+ * 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 = RelationGetRelid(rel);
+ ColumnDef *colDef = castNode(ColumnDef, (*cmd)->def);
+ bool if_not_exists = (*cmd)->missing_ok;
Relation pgclass,
attrdesc;
HeapTuple reltup;
@@ -5613,6 +5847,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
Expr *defval;
List *children;
ListCell *child;
+ AlterTableCmd *childcmd;
AclResult aclresult;
ObjectAddress address;
@@ -5680,12 +5915,31 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
}
}
- pgclass = 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 = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid));
- if (!HeapTupleIsValid(reltup))
- elog(ERROR, "cache lookup failed for relation %u", myrelid);
- relkind = ((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 != NULL && !recursing)
+ {
+ *cmd = ATParseTransformCmd(wqueue, tab, rel, *cmd, recurse, lockmode,
+ cur_pass, context);
+ Assert(*cmd != NULL);
+ colDef = castNode(ColumnDef, (*cmd)->def);
+ }
/*
* Cannot add identity column if table has children, because identity does
@@ -5698,14 +5952,12 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("cannot recursively add identity column to table that has child 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 = table_open(RelationRelationId, RowExclusiveLock);
+
+ reltup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid));
+ if (!HeapTupleIsValid(reltup))
+ elog(ERROR, "cache lookup failed for relation %u", myrelid);
+ relkind = ((Form_pg_class) GETSTRUCT(reltup))->relkind;
/* Determine the new attribute's number */
newattnum = ((Form_pg_class) GETSTRUCT(reltup))->relnatts + 1;
@@ -5935,10 +6187,13 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
/* Children should see column as singly inherited */
if (!recursing)
{
- colDef = copyObject(colDef);
+ childcmd = copyObject(*cmd);
+ colDef = castNode(ColumnDef, childcmd->def);
colDef->inhcount = 1;
colDef->is_local = false;
}
+ else
+ childcmd = *cmd; /* no need to copy again */
foreach(child, children)
{
@@ -5955,8 +6210,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);
}
@@ -6215,7 +6470,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
@@ -6236,10 +6491,10 @@ ATPrepSetNotNull(List **wqueue, Relation rel,
newcmd->subtype = AT_CheckNotNull;
newcmd->name = 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);
}
/*
@@ -6979,7 +7234,8 @@ ATExecSetStorage(Relation rel, const char *colName, Node *newValue, LOCKMODE loc
*/
static void
ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
- AlterTableCmd *cmd, LOCKMODE lockmode)
+ AlterTableCmd *cmd, LOCKMODE lockmode,
+ AlterTableUtilityContext *context)
{
if (rel->rd_rel->reloftype && !recursing)
ereport(ERROR,
@@ -6987,7 +7243,7 @@ ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
errmsg("cannot drop column from typed table")));
if (rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
+ ATTypedTableRecursion(wqueue, rel, cmd, lockmode, context);
if (recurse)
cmd->subtype = AT_DropColumnRecurse;
@@ -10239,12 +10495,27 @@ ATExecDropConstraint(Relation rel, const char *constrName,
/*
* ALTER COLUMN TYPE
+ *
+ * Unlike other subcommand types, we do parse transformation for ALTER COLUMN
+ * 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 dropped.
+ *
+ * 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 effects
+ * 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 = cmd->name;
ColumnDef *def = (ColumnDef *) cmd->def;
@@ -10490,7 +10761,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);
}
}
@@ -10502,7 +10773,7 @@ ATPrepAlterColumnType(List **wqueue,
colName)));
if (tab->relkind == RELKIND_COMPOSITE_TYPE)
- ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
+ ATTypedTableRecursion(wqueue, rel, cmd, lockmode, context);
}
/*
@@ -11281,10 +11552,19 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
(IndexStmt *) stmt,
cmd));
else if (IsA(stmt, AlterTableStmt))
- querytree_list = list_concat(querytree_list,
- transformAlterTableStmt(oldRelId,
- (AlterTableStmt *) stmt,
- cmd));
+ {
+ List *beforeStmts;
+ List *afterStmts;
+
+ stmt = (Node *) transformAlterTableStmt(oldRelId,
+ (AlterTableStmt *) stmt,
+ cmd,
+ &beforeStmts,
+ &afterStmts);
+ querytree_list = list_concat(querytree_list, beforeStmts);
+ querytree_list = lappend(querytree_list, stmt);
+ querytree_list = list_concat(querytree_list, afterStmts);
+ }
else
querytree_list = lappend(querytree_list, stmt);
}
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 9b51480..73b3e60 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 b761fdf..77c4fc8 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/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,7 +489,10 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,
(Node *) attnamelist, -1));
altseqstmt->for_identity = for_identity;
- cxt->alist = lappend(cxt->alist, altseqstmt);
+ if (col_exists)
+ cxt->blist = lappend(cxt->blist, altseqstmt);
+ else
+ cxt->alist = lappend(cxt->alist, altseqstmt);
if (snamespace_p)
*snamespace_p = snamespace;
@@ -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);
@@ -3002,23 +3013,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;
@@ -3050,7 +3061,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;
@@ -3078,9 +3089,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)
{
@@ -3089,7 +3099,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
switch (cmd->subtype)
{
case AT_AddColumn:
- case AT_AddColumnToView:
+ case AT_AddColumnRecurse:
{
ColumnDef *def = castNode(ColumnDef, cmd->def);
@@ -3113,6 +3123,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
}
case AT_AddConstraint:
+ case AT_AddConstraintRecurse:
/*
* The original AddConstraint cmd node doesn't go to newcmds
@@ -3128,19 +3139,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;
/*
@@ -3159,13 +3160,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);
@@ -3194,16 +3195,16 @@ 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,
- NULL, NULL);
+ generateSerialExtraStmts(&cxt, newdef,
+ get_atttype(relid, attnum),
+ def->options, true, true,
+ NULL, NULL);
newcmds = lappend(newcmds, cmd);
break;
@@ -3219,6 +3220,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
@@ -3235,29 +3237,34 @@ 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)
- {
- AlterSeqStmt *seqstmt;
+ if (seq_relid)
+ {
+ AlterSeqStmt *seqstmt;
- seqstmt = makeNode(AlterSeqStmt);
- seqstmt->sequence = makeRangeVar(get_namespace_name(get_rel_namespace(seq_relid)),
- get_rel_name(seq_relid), -1);
- seqstmt->options = newseqopts;
- seqstmt->for_identity = true;
- seqstmt->missing_ok = false;
+ seqstmt = makeNode(AlterSeqStmt);
+ seqstmt->sequence = makeRangeVar(get_namespace_name(get_rel_namespace(seq_relid)),
+ get_rel_name(seq_relid), -1);
+ seqstmt->options = newseqopts;
+ 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;
@@ -3279,6 +3286,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;
}
@@ -3359,11 +3372,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;
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 3a03ca7..ad16f8a 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1092,8 +1092,6 @@ ProcessUtilitySlow(ParseState *pstate,
{
AlterTableStmt *atstmt = (AlterTableStmt *) parsetree;
Oid relid;
- List *stmts;
- ListCell *l;
LOCKMODE lockmode;
/*
@@ -1107,59 +1105,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();
@@ -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 = 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
*/
static void
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 9c25a80..7245f83 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 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);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index ff626cb..c5ea5c9 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1780,8 +1780,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 1348064..7d640cf 100644
--- a/src/include/parser/parse_utilcmd.h
+++ b/src/include/parser/parse_utilcmd.h
@@ -18,8 +18,10 @@
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,
diff --git a/src/include/tcop/utility.h b/src/include/tcop/utility.h
index 5abcacf..d8f5d4f 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/test/modules/test_ddl_deparse/test_ddl_deparse.c
index 7f77f19..04d963b 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 = "VALIDATE CONSTRAINT (and recurse)";
break;
- case AT_ProcessedConstraint:
- strtype = "ADD (processed) CONSTRAINT";
- break;
case AT_AddIndexConstraint:
strtype = "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..45217d0 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" 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,84 @@ 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;
+\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)
+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;
+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)
+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*
-- unsupported constraint types for partitioned tables
CREATE TABLE partitioned (
a int,
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/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 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
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index abe7be3..25d9717 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,32 @@ 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;
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c5 SERIAL;
+\d test_add_column*
DROP TABLE test_add_column;
+\d test_add_column*
-- unsupported constraint types for partitioned tables
CREATE TABLE partitioned (
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.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)
pgsql-hackers by date: