aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/backend/catalog/index.c47
-rw-r--r--src/backend/commands/tablecmds.c126
-rw-r--r--src/backend/parser/parse_utilcmd.c176
-rw-r--r--src/include/nodes/parsenodes.h1
-rw-r--r--src/include/parser/parse_utilcmd.h2
-rw-r--r--src/test/modules/test_ddl_deparse/expected/alter_table.out3
-rw-r--r--src/test/modules/test_ddl_deparse/expected/create_table.out2
-rw-r--r--src/test/modules/test_ddl_deparse/test_ddl_deparse.c3
-rw-r--r--src/test/regress/expected/alter_table.out16
-rw-r--r--src/test/regress/expected/identity.out12
-rw-r--r--src/test/regress/expected/indexing.out15
-rw-r--r--src/test/regress/sql/alter_table.sql8
-rw-r--r--src/test/regress/sql/identity.sql6
-rw-r--r--src/test/regress/sql/indexing.sql12
14 files changed, 320 insertions, 109 deletions
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index e9399bef14c..331f90528cd 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -185,13 +185,14 @@ relationHasPrimaryKey(Relation rel)
*
* We check for a pre-existing primary key, and that all columns of the index
* are simple column references (not expressions), and that all those
- * columns are marked NOT NULL. If they aren't (which can only happen during
- * ALTER TABLE ADD CONSTRAINT, since the parser forces such columns to be
- * created NOT NULL during CREATE TABLE), do an ALTER SET NOT NULL to mark
- * them so --- or fail if they are not in fact nonnull.
+ * columns are marked NOT NULL. If not, fail.
*
- * As of PG v10, the SET NOT NULL is applied to child tables as well, so
- * that the behavior is like a manual SET NOT NULL.
+ * We used to automatically change unmarked columns to NOT NULL here by doing
+ * our own local ALTER TABLE command. But that doesn't work well if we're
+ * executing one subcommand of an ALTER TABLE: the operations may not get
+ * performed in the right order overall. Now we expect that the parser
+ * inserted any required ALTER TABLE SET NOT NULL operations before trying
+ * to create a primary-key index.
*
* Caller had better have at least ShareLock on the table, else the not-null
* checking isn't trustworthy.
@@ -202,12 +203,11 @@ index_check_primary_key(Relation heapRel,
bool is_alter_table,
IndexStmt *stmt)
{
- List *cmds;
int i;
/*
- * If ALTER TABLE and CREATE TABLE .. PARTITION OF, check that there isn't
- * already a PRIMARY KEY. In CREATE TABLE for an ordinary relations, we
+ * If ALTER TABLE or CREATE TABLE .. PARTITION OF, check that there isn't
+ * already a PRIMARY KEY. In CREATE TABLE for an ordinary relation, we
* have faith that the parser rejected multiple pkey clauses; and CREATE
* INDEX doesn't have a way to say PRIMARY KEY, so it's no problem either.
*/
@@ -222,9 +222,9 @@ index_check_primary_key(Relation heapRel,
/*
* Check that all of the attributes in a primary key are marked as not
- * null, otherwise attempt to ALTER TABLE .. SET NOT NULL
+ * null. (We don't really expect to see that; it'd mean the parser messed
+ * up. But it seems wise to check anyway.)
*/
- cmds = NIL;
for (i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
{
AttrNumber attnum = indexInfo->ii_IndexAttrNumbers[i];
@@ -249,30 +249,13 @@ index_check_primary_key(Relation heapRel,
attform = (Form_pg_attribute) GETSTRUCT(atttuple);
if (!attform->attnotnull)
- {
- /* Add a subcommand to make this one NOT NULL */
- AlterTableCmd *cmd = makeNode(AlterTableCmd);
-
- cmd->subtype = AT_SetNotNull;
- cmd->name = pstrdup(NameStr(attform->attname));
- cmds = lappend(cmds, cmd);
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("primary key column \"%s\" is not marked NOT NULL",
+ NameStr(attform->attname))));
ReleaseSysCache(atttuple);
}
-
- /*
- * XXX: possible future improvement: when being called from ALTER TABLE,
- * it would be more efficient to merge this with the outer ALTER TABLE, so
- * as to avoid two scans. But that seems to complicate DefineIndex's API
- * unduly.
- */
- if (cmds)
- {
- EventTriggerAlterTableStart((Node *) stmt);
- AlterTableInternal(RelationGetRelid(heapRel), cmds, true);
- EventTriggerAlterTableEnd();
- }
}
/*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d48a947f7c6..aa7328ea400 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -142,9 +142,9 @@ static List *on_commits = NIL;
#define AT_PASS_ALTER_TYPE 1 /* ALTER COLUMN TYPE */
#define AT_PASS_OLD_INDEX 2 /* re-add existing indexes */
#define AT_PASS_OLD_CONSTR 3 /* re-add existing constraints */
-#define AT_PASS_COL_ATTRS 4 /* set other column attributes */
/* We could support a RENAME COLUMN pass here, but not currently used */
-#define AT_PASS_ADD_COL 5 /* ADD COLUMN */
+#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 */
@@ -370,9 +370,13 @@ static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid);
static void ATPrepDropNotNull(Relation rel, bool recurse, bool recursing);
static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode);
-static void ATPrepSetNotNull(Relation rel, bool recurse, bool recursing);
+static void ATPrepSetNotNull(List **wqueue, Relation rel,
+ AlterTableCmd *cmd, bool recurse, bool recursing,
+ LOCKMODE lockmode);
static ObjectAddress ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
const char *colName, LOCKMODE lockmode);
+static void ATExecCheckNotNull(AlteredTableInfo *tab, Relation rel,
+ const char *colName, LOCKMODE lockmode);
static bool NotNullImpliedByRelConstraints(Relation rel, Form_pg_attribute attr);
static bool ConstraintImpliedByRelConstraint(Relation scanrel,
List *partConstraint, List *existedConstraints);
@@ -1068,7 +1072,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
RelationGetDescr(parent),
gettext_noop("could not convert row type"));
idxstmt =
- generateClonedIndexStmt(NULL, RelationGetRelid(rel), idxRel,
+ generateClonedIndexStmt(NULL, idxRel,
attmap, RelationGetDescr(rel)->natts,
&constraintOid);
DefineIndex(RelationGetRelid(rel),
@@ -3765,6 +3769,15 @@ AlterTableGetLockLevel(List *cmds)
cmd_lockmode = AccessExclusiveLock;
break;
+ case AT_CheckNotNull:
+
+ /*
+ * This only examines the table's schema; but lock must be
+ * strong enough to prevent concurrent DROP NOT NULL.
+ */
+ cmd_lockmode = AccessShareLock;
+ break;
+
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
(int) cmd->subtype);
@@ -3889,15 +3902,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
ATPrepDropNotNull(rel, recurse, recursing);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
- /* No command-specific prep needed */
pass = AT_PASS_DROP;
break;
case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */
ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
- ATPrepSetNotNull(rel, recurse, recursing);
+ /* Need command-specific recursion decision */
+ ATPrepSetNotNull(wqueue, rel, cmd, recurse, recursing, lockmode);
+ 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);
/* No command-specific prep needed */
- pass = AT_PASS_ADD_CONSTR;
+ pass = AT_PASS_COL_ATTRS;
break;
case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
@@ -4214,6 +4231,9 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */
address = ATExecSetNotNull(tab, rel, cmd->name, lockmode);
break;
+ case AT_CheckNotNull: /* check column is already marked NOT NULL */
+ ATExecCheckNotNull(tab, rel, cmd->name, lockmode);
+ break;
case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */
address = ATExecSetStatistics(rel, cmd->name, cmd->num, cmd->def, lockmode);
break;
@@ -5966,9 +5986,6 @@ add_column_collation_dependency(Oid relid, int32 attnum, Oid collid)
/*
* ALTER TABLE ALTER COLUMN DROP NOT NULL
- *
- * Return the address of the modified column. If the column was already
- * nullable, InvalidObjectAddress is returned.
*/
static void
@@ -5990,6 +6007,11 @@ ATPrepDropNotNull(Relation rel, bool recurse, bool recursing)
errhint("Do not specify the ONLY keyword.")));
}
}
+
+/*
+ * Return the address of the modified column. If the column was already
+ * nullable, InvalidObjectAddress is returned.
+ */
static ObjectAddress
ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
{
@@ -6116,23 +6138,33 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
*/
static void
-ATPrepSetNotNull(Relation rel, bool recurse, bool recursing)
+ATPrepSetNotNull(List **wqueue, Relation rel,
+ AlterTableCmd *cmd, bool recurse, bool recursing,
+ LOCKMODE lockmode)
{
/*
- * If the parent is a partitioned table, like check constraints, NOT NULL
- * constraints must be added to the child tables. Complain if requested
- * otherwise and partitions exist.
+ * If we're already recursing, there's nothing to do; the topmost
+ * invocation of ATSimpleRecursion already visited all children.
*/
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ if (recursing)
+ return;
+
+ /*
+ * If we have ALTER TABLE ONLY ... SET NOT NULL on a partitioned table,
+ * apply ALTER TABLE ... CHECK NOT NULL to every child. Otherwise, use
+ * normal recursion logic.
+ */
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+ !recurse)
{
- PartitionDesc partdesc = RelationGetPartitionDesc(rel);
+ AlterTableCmd *newcmd = makeNode(AlterTableCmd);
- if (partdesc && partdesc->nparts > 0 && !recurse && !recursing)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot add constraint to only the partitioned table when partitions exist"),
- errhint("Do not specify the ONLY keyword.")));
+ newcmd->subtype = AT_CheckNotNull;
+ newcmd->name = pstrdup(cmd->name);
+ ATSimpleRecursion(wqueue, rel, newcmd, true, lockmode);
}
+ else
+ ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
}
/*
@@ -6208,6 +6240,46 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
}
/*
+ * ALTER TABLE ALTER COLUMN CHECK NOT NULL
+ *
+ * This doesn't exist in the grammar, but we generate AT_CheckNotNull
+ * commands against the partitions of a partitioned table if the user
+ * writes ALTER TABLE ONLY ... SET NOT NULL on the partitioned table,
+ * or tries to create a primary key on it (which internally creates
+ * AT_SetNotNull on the partitioned table). Such a command doesn't
+ * allow us to actually modify any partition, but we want to let it
+ * go through if the partitions are already properly marked.
+ *
+ * In future, this might need to adjust the child table's state, likely
+ * by incrementing an inheritance count for the attnotnull constraint.
+ * For now we need only check for the presence of the flag.
+ */
+static void
+ATExecCheckNotNull(AlteredTableInfo *tab, Relation rel,
+ const char *colName, LOCKMODE lockmode)
+{
+ HeapTuple tuple;
+
+ tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
+
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_COLUMN),
+ errmsg("column \"%s\" of relation \"%s\" does not exist",
+ colName, RelationGetRelationName(rel))));
+
+ if (!((Form_pg_attribute) GETSTRUCT(tuple))->attnotnull)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("constraint must be added to child tables too"),
+ errdetail("Column \"%s\" of relation \"%s\" is not already NOT NULL.",
+ colName, RelationGetRelationName(rel)),
+ errhint("Do not specify the ONLY keyword.")));
+
+ ReleaseSysCache(tuple);
+}
+
+/*
* NotNullImpliedByRelConstraints
* Does rel's existing constraints imply NOT NULL for the given attribute?
*/
@@ -11269,6 +11341,16 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
NIL,
con->conname);
}
+ else if (cmd->subtype == AT_SetNotNull)
+ {
+ /*
+ * The parser will create AT_SetNotNull subcommands for
+ * columns of PRIMARY KEY indexes/constraints, but we need
+ * not do anything with them here, because the columns'
+ * NOT NULL marks will already have been propagated into
+ * the new table definition.
+ */
+ }
else
elog(ERROR, "unexpected statement subtype: %d",
(int) cmd->subtype);
@@ -15649,7 +15731,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
IndexStmt *stmt;
Oid constraintOid;
- stmt = generateClonedIndexStmt(NULL, RelationGetRelid(attachrel),
+ stmt = generateClonedIndexStmt(NULL,
idxRel, attmap,
RelationGetDescr(rel)->natts,
&constraintOid);
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 674f4b98f40..23996904c47 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -293,8 +293,10 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
}
/*
- * transformIndexConstraints wants cxt.alist to contain only index
- * statements, so transfer anything we already have into save_alist.
+ * Transfer anything we already have in cxt.alist into save_alist, to keep
+ * it separate from the output of transformIndexConstraints. (This may
+ * not be necessary anymore, but we'll keep doing it to preserve the
+ * historical order of execution of the alist commands.)
*/
save_alist = cxt.alist;
cxt.alist = NIL;
@@ -1196,9 +1198,10 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
parent_index = index_open(parent_index_oid, AccessShareLock);
/* Build CREATE INDEX statement to recreate the parent_index */
- index_stmt = generateClonedIndexStmt(cxt->relation, InvalidOid,
+ index_stmt = generateClonedIndexStmt(cxt->relation,
parent_index,
- attmap, tupleDesc->natts, NULL);
+ attmap, tupleDesc->natts,
+ NULL);
/* Copy comment on index, if requested */
if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
@@ -1311,13 +1314,26 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
/*
* Generate an IndexStmt node using information from an already existing index
- * "source_idx", for the rel identified either by heapRel or heapRelid.
+ * "source_idx".
*
- * Attribute numbers should be adjusted according to attmap.
+ * heapRel is stored into the IndexStmt's relation field, but we don't use it
+ * otherwise; some callers pass NULL, if they don't need it to be valid.
+ * (The target relation might not exist yet, so we mustn't try to access it.)
+ *
+ * Attribute numbers in expression Vars are adjusted according to attmap.
+ *
+ * If constraintOid isn't NULL, we store the OID of any constraint associated
+ * with the index there.
+ *
+ * Unlike transformIndexConstraint, we don't make any effort to force primary
+ * key columns to be NOT NULL. The larger cloning process this is part of
+ * should have cloned their NOT NULL status separately (and DefineIndex will
+ * complain if that fails to happen).
*/
IndexStmt *
-generateClonedIndexStmt(RangeVar *heapRel, Oid heapRelid, Relation source_idx,
- const AttrNumber *attmap, int attmap_length, Oid *constraintOid)
+generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
+ const AttrNumber *attmap, int attmap_length,
+ Oid *constraintOid)
{
Oid source_relid = RelationGetRelid(source_idx);
HeapTuple ht_idxrel;
@@ -1337,8 +1353,8 @@ generateClonedIndexStmt(RangeVar *heapRel, Oid heapRelid, Relation source_idx,
Datum datum;
bool isnull;
- Assert((heapRel == NULL && OidIsValid(heapRelid)) ||
- (heapRel != NULL && !OidIsValid(heapRelid)));
+ if (constraintOid)
+ *constraintOid = InvalidOid;
/*
* Fetch pg_class tuple of source index. We can't use the copy in the
@@ -1821,6 +1837,7 @@ transformIndexConstraints(CreateStmtContext *cxt)
{
IndexStmt *index;
List *indexlist = NIL;
+ List *finalindexlist = NIL;
ListCell *lc;
/*
@@ -1869,11 +1886,10 @@ transformIndexConstraints(CreateStmtContext *cxt)
* XXX in ALTER TABLE case, it'd be nice to look for duplicate
* pre-existing indexes, too.
*/
- Assert(cxt->alist == NIL);
if (cxt->pkey != NULL)
{
/* Make sure we keep the PKEY index in preference to others... */
- cxt->alist = list_make1(cxt->pkey);
+ finalindexlist = list_make1(cxt->pkey);
}
foreach(lc, indexlist)
@@ -1883,11 +1899,11 @@ transformIndexConstraints(CreateStmtContext *cxt)
index = lfirst(lc);
- /* if it's pkey, it's already in cxt->alist */
+ /* if it's pkey, it's already in finalindexlist */
if (index == cxt->pkey)
continue;
- foreach(k, cxt->alist)
+ foreach(k, finalindexlist)
{
IndexStmt *priorindex = lfirst(k);
@@ -1915,19 +1931,32 @@ transformIndexConstraints(CreateStmtContext *cxt)
}
if (keep)
- cxt->alist = lappend(cxt->alist, index);
+ finalindexlist = lappend(finalindexlist, index);
}
+
+ /*
+ * Now append all the IndexStmts to cxt->alist. If we generated an ALTER
+ * TABLE SET NOT NULL statement to support a primary key, it's already in
+ * cxt->alist.
+ */
+ cxt->alist = list_concat(cxt->alist, finalindexlist);
}
/*
* transformIndexConstraint
* Transform one UNIQUE, PRIMARY KEY, or EXCLUDE constraint for
* transformIndexConstraints.
+ *
+ * We return an IndexStmt. For a PRIMARY KEY constraint, we additionally
+ * produce NOT NULL constraints, either by marking ColumnDefs in cxt->columns
+ * as is_not_null or by adding an ALTER TABLE SET NOT NULL command to
+ * cxt->alist.
*/
static IndexStmt *
transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
{
IndexStmt *index;
+ List *notnullcmds = NIL;
ListCell *lc;
index = makeNode(IndexStmt);
@@ -2170,9 +2199,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
* For UNIQUE and PRIMARY KEY, we just have a list of column names.
*
* Make sure referenced keys exist. If we are making a PRIMARY KEY index,
- * also make sure they are NOT NULL, if possible. (Although we could leave
- * it to DefineIndex to mark the columns NOT NULL, it's more efficient to
- * get it right the first time.)
+ * also make sure they are NOT NULL.
*/
else
{
@@ -2180,11 +2207,12 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
{
char *key = strVal(lfirst(lc));
bool found = false;
+ bool forced_not_null = false;
ColumnDef *column = NULL;
ListCell *columns;
IndexElem *iparam;
- /* Make sure referenced column exist. */
+ /* Make sure referenced column exists. */
foreach(columns, cxt->columns)
{
column = castNode(ColumnDef, lfirst(columns));
@@ -2196,9 +2224,18 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
}
if (found)
{
- /* found column in the new table; force it to be NOT NULL */
- if (constraint->contype == CONSTR_PRIMARY)
+ /*
+ * column is defined in the new table. For PRIMARY KEY, we
+ * can apply the NOT NULL constraint cheaply here ... unless
+ * the column is marked is_from_type, in which case marking it
+ * here would be ineffective (see MergeAttributes).
+ */
+ if (constraint->contype == CONSTR_PRIMARY &&
+ !column->is_from_type)
+ {
column->is_not_null = true;
+ forced_not_null = true;
+ }
}
else if (SystemAttributeByName(key) != NULL)
{
@@ -2242,10 +2279,11 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
found = true;
/*
- * We currently have no easy way to force an
- * inherited column to be NOT NULL at creation, if
- * its parent wasn't so already. We leave it to
- * DefineIndex to fix things up in this case.
+ * It's tempting to set forced_not_null if the
+ * parent column is already NOT NULL, but that
+ * seems unsafe because the column's NOT NULL
+ * marking might disappear between now and
+ * execution. Do the runtime check to be safe.
*/
break;
}
@@ -2259,8 +2297,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
/*
* In the ALTER TABLE case, don't complain about index keys not
* created in the command; they may well exist already.
- * DefineIndex will complain about them if not, and will also take
- * care of marking them NOT NULL.
+ * DefineIndex will complain about them if not.
*/
if (!found && !cxt->isalter)
ereport(ERROR,
@@ -2299,10 +2336,29 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
iparam->ordering = SORTBY_DEFAULT;
iparam->nulls_ordering = SORTBY_NULLS_DEFAULT;
index->indexParams = lappend(index->indexParams, iparam);
+
+ /*
+ * For a primary-key column, also create an item for ALTER TABLE
+ * SET NOT NULL if we couldn't ensure it via is_not_null above.
+ */
+ if (constraint->contype == CONSTR_PRIMARY && !forced_not_null)
+ {
+ AlterTableCmd *notnullcmd = makeNode(AlterTableCmd);
+
+ notnullcmd->subtype = AT_SetNotNull;
+ notnullcmd->name = pstrdup(key);
+ notnullcmds = lappend(notnullcmds, notnullcmd);
+ }
}
}
- /* Add included columns to index definition */
+ /*
+ * Add included columns to index definition. This is much like the
+ * simple-column-name-list code above, except that we don't worry about
+ * NOT NULL marking; included columns in a primary key should not be
+ * forced NOT NULL. We don't complain about duplicate columns, either,
+ * though maybe we should?
+ */
foreach(lc, constraint->including)
{
char *key = strVal(lfirst(lc));
@@ -2327,8 +2383,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
{
/*
* column will be a system column in the new table, so accept
- * it. System columns can't ever be null, so no need to worry
- * about PRIMARY/NOT NULL constraint.
+ * it.
*/
found = true;
}
@@ -2363,13 +2418,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
if (strcmp(key, inhname) == 0)
{
found = true;
-
- /*
- * We currently have no easy way to force an
- * inherited column to be NOT NULL at creation, if
- * its parent wasn't so already. We leave it to
- * DefineIndex to fix things up in this case.
- */
break;
}
}
@@ -2383,8 +2431,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
/*
* In the ALTER TABLE case, don't complain about index keys not
* created in the command; they may well exist already. DefineIndex
- * will complain about them if not, and will also take care of marking
- * them NOT NULL.
+ * will complain about them if not.
*/
if (!found && !cxt->isalter)
ereport(ERROR,
@@ -2402,6 +2449,22 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
index->indexIncludingParams = lappend(index->indexIncludingParams, iparam);
}
+ /*
+ * If we found anything that requires run-time SET NOT NULL, build a full
+ * ALTER TABLE command for that and add it to cxt->alist.
+ */
+ if (notnullcmds)
+ {
+ AlterTableStmt *alterstmt = makeNode(AlterTableStmt);
+
+ alterstmt->relation = copyObject(cxt->relation);
+ alterstmt->cmds = notnullcmds;
+ alterstmt->relkind = OBJECT_TABLE;
+ alterstmt->missing_ok = false;
+
+ cxt->alist = lappend(cxt->alist, alterstmt);
+ }
+
return index;
}
@@ -3220,9 +3283,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
}
/*
- * transformIndexConstraints wants cxt.alist to contain only index
- * statements, so transfer anything we already have into save_alist
- * immediately.
+ * Transfer anything we already have in cxt.alist into save_alist, to keep
+ * it separate from the output of transformIndexConstraints.
*/
save_alist = cxt.alist;
cxt.alist = NIL;
@@ -3240,13 +3302,31 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
*/
foreach(l, cxt.alist)
{
- IndexStmt *idxstmt = lfirst_node(IndexStmt, l);
+ Node *istmt = (Node *) lfirst(l);
- idxstmt = transformIndexStmt(relid, idxstmt, queryString);
- newcmd = makeNode(AlterTableCmd);
- newcmd->subtype = OidIsValid(idxstmt->indexOid) ? AT_AddIndexConstraint : AT_AddIndex;
- newcmd->def = (Node *) idxstmt;
- newcmds = lappend(newcmds, newcmd);
+ /*
+ * We assume here that cxt.alist contains only IndexStmts and possibly
+ * ALTER TABLE SET NOT NULL statements generated from primary key
+ * constraints. We absorb the subcommands of the latter directly.
+ */
+ if (IsA(istmt, IndexStmt))
+ {
+ IndexStmt *idxstmt = (IndexStmt *) istmt;
+
+ idxstmt = transformIndexStmt(relid, idxstmt, queryString);
+ newcmd = makeNode(AlterTableCmd);
+ newcmd->subtype = OidIsValid(idxstmt->indexOid) ? AT_AddIndexConstraint : AT_AddIndex;
+ newcmd->def = (Node *) idxstmt;
+ newcmds = lappend(newcmds, newcmd);
+ }
+ else if (IsA(istmt, AlterTableStmt))
+ {
+ AlterTableStmt *alterstmt = (AlterTableStmt *) istmt;
+
+ newcmds = list_concat(newcmds, alterstmt->cmds);
+ }
+ else
+ elog(ERROR, "unexpected stmt type %d", (int) nodeTag(istmt));
}
cxt.alist = NIL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 94c0b7a9dd5..462237d588f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1764,6 +1764,7 @@ typedef enum AlterTableType
AT_ColumnDefault, /* alter column default */
AT_DropNotNull, /* alter column drop not null */
AT_SetNotNull, /* alter column set not null */
+ AT_CheckNotNull, /* check column is already marked not null */
AT_SetStatistics, /* alter column set statistics */
AT_SetOptions, /* alter column set ( options ) */
AT_ResetOptions, /* alter column reset ( options ) */
diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h
index 09a99d9479f..6928aefb06d 100644
--- a/src/include/parser/parse_utilcmd.h
+++ b/src/include/parser/parse_utilcmd.h
@@ -27,7 +27,7 @@ extern void transformRuleStmt(RuleStmt *stmt, const char *queryString,
extern List *transformCreateSchemaStmt(CreateSchemaStmt *stmt);
extern PartitionBoundSpec *transformPartitionBound(ParseState *pstate, Relation parent,
PartitionBoundSpec *spec);
-extern IndexStmt *generateClonedIndexStmt(RangeVar *heapRel, Oid heapOid,
+extern IndexStmt *generateClonedIndexStmt(RangeVar *heapRel,
Relation source_idx,
const AttrNumber *attmap, int attmap_length,
Oid *constraintOid);
diff --git a/src/test/modules/test_ddl_deparse/expected/alter_table.out b/src/test/modules/test_ddl_deparse/expected/alter_table.out
index 7da847d49e5..141060fbdcf 100644
--- a/src/test/modules/test_ddl_deparse/expected/alter_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/alter_table.out
@@ -23,8 +23,7 @@ NOTICE: DDL test: type simple, tag CREATE TABLE
CREATE TABLE part1 PARTITION OF part FOR VALUES FROM (1) to (100);
NOTICE: DDL test: type simple, tag CREATE TABLE
ALTER TABLE part ADD PRIMARY KEY (a);
-NOTICE: DDL test: type alter table, tag CREATE INDEX
+NOTICE: DDL test: type alter table, tag ALTER TABLE
NOTICE: subcommand: SET NOT NULL
NOTICE: subcommand: SET NOT NULL
-NOTICE: DDL test: type alter table, tag ALTER TABLE
NOTICE: subcommand: ADD INDEX
diff --git a/src/test/modules/test_ddl_deparse/expected/create_table.out b/src/test/modules/test_ddl_deparse/expected/create_table.out
index 2d7dfd533e4..523c9960933 100644
--- a/src/test/modules/test_ddl_deparse/expected/create_table.out
+++ b/src/test/modules/test_ddl_deparse/expected/create_table.out
@@ -85,6 +85,8 @@ CREATE TABLE employees OF employee_type (
salary WITH OPTIONS DEFAULT 1000
);
NOTICE: DDL test: type simple, tag CREATE TABLE
+NOTICE: DDL test: type alter table, tag ALTER TABLE
+NOTICE: subcommand: SET NOT NULL
NOTICE: DDL test: type simple, tag CREATE INDEX
-- Inheritance
CREATE TABLE person (
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 2fe0c24cf4e..7f77f194407 100644
--- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
+++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c
@@ -117,6 +117,9 @@ get_altertable_subcmdtypes(PG_FUNCTION_ARGS)
case AT_SetNotNull:
strtype = "SET NOT NULL";
break;
+ case AT_CheckNotNull:
+ strtype = "CHECK NOT NULL";
+ break;
case AT_SetStatistics:
strtype = "SET STATS";
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 2a26aa3a894..3e9d7175b42 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -978,7 +978,7 @@ drop table atacc1;
create table atacc1 ( test int );
-- add a primary key constraint (fails)
alter table atacc1 add constraint atacc_test1 primary key (test1);
-ERROR: column "test1" named in key does not exist
+ERROR: column "test1" of relation "atacc1" does not exist
drop table atacc1;
-- adding a new column as primary key to a non-empty table.
-- should fail unless the column has a non-null default value.
@@ -990,6 +990,13 @@ ERROR: column "test2" contains null values
-- now add a primary key column with a default (succeeds).
alter table atacc1 add column test2 int default 0 primary key;
drop table atacc1;
+-- this combination used to have order-of-execution problems (bug #15580)
+create table atacc1 (a int);
+insert into atacc1 values(1);
+alter table atacc1
+ add column b float8 not null default random(),
+ add primary key(a);
+drop table atacc1;
-- something a little more complicated
create table atacc1 ( test int, test2 int);
-- add a primary key constraint
@@ -1404,9 +1411,9 @@ ERROR: column "a" does not exist
alter table atacc1 rename "........pg.dropped.1........" to x;
ERROR: column "........pg.dropped.1........" does not exist
alter table atacc1 add primary key(a);
-ERROR: column "a" named in key does not exist
+ERROR: column "a" of relation "atacc1" does not exist
alter table atacc1 add primary key("........pg.dropped.1........");
-ERROR: column "........pg.dropped.1........" named in key does not exist
+ERROR: column "........pg.dropped.1........" of relation "atacc1" does not exist
alter table atacc1 add unique(a);
ERROR: column "a" named in key does not exist
alter table atacc1 add unique("........pg.dropped.1........");
@@ -3751,7 +3758,8 @@ ERROR: cannot alter inherited column "b"
-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
-- partitions exist
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
-ERROR: cannot add constraint to only the partitioned table when partitions exist
+ERROR: constraint must be added to child tables too
+DETAIL: Column "b" of relation "part_2" is not already NOT NULL.
HINT: Do not specify the ONLY keyword.
ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
ERROR: constraint must be added to child tables too
diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out
index f8f3ae8d11b..2286519b0ca 100644
--- a/src/test/regress/expected/identity.out
+++ b/src/test/regress/expected/identity.out
@@ -290,6 +290,18 @@ SELECT seqtypid::regtype FROM pg_sequence WHERE seqrelid = 'itest3_a_seq'::regcl
ALTER TABLE itest3 ALTER COLUMN a TYPE text; -- error
ERROR: identity column type must be smallint, integer, or bigint
+-- kinda silly to change property in the same command, but it should work
+ALTER TABLE itest3
+ ADD COLUMN c int GENERATED BY DEFAULT AS IDENTITY,
+ ALTER COLUMN c SET GENERATED ALWAYS;
+\d itest3
+ Table "public.itest3"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+----------------------------------
+ a | integer | | not null | generated by default as identity
+ b | text | | |
+ c | integer | | not null | generated always as identity
+
-- ALTER COLUMN ... SET
CREATE TABLE itest6 (a int GENERATED ALWAYS AS IDENTITY, b text);
INSERT INTO itest6 DEFAULT VALUES;
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index e9ac715d726..c143df5114f 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1099,6 +1099,21 @@ select indrelid::regclass, indexrelid::regclass, inhparent::regclass, indisvalid
(2 rows)
drop table idxpart;
+-- Related to the above scenario: ADD PRIMARY KEY on the parent mustn't
+-- automatically propagate NOT NULL to child columns.
+create table idxpart (a int) partition by range (a);
+create table idxpart0 (like idxpart);
+alter table idxpart0 add unique (a);
+alter table idxpart attach partition idxpart0 default;
+alter table only idxpart add primary key (a); -- fail, no NOT NULL constraint
+ERROR: constraint must be added to child tables too
+DETAIL: Column "a" of relation "idxpart0" is not already NOT NULL.
+HINT: Do not specify the ONLY keyword.
+alter table idxpart0 alter column a set not null;
+alter table only idxpart add primary key (a); -- now it works
+alter table idxpart0 alter column a drop not null; -- fail, pkey needs it
+ERROR: column "a" is marked NOT NULL in parent table
+drop table idxpart;
-- if a partition has a unique index without a constraint, does not attach
-- automatically; creates a new index instead.
create table idxpart (a int, b int) partition by range (a);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 5bda7febde0..5e3d6ecfcc7 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -749,6 +749,14 @@ alter table atacc1 add column test2 int primary key;
alter table atacc1 add column test2 int default 0 primary key;
drop table atacc1;
+-- this combination used to have order-of-execution problems (bug #15580)
+create table atacc1 (a int);
+insert into atacc1 values(1);
+alter table atacc1
+ add column b float8 not null default random(),
+ add primary key(a);
+drop table atacc1;
+
-- something a little more complicated
create table atacc1 ( test int, test2 int);
-- add a primary key constraint
diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql
index 43c2a59d02e..8dcfdf3dc15 100644
--- a/src/test/regress/sql/identity.sql
+++ b/src/test/regress/sql/identity.sql
@@ -174,6 +174,12 @@ SELECT seqtypid::regtype FROM pg_sequence WHERE seqrelid = 'itest3_a_seq'::regcl
ALTER TABLE itest3 ALTER COLUMN a TYPE text; -- error
+-- kinda silly to change property in the same command, but it should work
+ALTER TABLE itest3
+ ADD COLUMN c int GENERATED BY DEFAULT AS IDENTITY,
+ ALTER COLUMN c SET GENERATED ALWAYS;
+\d itest3
+
-- ALTER COLUMN ... SET
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 7d46e036e7a..cc3d0abfb7b 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -578,6 +578,18 @@ select indrelid::regclass, indexrelid::regclass, inhparent::regclass, indisvalid
order by indexrelid::regclass::text collate "C";
drop table idxpart;
+-- Related to the above scenario: ADD PRIMARY KEY on the parent mustn't
+-- automatically propagate NOT NULL to child columns.
+create table idxpart (a int) partition by range (a);
+create table idxpart0 (like idxpart);
+alter table idxpart0 add unique (a);
+alter table idxpart attach partition idxpart0 default;
+alter table only idxpart add primary key (a); -- fail, no NOT NULL constraint
+alter table idxpart0 alter column a set not null;
+alter table only idxpart add primary key (a); -- now it works
+alter table idxpart0 alter column a drop not null; -- fail, pkey needs it
+drop table idxpart;
+
-- if a partition has a unique index without a constraint, does not attach
-- automatically; creates a new index instead.
create table idxpart (a int, b int) partition by range (a);
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-04 20:39:08 +0000