pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed

From: Sergei Kornilov <sk@zsrv.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Stephen Frost <sfrost@snowman.net>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
Date: Wed, 29 Nov 2017 19:53:01 +0300
Message-ID: <491651511974381@web27j.yandex.ru> (raw)
In-Reply-To: <370481511970756@web3j.yandex.ru>
References: <81911511895540@web58j.yandex.ru>
	<CA+TgmoZKR4dTR7KyoM4jfJN2J-eNoBRG2_XHdZa1f0VUbK1xXQ@mail.gmail.com>
	<20171129152135.GN4628@tamriel.snowman.net>
	<9878.1511970441@sss.pgh.pa.us>
	<370481511970756@web3j.yandex.ru>

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

Thanks

Attachments:

  [text/x-diff] alter_table_set_not_null_by_constraints_only_v2.patch (3.3K, ../491651511974381@web27j.yandex.ru/2-alter_table_set_not_null_by_constraints_only_v2.patch)
  download | inline diff:
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;

view thread (73+ messages)  latest in thread

Message-ID: <491651511974381@web27j.yandex.ru>
Permalink:  ../491651511974381@web27j.yandex.ru/
Also on:    postgresql.org/message-id/491651511974381@web27j.yandex.ru

copy link · copy postgr.es · copy git am

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: sk@zsrv.org, tgl@sss.pgh.pa.us, sfrost@snowman.net, robertmhaas@gmail.com
  Subject: Re: using index or check in ALTER TABLE SET NOT NULL
  In-Reply-To: <491651511974381@web27j.yandex.ru>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox