Re: using index or check in ALTER TABLE SET NOT NULL

[Sergei Kornilov]

Here is new patch with check only existed valid constraints and without SPI at 
all.

Thanks
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d979ce2..7ab7580 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -370,6 +370,8 @@ static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMO
 static void ATPrepSetNotNull(Relation rel, bool recurse, bool recursing);
 static ObjectAddress ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 				 const char *colName, LOCKMODE lockmode);
+static bool isSetNotNullNeedsTableScan(Relation rel, Form_pg_attribute attr);
+static bool ConstraintImpliedByRelConstraint(Relation scanrel, List *partConstraint);
 static ObjectAddress ATExecColumnDefault(Relation rel, const char *colName,
 					Node *newDefault, LOCKMODE lockmode);
 static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName,
@@ -5863,8 +5865,10 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 
 		CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
 
-		/* Tell Phase 3 it needs to test the constraint */
-		tab->new_notnull = true;
+		if (isSetNotNullNeedsTableScan(rel, (Form_pg_attribute) GETSTRUCT(tuple))) {
+			/* Tell Phase 3 it needs to test the constraint */
+			tab->new_notnull = true;
+		}
 
 		ObjectAddressSubSet(address, RelationRelationId,
 							RelationGetRelid(rel), attnum);
@@ -5880,6 +5884,42 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
 	return address;
 }
 
+static bool
+isSetNotNullNeedsTableScan(Relation rel, Form_pg_attribute attr)
+{
+	List       *notNullConstraint = NIL;
+	NullTest   *nnulltest = makeNode(NullTest);
+
+	nnulltest->arg = (Expr *) makeVar(1,
+				  attr->attnum,
+				  attr->atttypid,
+				  attr->atttypmod,
+				  attr->attcollation,
+				  0);
+	nnulltest->nulltesttype = IS_NOT_NULL;
+
+	/*
+	 * same thing as in ConstraintImpliedByRelConstraint
+	 * argisrow=false is correct even for a composite column,
+	 * because attnotnull does not represent a SQL-spec IS NOT
+	 * NULL test in such a case, just IS DISTINCT FROM NULL.
+	 */
+	nnulltest->argisrow = false;
+	nnulltest->location = -1;
+	notNullConstraint = lappend(notNullConstraint, nnulltest);
+
+	if (ConstraintImpliedByRelConstraint(rel, notNullConstraint))
+	{
+		ereport(DEBUG1,
+			(errmsg("verifying table \"%s\" NOT NULL constraint "
+				"on %s attribute by existed constraints",
+			RelationGetRelationName(rel), NameStr(attr->attname))));
+		return false;
+	}
+
+	return true;
+}
+
 /*
  * ALTER TABLE ALTER COLUMN SET/DROP DEFAULT
  *
@@ -13620,12 +13660,23 @@ ComputePartitionAttrs(Relation rel, List *partParams, AttrNumber *partattrs,
 /*
  * PartConstraintImpliedByRelConstraint
  *		Does scanrel's existing constraints imply the partition constraint?
+ */
+bool
+PartConstraintImpliedByRelConstraint(Relation scanrel,
+									 List *partConstraint)
+{
+	return ConstraintImpliedByRelConstraint(scanrel, partConstraint);
+}
+
+/*
+ * PartConstraintImpliedByRelConstraint
+ *		Does scanrel's existing constraints imply given constraint
  *
  * Existing constraints includes its check constraints and column-level
  * NOT NULL constraints and partConstraint describes the partition constraint.
  */
-bool
-PartConstraintImpliedByRelConstraint(Relation scanrel,
+static bool
+ConstraintImpliedByRelConstraint(Relation scanrel,
 									 List *partConstraint)
 {
 	List	   *existConstraint = NIL;