Catalog not-null constraints

We now create contype='n' pg_constraint rows for not-null constraints.

We propagate these constraints to other tables during operations such as
adding inheritance relationships, creating and attaching partitions and
creating tables LIKE other tables.  We also spawn not-null constraints
for inheritance child tables when their parents have primary keys.
These related constraints mostly follow the well-known rules of
conislocal and coninhcount that we have for CHECK constraints, with some
adaptations: for example, as opposed to CHECK constraints, we don't
match not-null ones by name when descending a hierarchy to alter it,
instead matching by column name that they apply to.  This means we don't
require the constraint names to be identical across a hierarchy.

For now, we omit them for system catalogs.  Maybe this is worth
reconsidering.  We don't support NOT VALID nor DEFERRABLE clauses
either; these can be added as separate features later (this patch is
already large and complicated enough.)

psql shows these constraints in \d+.

pg_dump requires some ad-hoc hacks, particularly when dumping a primary
key.  We now create one "throwaway" not-null constraint for each column
in the PK together with the CREATE TABLE command, and once the PK is
created, all those throwaway constraints are removed.  This avoids
having to check each tuple for nullness when the dump restores the
primary key creation.

pg_upgrading from an older release requires a somewhat brittle procedure
to create a constraint state that matches what would be created if the
database were being created fresh in Postgres 17.  I have tested all the
scenarios I could think of, and it works correctly as far as I can tell,
but I could have neglected weird cases.

This patch has been very long in the making.  The first patch was
written by Bernd Helmle in 2010 to add a new pg_constraint.contype value
('n'), which I (Álvaro) then hijacked in 2011 and 2012, until that one
was killed by the realization that we ought to use contype='c' instead:
manufactured CHECK constraints.  However, later SQL standard
development, as well as nonobvious emergent properties of that design
(mostly, failure to distinguish them from "normal" CHECK constraints as
well as the performance implication of having to test the CHECK
expression) led us to reconsider this choice, so now the current
implementation uses contype='n' again.  During Postgres 16 this had
already been introduced by commit e056c557aef4, but there were some
problems mainly with the pg_upgrade procedure that couldn't be fixed in
reasonable time, so it was reverted.

In 2016 Vitaly Burovoy also worked on this feature[1] but found no
consensus for his proposed approach, which was claimed to be closer to
the letter of the standard, requiring an additional pg_attribute column
to track the OID of the not-null constraint for that column.
[1] https://postgr.es/m/CAKOSWNkN6HSyatuys8xZxzRCR-KL1OkHS5-b9qd9bf1Rad3PLA@mail.gmail.com

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Author: Bernd Helmle <mailings@oopsware.de>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>

1 files changed, 218 insertions, 59 deletions

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index e48e9e99d36..bab7b87fe86 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -81,6 +81,7 @@ typedef struct
 	bool		isalter;		/* true if altering existing table */
 	List	   *columns;		/* ColumnDef items */
 	List	   *ckconstraints;	/* CHECK constraints */
+	List	   *nnconstraints;	/* NOT NULL constraints */
 	List	   *fkconstraints;	/* FOREIGN KEY constraints */
 	List	   *ixconstraints;	/* index-creating constraints */
 	List	   *likeclauses;	/* LIKE clauses that need post-processing */
@@ -240,6 +241,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	cxt.isalter = false;
 	cxt.columns = NIL;
 	cxt.ckconstraints = NIL;
+	cxt.nnconstraints = NIL;
 	cxt.fkconstraints = NIL;
 	cxt.ixconstraints = NIL;
 	cxt.likeclauses = NIL;
@@ -346,6 +348,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
 	 */
 	stmt->tableElts = cxt.columns;
 	stmt->constraints = cxt.ckconstraints;
+	stmt->nnconstraints = cxt.nnconstraints;
 
 	result = lappend(cxt.blist, stmt);
 	result = list_concat(result, cxt.alist);
@@ -535,6 +538,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 	bool		saw_default;
 	bool		saw_identity;
 	bool		saw_generated;
+	bool		need_notnull = false;
 	ListCell   *clist;
 
 	cxt->columns = lappend(cxt->columns, column);
@@ -632,10 +636,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 		constraint->cooked_expr = NULL;
 		column->constraints = lappend(column->constraints, constraint);
 
-		constraint = makeNode(Constraint);
-		constraint->contype = CONSTR_NOTNULL;
-		constraint->location = -1;
-		column->constraints = lappend(column->constraints, constraint);
+		/* have a not-null constraint added later */
+		need_notnull = true;
 	}
 
 	/* Process column constraints, if any... */
@@ -653,7 +655,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 		switch (constraint->contype)
 		{
 			case CONSTR_NULL:
-				if (saw_nullable && column->is_not_null)
+				if ((saw_nullable && column->is_not_null) || need_notnull)
 					ereport(ERROR,
 							(errcode(ERRCODE_SYNTAX_ERROR),
 							 errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"",
@@ -665,6 +667,10 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 				break;
 
 			case CONSTR_NOTNULL:
+
+				/*
+				 * Disallow conflicting [NOT] NULL markings
+				 */
 				if (saw_nullable && !column->is_not_null)
 					ereport(ERROR,
 							(errcode(ERRCODE_SYNTAX_ERROR),
@@ -672,8 +678,25 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 									column->colname, cxt->relation->relname),
 							 parser_errposition(cxt->pstate,
 												constraint->location)));
-				column->is_not_null = true;
-				saw_nullable = true;
+				/* Ignore redundant NOT NULL markings */
+
+				/*
+				 * If this is the first time we see this column being marked
+				 * not null, add the constraint entry; and get rid of any
+				 * previous markings to mark the column NOT NULL.
+				 */
+				if (!column->is_not_null)
+				{
+					column->is_not_null = true;
+					saw_nullable = true;
+
+					constraint->keys = list_make1(makeString(column->colname));
+					cxt->nnconstraints = lappend(cxt->nnconstraints, constraint);
+
+					/* Don't need this anymore, if we had it */
+					need_notnull = false;
+				}
+
 				break;
 
 			case CONSTR_DEFAULT:
@@ -723,16 +746,19 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 					column->identity = constraint->generated_when;
 					saw_identity = true;
 
-					/* An identity column is implicitly NOT NULL */
-					if (saw_nullable && !column->is_not_null)
+					/*
+					 * Identity columns are always NOT NULL, but we may have a
+					 * constraint already.
+					 */
+					if (!saw_nullable)
+						need_notnull = true;
+					else if (!column->is_not_null)
 						ereport(ERROR,
 								(errcode(ERRCODE_SYNTAX_ERROR),
 								 errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"",
 										column->colname, cxt->relation->relname),
 								 parser_errposition(cxt->pstate,
 													constraint->location)));
-					column->is_not_null = true;
-					saw_nullable = true;
 					break;
 				}
 
@@ -839,6 +865,29 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 	}
 
 	/*
+	 * If we need a not-null constraint for SERIAL or IDENTITY, and one was
+	 * not explicitly specified, add one now.
+	 */
+	if (need_notnull && !(saw_nullable && column->is_not_null))
+	{
+		Constraint *notnull;
+
+		column->is_not_null = true;
+
+		notnull = makeNode(Constraint);
+		notnull->contype = CONSTR_NOTNULL;
+		notnull->conname = NULL;
+		notnull->deferrable = false;
+		notnull->initdeferred = false;
+		notnull->location = -1;
+		notnull->keys = list_make1(makeString(column->colname));
+		notnull->skip_validation = false;
+		notnull->initially_valid = true;
+
+		cxt->nnconstraints = lappend(cxt->nnconstraints, notnull);
+	}
+
+	/*
 	 * If needed, generate ALTER FOREIGN TABLE ALTER COLUMN statement to add
 	 * per-column foreign data wrapper options to this column after creation.
 	 */
@@ -907,6 +956,10 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
 			cxt->ckconstraints = lappend(cxt->ckconstraints, constraint);
 			break;
 
+		case CONSTR_NOTNULL:
+			cxt->nnconstraints = lappend(cxt->nnconstraints, constraint);
+			break;
+
 		case CONSTR_FOREIGN:
 			if (cxt->isforeign)
 				ereport(ERROR,
@@ -918,7 +971,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
 			break;
 
 		case CONSTR_NULL:
-		case CONSTR_NOTNULL:
 		case CONSTR_DEFAULT:
 		case CONSTR_ATTR_DEFERRABLE:
 		case CONSTR_ATTR_NOT_DEFERRABLE:
@@ -954,6 +1006,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 	AclResult	aclresult;
 	char	   *comment;
 	ParseCallbackState pcbstate;
+	bool		process_notnull_constraints = false;
 
 	setup_parser_errposition_callback(&pcbstate, cxt->pstate,
 									  table_like_clause->relation->location);
@@ -1025,8 +1078,9 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		/*
 		 * Create a new column, which is marked as NOT inherited.
 		 *
-		 * For constraints, ONLY the NOT NULL constraint is inherited by the
-		 * new column definition per SQL99.
+		 * For constraints, ONLY the not-null constraint is inherited by the
+		 * new column definition per SQL99; however we cannot do that
+		 * correctly here, so we leave it for expandTableLikeClause to handle.
 		 */
 		def = makeNode(ColumnDef);
 		def->colname = pstrdup(attributeName);
@@ -1034,7 +1088,9 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 											attribute->atttypmod);
 		def->inhcount = 0;
 		def->is_local = true;
-		def->is_not_null = attribute->attnotnull;
+		def->is_not_null = false;
+		if (attribute->attnotnull)
+			process_notnull_constraints = true;
 		def->is_from_type = false;
 		def->storage = 0;
 		def->raw_default = NULL;
@@ -1116,20 +1172,78 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 	 * we don't yet know what column numbers the copied columns will have in
 	 * the finished table.  If any of those options are specified, add the
 	 * LIKE clause to cxt->likeclauses so that expandTableLikeClause will be
-	 * called after we do know that.  Also, remember the relation OID so that
+	 * called after we do know that; in addition, do that if there are any NOT
+	 * NULL constraints, because those must be propagated even if not
+	 * explicitly requested.
+	 *
+	 * In order for this to work, we remember the relation OID so that
 	 * expandTableLikeClause is certain to open the same table.
 	 */
-	if (table_like_clause->options &
-		(CREATE_TABLE_LIKE_DEFAULTS |
-		 CREATE_TABLE_LIKE_GENERATED |
-		 CREATE_TABLE_LIKE_CONSTRAINTS |
-		 CREATE_TABLE_LIKE_INDEXES))
+	if ((table_like_clause->options &
+		 (CREATE_TABLE_LIKE_DEFAULTS |
+		  CREATE_TABLE_LIKE_GENERATED |
+		  CREATE_TABLE_LIKE_CONSTRAINTS |
+		  CREATE_TABLE_LIKE_INDEXES)) ||
+		process_notnull_constraints)
 	{
 		table_like_clause->relationOid = RelationGetRelid(relation);
 		cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
 	}
 
 	/*
+	 * If INCLUDING INDEXES is not given and a primary key exists, we need to
+	 * add not-null constraints to the columns covered by the PK (except those
+	 * that already have one.)  This is required for backwards compatibility.
+	 */
+	if ((table_like_clause->options & CREATE_TABLE_LIKE_INDEXES) == 0)
+	{
+		Bitmapset  *pkcols;
+		int			x = -1;
+		Bitmapset  *donecols = NULL;
+		ListCell   *lc;
+
+		/*
+		 * Obtain a bitmapset of columns on which we'll add not-null
+		 * constraints in expandTableLikeClause, so that we skip this for
+		 * those.
+		 */
+		foreach(lc, RelationGetNotNullConstraints(RelationGetRelid(relation), true))
+		{
+			CookedConstraint *cooked = (CookedConstraint *) lfirst(lc);
+
+			donecols = bms_add_member(donecols, cooked->attnum);
+		}
+
+		pkcols = RelationGetIndexAttrBitmap(relation,
+											INDEX_ATTR_BITMAP_PRIMARY_KEY);
+		while ((x = bms_next_member(pkcols, x)) >= 0)
+		{
+			Constraint *notnull;
+			AttrNumber	attnum = x + FirstLowInvalidHeapAttributeNumber;
+			Form_pg_attribute attForm;
+
+			/* ignore if we already have one for this column */
+			if (bms_is_member(attnum, donecols))
+				continue;
+
+			attForm = TupleDescAttr(tupleDesc, attnum - 1);
+
+			notnull = makeNode(Constraint);
+			notnull->contype = CONSTR_NOTNULL;
+			notnull->conname = NULL;
+			notnull->is_no_inherit = false;
+			notnull->deferrable = false;
+			notnull->initdeferred = false;
+			notnull->location = -1;
+			notnull->keys = list_make1(makeString(pstrdup(NameStr(attForm->attname))));
+			notnull->skip_validation = false;
+			notnull->initially_valid = true;
+
+			cxt->nnconstraints = lappend(cxt->nnconstraints, notnull);
+		}
+	}
+
+	/*
 	 * We may copy extended statistics if requested, since the representation
 	 * of CreateStatsStmt doesn't depend on column numbers.
 	 */
@@ -1195,6 +1309,8 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
 	TupleConstr *constr;
 	AttrMap    *attmap;
 	char	   *comment;
+	bool		at_pushed = false;
+	ListCell   *lc;
 
 	/*
 	 * Open the relation referenced by the LIKE clause.  We should still have
@@ -1375,6 +1491,20 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
 	}
 
 	/*
+	 * Copy not-null constraints, too (these do not require any option to have
+	 * been given).
+	 */
+	foreach(lc, RelationGetNotNullConstraints(RelationGetRelid(relation), false))
+	{
+		AlterTableCmd *atsubcmd;
+
+		atsubcmd = makeNode(AlterTableCmd);
+		atsubcmd->subtype = AT_AddConstraint;
+		atsubcmd->def = (Node *) lfirst_node(Constraint, lc);
+		atsubcmds = lappend(atsubcmds, atsubcmd);
+	}
+
+	/*
 	 * If we generated any ALTER TABLE actions above, wrap them into a single
 	 * ALTER TABLE command.  Stick it at the front of the result, so it runs
 	 * before any CommentStmts we made above.
@@ -1388,6 +1518,8 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
 		atcmd->objtype = OBJECT_TABLE;
 		atcmd->missing_ok = false;
 		result = lcons(atcmd, result);
+
+		at_pushed = true;
 	}
 
 	/*
@@ -1415,6 +1547,39 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
 												 attmap,
 												 NULL);
 
+			/*
+			 * The PK columns might not yet non-nullable, so make sure they
+			 * become so.
+			 */
+			if (index_stmt->primary)
+			{
+				foreach(lc, index_stmt->indexParams)
+				{
+					IndexElem  *col = lfirst_node(IndexElem, lc);
+					AlterTableCmd *notnullcmd = makeNode(AlterTableCmd);
+
+					notnullcmd->subtype = AT_SetAttNotNull;
+					notnullcmd->name = pstrdup(col->name);
+					/* Luckily we can still add more AT-subcmds here */
+					atsubcmds = lappend(atsubcmds, notnullcmd);
+				}
+
+				/*
+				 * If we had already put the AlterTableStmt into the output
+				 * list, we don't need to do so again; otherwise do it.
+				 */
+				if (!at_pushed)
+				{
+					AlterTableStmt *atcmd = makeNode(AlterTableStmt);
+
+					atcmd->relation = copyObject(heapRel);
+					atcmd->cmds = atsubcmds;
+					atcmd->objtype = OBJECT_TABLE;
+					atcmd->missing_ok = false;
+					result = lcons(atcmd, result);
+				}
+			}
+
 			/* Copy comment on index, if requested */
 			if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
 			{
@@ -1505,8 +1670,8 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
  * 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
+ * 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 *
@@ -2051,10 +2216,12 @@ transformIndexConstraints(CreateStmtContext *cxt)
 	ListCell   *lc;
 
 	/*
-	 * Run through the constraints that need to generate an index. For PRIMARY
-	 * KEY, mark each column as NOT NULL and create an index. For UNIQUE or
-	 * EXCLUDE, create an index as for PRIMARY KEY, but do not insist on NOT
-	 * NULL.
+	 * Run through the constraints that need to generate an index, and do so.
+	 *
+	 * For PRIMARY KEY, in addition we set each column's attnotnull flag true.
+	 * We do not create a separate not-null constraint, as that would be
+	 * redundant: the PRIMARY KEY constraint itself fulfills that role.  Other
+	 * constraint types don't need any not-null markings.
 	 */
 	foreach(lc, cxt->ixconstraints)
 	{
@@ -2128,9 +2295,7 @@ transformIndexConstraints(CreateStmtContext *cxt)
 	}
 
 	/*
-	 * 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.
+	 * Now append all the IndexStmts to cxt->alist.
 	 */
 	cxt->alist = list_concat(cxt->alist, finalindexlist);
 }
@@ -2138,12 +2303,10 @@ transformIndexConstraints(CreateStmtContext *cxt)
 /*
  * transformIndexConstraint
  *		Transform one UNIQUE, PRIMARY KEY, or EXCLUDE constraint for
- *		transformIndexConstraints.
+ *		transformIndexConstraints. An IndexStmt is returned.
  *
- * 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.
+ * For a PRIMARY KEY constraint, we additionally force the columns to be
+ * marked as not-null, without producing a not-null constraint.
  */
 static IndexStmt *
 transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
@@ -2401,7 +2564,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.
+	 * also make sure they are not-null.
 	 */
 	else
 	{
@@ -2409,7 +2572,6 @@ 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;
@@ -2428,15 +2590,16 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 			{
 				/*
 				 * column is defined in the new table.  For PRIMARY KEY, we
-				 * can apply the NOT NULL constraint cheaply here ... unless
+				 * 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).
+				 * here would be ineffective (see MergeAttributes).  Note that
+				 * this isn't effective in ALTER TABLE either, unless the
+				 * column is being added in the same command.
 				 */
 				if (constraint->contype == CONSTR_PRIMARY &&
 					!column->is_from_type)
 				{
 					column->is_not_null = true;
-					forced_not_null = true;
 				}
 			}
 			else if (SystemAttributeByName(key) != NULL)
@@ -2479,14 +2642,6 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 						if (strcmp(key, inhname) == 0)
 						{
 							found = true;
-
-							/*
-							 * 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;
 						}
 					}
@@ -2540,15 +2695,11 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
 			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)
+			if (constraint->contype == CONSTR_PRIMARY)
 			{
 				AlterTableCmd *notnullcmd = makeNode(AlterTableCmd);
 
-				notnullcmd->subtype = AT_SetNotNull;
+				notnullcmd->subtype = AT_SetAttNotNull;
 				notnullcmd->name = pstrdup(key);
 				notnullcmds = lappend(notnullcmds, notnullcmd);
 			}
@@ -3320,6 +3471,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 	cxt.isalter = true;
 	cxt.columns = NIL;
 	cxt.ckconstraints = NIL;
+	cxt.nnconstraints = NIL;
 	cxt.fkconstraints = NIL;
 	cxt.ixconstraints = NIL;
 	cxt.likeclauses = NIL;
@@ -3563,8 +3715,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 
 		/*
 		 * 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.
+		 * AT_SetAttNotNull statements generated from primary key constraints.
+		 * We absorb the subcommands of the latter directly.
 		 */
 		if (IsA(istmt, IndexStmt))
 		{
@@ -3587,19 +3739,26 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
 	}
 	cxt.alist = NIL;
 
-	/* Append any CHECK or FK constraints to the commands list */
+	/* Append any CHECK, NOT NULL or FK constraints to the commands list */
 	foreach(l, cxt.ckconstraints)
 	{
 		newcmd = makeNode(AlterTableCmd);
 		newcmd->subtype = AT_AddConstraint;
-		newcmd->def = (Node *) lfirst(l);
+		newcmd->def = (Node *) lfirst_node(Constraint, l);
+		newcmds = lappend(newcmds, newcmd);
+	}
+	foreach(l, cxt.nnconstraints)
+	{
+		newcmd = makeNode(AlterTableCmd);
+		newcmd->subtype = AT_AddConstraint;
+		newcmd->def = (Node *) lfirst_node(Constraint, l);
 		newcmds = lappend(newcmds, newcmd);
 	}
 	foreach(l, cxt.fkconstraints)
 	{
 		newcmd = makeNode(AlterTableCmd);
 		newcmd->subtype = AT_AddConstraint;
-		newcmd->def = (Node *) lfirst(l);
+		newcmd->def = (Node *) lfirst_node(Constraint, l);
 		newcmds = lappend(newcmds, newcmd);
 	}