[CF 5554] Support NOT VALID / VALIDATE constraint options for named NOT NULL constraints

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/5554

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/CAGPqQf2jXkkcHzicPJD6_PjMCp1cVG-ZNtftp9cvWn6XUve9wA@mail.gmail.com
Author(s): Rushabh Lathia

Author: Commitfest Bot

contrib/postgres_fdw/postgres_fdw.c

@@ -5549,7 +5549,15 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 							   "SELECT relname, "
 							   "  attname, "
 							   "  format_type(atttypid, atttypmod), "
-							   "  attnotnull, "
+							   "  attnotnull, ");
+
+		/* NOT VALID NOT NULL columns are supported since Postgres 18 */
+		if (PQserverVersion(conn) >= 180000)
+			appendStringInfoString(&buf, "attnotnullvalid, ");
+		else
+			appendStringInfoString(&buf, "attnotnull AS attnotnullvalid, ");
+
+		appendStringInfoString(&buf,
 							   "  pg_get_expr(adbin, adrelid), ");
 
 		/* Generated columns are supported since Postgres 12 */
@@ -5651,6 +5659,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 				char	   *attname;
 				char	   *typename;
 				char	   *attnotnull;
+				char	   *attnotnullvalid;
 				char	   *attgenerated;
 				char	   *attdefault;
 				char	   *collname;
@@ -5663,14 +5672,15 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 				attname = PQgetvalue(res, i, 1);
 				typename = PQgetvalue(res, i, 2);
 				attnotnull = PQgetvalue(res, i, 3);
-				attdefault = PQgetisnull(res, i, 4) ? NULL :
-					PQgetvalue(res, i, 4);
-				attgenerated = PQgetisnull(res, i, 5) ? NULL :
+				attnotnullvalid = PQgetvalue(res, i, 4);
+				attdefault = PQgetisnull(res, i, 5) ? NULL :
 					PQgetvalue(res, i, 5);
-				collname = PQgetisnull(res, i, 6) ? NULL :
+				attgenerated = PQgetisnull(res, i, 6) ? NULL :
 					PQgetvalue(res, i, 6);
-				collnamespace = PQgetisnull(res, i, 7) ? NULL :
+				collname = PQgetisnull(res, i, 7) ? NULL :
 					PQgetvalue(res, i, 7);
+				collnamespace = PQgetisnull(res, i, 8) ? NULL :
+					PQgetvalue(res, i, 8);
 
 				if (first_item)
 					first_item = false;
@@ -5714,7 +5724,11 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid)
 
 				/* Add NOT NULL if needed */
 				if (import_not_null && attnotnull[0] == 't')
+				{
 					appendStringInfoString(&buf, " NOT NULL");
+					if (attnotnullvalid[0] == 'f')
+						appendStringInfoString(&buf, " NOT VALID");
+				}
 			}
 			while (++i < numrows &&
 				   strcmp(PQgetvalue(res, i, 0), tablename) == 0);

doc/src/sgml/catalogs.sgml

@@ -1260,7 +1260,16 @@
        <structfield>attnotnull</structfield> <type>bool</type>
       </para>
       <para>
-       This column has a not-null constraint.
+       This column has a (possibly unvalidated) not-null constraint.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>attnotnullvalid</structfield> <type>bool</type>
+      </para>
+      <para>
+       Whether the not-null constraint, if one exists, has been validated.
       </para></entry>
      </row>
 

doc/src/sgml/ref/alter_table.sgml

@@ -243,6 +243,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
       entire table; however, if a valid <literal>CHECK</literal> constraint is
       found which proves no <literal>NULL</literal> can exist, then the
       table scan is skipped.
+      If a column has an invalid not-null constraint,
+      <literal>SET NOT NULL</literal> validates it.
      </para>
 
      <para>
@@ -458,8 +460,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
      <para>
       This form adds a new constraint to a table using the same constraint
       syntax as <link linkend="sql-createtable"><command>CREATE TABLE</command></link>, plus the option <literal>NOT
-      VALID</literal>, which is currently only allowed for foreign key
-      and CHECK constraints.
+      VALID</literal>, which is currently only allowed for foreign key,
+      <literal>CHECK</literal> constraints and not-null constraints.
      </para>
 
      <para>
@@ -586,7 +588,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     <term><literal>VALIDATE CONSTRAINT</literal></term>
     <listitem>
      <para>
-      This form validates a foreign key or check constraint that was
+      This form validates a foreign key, check, or not-null constraint that was
       previously created as <literal>NOT VALID</literal>, by scanning the
       table to ensure there are no rows for which the constraint is not
       satisfied.  If the constraint is not enforced, an error is thrown.

src/backend/access/common/tupdesc.c

@@ -74,7 +74,8 @@ populate_compact_attribute_internal(Form_pg_attribute src,
 	dst->atthasmissing = src->atthasmissing;
 	dst->attisdropped = src->attisdropped;
 	dst->attgenerated = (src->attgenerated != '\0');
-	dst->attnotnull = src->attnotnull;
+	dst->attnullability = !src->attnotnull ? ATTNULLABLE_NONE :
+		src->attnotnullvalid ? ATTNULLABLE_VALID : ATTNULLABLE_INVALID;
 
 	switch (src->attalign)
 	{
@@ -252,6 +253,7 @@ CreateTupleDescCopy(TupleDesc tupdesc)
 		Form_pg_attribute att = TupleDescAttr(desc, i);
 
 		att->attnotnull = false;
+		att->attnotnullvalid = false;
 		att->atthasdef = false;
 		att->atthasmissing = false;
 		att->attidentity = '\0';
@@ -298,6 +300,7 @@ CreateTupleDescTruncatedCopy(TupleDesc tupdesc, int natts)
 		Form_pg_attribute att = TupleDescAttr(desc, i);
 
 		att->attnotnull = false;
+		att->attnotnullvalid = false;
 		att->atthasdef = false;
 		att->atthasmissing = false;
 		att->attidentity = '\0';
@@ -418,6 +421,7 @@ TupleDescCopy(TupleDesc dst, TupleDesc src)
 		Form_pg_attribute att = TupleDescAttr(dst, i);
 
 		att->attnotnull = false;
+		att->attnotnullvalid = false;
 		att->atthasdef = false;
 		att->atthasmissing = false;
 		att->attidentity = '\0';
@@ -464,6 +468,7 @@ TupleDescCopyEntry(TupleDesc dst, AttrNumber dstAttno,
 
 	/* since we're not copying constraints or defaults, clear these */
 	dstAtt->attnotnull = false;
+	dstAtt->attnotnullvalid = false;
 	dstAtt->atthasdef = false;
 	dstAtt->atthasmissing = false;
 	dstAtt->attidentity = '\0';
@@ -613,6 +618,8 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
 			return false;
 		if (attr1->attnotnull != attr2->attnotnull)
 			return false;
+		if (attr1->attnotnullvalid != attr2->attnotnullvalid)
+			return false;
 		if (attr1->atthasdef != attr2->atthasdef)
 			return false;
 		if (attr1->attidentity != attr2->attidentity)
@@ -841,6 +848,7 @@ TupleDescInitEntry(TupleDesc desc,
 	att->attndims = attdim;
 
 	att->attnotnull = false;
+	att->attnotnullvalid = false;
 	att->atthasdef = false;
 	att->atthasmissing = false;
 	att->attidentity = '\0';
@@ -904,6 +912,7 @@ TupleDescInitBuiltinEntry(TupleDesc desc,
 	att->attndims = attdim;
 
 	att->attnotnull = false;
+	att->attnotnullvalid = false;
 	att->atthasdef = false;
 	att->atthasmissing = false;
 	att->attidentity = '\0';

src/backend/bootstrap/bootstrap.c

@@ -615,6 +615,9 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
 				attrtypes[attnum]->attnotnull = true;
 		}
 	}
+
+	/* Not-null constraints on system catalogs are always valid. */
+	attrtypes[attnum]->attnotnullvalid = attrtypes[attnum]->attnotnull;
 }
 
 

src/backend/catalog/genbki.pl

@@ -986,6 +986,9 @@ sub morph_row_for_pgattr
 		$row->{attnotnull} = 'f';
 	}
 
+	# Not-null constraints on system catalogs are always valid.
+	$row->{attnotnullvalid} = $row->{attnotnull};
+
 	Catalog::AddDefaultValues($row, $pgattr_schema, 'pg_attribute');
 	return;
 }

src/backend/catalog/heap.c

@@ -151,6 +151,7 @@ static const FormData_pg_attribute a1 = {
 	.attalign = TYPALIGN_SHORT,
 	.attstorage = TYPSTORAGE_PLAIN,
 	.attnotnull = true,
+	.attnotnullvalid = true,
 	.attislocal = true,
 };
 
@@ -164,6 +165,7 @@ static const FormData_pg_attribute a2 = {
 	.attalign = TYPALIGN_INT,
 	.attstorage = TYPSTORAGE_PLAIN,
 	.attnotnull = true,
+	.attnotnullvalid = true,
 	.attislocal = true,
 };
 
@@ -177,6 +179,7 @@ static const FormData_pg_attribute a3 = {
 	.attalign = TYPALIGN_INT,
 	.attstorage = TYPSTORAGE_PLAIN,
 	.attnotnull = true,
+	.attnotnullvalid = true,
 	.attislocal = true,
 };
 
@@ -190,6 +193,7 @@ static const FormData_pg_attribute a4 = {
 	.attalign = TYPALIGN_INT,
 	.attstorage = TYPSTORAGE_PLAIN,
 	.attnotnull = true,
+	.attnotnullvalid = true,
 	.attislocal = true,
 };
 
@@ -203,6 +207,7 @@ static const FormData_pg_attribute a5 = {
 	.attalign = TYPALIGN_INT,
 	.attstorage = TYPSTORAGE_PLAIN,
 	.attnotnull = true,
+	.attnotnullvalid = true,
 	.attislocal = true,
 };
 
@@ -222,6 +227,7 @@ static const FormData_pg_attribute a6 = {
 	.attalign = TYPALIGN_INT,
 	.attstorage = TYPSTORAGE_PLAIN,
 	.attnotnull = true,
+	.attnotnullvalid = true,
 	.attislocal = true,
 };
 
@@ -753,6 +759,7 @@ InsertPgAttributeTuples(Relation pg_attribute_rel,
 		slot[slotCount]->tts_values[Anum_pg_attribute_attstorage - 1] = CharGetDatum(attrs->attstorage);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attcompression - 1] = CharGetDatum(attrs->attcompression);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attnotnull - 1] = BoolGetDatum(attrs->attnotnull);
+		slot[slotCount]->tts_values[Anum_pg_attribute_attnotnullvalid - 1] = BoolGetDatum(attrs->attnotnullvalid);
 		slot[slotCount]->tts_values[Anum_pg_attribute_atthasdef - 1] = BoolGetDatum(attrs->atthasdef);
 		slot[slotCount]->tts_values[Anum_pg_attribute_atthasmissing - 1] = BoolGetDatum(attrs->atthasmissing);
 		slot[slotCount]->tts_values[Anum_pg_attribute_attidentity - 1] = CharGetDatum(attrs->attidentity);
@@ -1714,6 +1721,7 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
 
 	/* Remove any not-null constraint the column may have */
 	attStruct->attnotnull = false;
+	attStruct->attnotnullvalid = false;
 
 	/* Unset this so no one tries to look up the generation expression */
 	attStruct->attgenerated = '\0';
@@ -2616,12 +2624,17 @@ AddRelationNewConstraints(Relation rel,
 						errmsg("cannot add not-null constraint on system column \"%s\"",
 							   strVal(linitial(cdef->keys))));
 
+			Assert(cdef->initially_valid != cdef->skip_validation);
+
 			/*
 			 * If the column already has a not-null constraint, we don't want
-			 * to add another one; just adjust inheritance status as needed.
+			 * to add another one; adjust inheritance status as needed.  This
+			 * also checks whether the existing constraint matches the
+			 * requested validity.
 			 */
-			if (AdjustNotNullInheritance(RelationGetRelid(rel), colnum,
-										 is_local, cdef->is_no_inherit))
+			if (AdjustNotNullInheritance(rel, colnum, is_local,
+										 cdef->is_no_inherit,
+										 cdef->skip_validation))
 				continue;
 
 			/*
@@ -2826,11 +2839,24 @@ MergeWithExistingConstraint(Relation rel, const char *ccname, Node *expr,
 		{
 			if (is_local)
 				con->conislocal = true;
-			else if (pg_add_s16_overflow(con->coninhcount, 1,
+			else
+			{
+				if(pg_add_s16_overflow(con->coninhcount, 1,
 										 &con->coninhcount))
-				ereport(ERROR,
-						errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-						errmsg("too many inheritance parents"));
+					ereport(ERROR,
+							errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+							errmsg("too many inheritance parents"));
+
+				/*
+				 * If the child already has a valid constraint and we are
+				 * creating an invalid one with same definition on it.  The
+				 * child's constraint will remain valid, but can no longer be
+				 * marked as local.
+				*/
+				if (!is_initially_valid && con->convalidated &&
+					is_enforced && con->conenforced)
+					con->conislocal = false;
+			}
 		}
 
 		if (is_no_inherit)