0002-Some-refactoring-of-code-in-ATExecAttachPartition.patch

text/plain

Filename: 0002-Some-refactoring-of-code-in-ATExecAttachPartition.patch
Type: text/plain
Part: 1
Message: Re: A bug in mapping attributes in ATExecAttachPartition()

Patch

Format: format-patch
Series: patch 0002
Subject: Some refactoring of code in ATExecAttachPartition()
File+
src/backend/commands/tablecmds.c 101 86
From fc6de6ab2800d509ec2af35c96722672e47e99cc Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 15 Jun 2017 19:22:31 +0900
Subject: [PATCH 2/3] Some refactoring of code in ATExecAttachPartition()

Take the code to check using table's constraints if the partition
constraint validation can be skipped and put it into a separate
function PartConstraintImpliedByRelConstraint().
---
 src/backend/commands/tablecmds.c | 187 +++++++++++++++++++++------------------
 1 file changed, 101 insertions(+), 86 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d27c43bdc7..818335ffe9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -473,6 +473,8 @@ static void CreateInheritance(Relation child_rel, Relation parent_rel);
 static void RemoveInheritance(Relation child_rel, Relation parent_rel);
 static ObjectAddress ATExecAttachPartition(List **wqueue, Relation rel,
 					  PartitionCmd *cmd);
+static bool PartConstraintImpliedByRelConstraint(Relation partrel,
+					  List *partConstraint);
 static ObjectAddress ATExecDetachPartition(Relation rel, RangeVar *name);
 
 
@@ -13422,15 +13424,12 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	Relation	attachrel,
 				catalog;
 	List	   *attachrel_children;
-	TupleConstr *attachrel_constr;
-	List	   *partConstraint,
-			   *existConstraint;
+	List	   *partConstraint;
 	SysScanDesc scan;
 	ScanKeyData skey;
 	AttrNumber	attno;
 	int			natts;
 	TupleDesc	tupleDesc;
-	bool		skip_validate = false;
 	ObjectAddress address;
 	const char *trigger_name;
 	bool		found_whole_row;
@@ -13625,88 +13624,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 		elog(ERROR, "unexpected whole-row reference found in partition key");
 
 	/*
-	 * Check if we can do away with having to scan the table being attached to
-	 * validate the partition constraint, by *proving* that the existing
-	 * constraints of the table *imply* the partition predicate.  We include
-	 * the table's check constraints and NOT NULL constraints in the list of
-	 * clauses passed to predicate_implied_by().
-	 *
-	 * There is a case in which we cannot rely on just the result of the
-	 * proof.
+	 * Based on the table's existing constraints, determine if we can skip
+	 * scanning the table to validate the partition constraint.
 	 */
-	attachrel_constr = tupleDesc->constr;
-	existConstraint = NIL;
-	if (attachrel_constr != NULL)
-	{
-		int			num_check = attachrel_constr->num_check;
-		int			i;
-
-		if (attachrel_constr->has_not_null)
-		{
-			int			natts = attachrel->rd_att->natts;
-
-			for (i = 1; i <= natts; i++)
-			{
-				Form_pg_attribute att = attachrel->rd_att->attrs[i - 1];
-
-				if (att->attnotnull && !att->attisdropped)
-				{
-					NullTest   *ntest = makeNode(NullTest);
-
-					ntest->arg = (Expr *) makeVar(1,
-												  i,
-												  att->atttypid,
-												  att->atttypmod,
-												  att->attcollation,
-												  0);
-					ntest->nulltesttype = IS_NOT_NULL;
-
-					/*
-					 * 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.
-					 */
-					ntest->argisrow = false;
-					ntest->location = -1;
-					existConstraint = lappend(existConstraint, ntest);
-				}
-			}
-		}
-
-		for (i = 0; i < num_check; i++)
-		{
-			Node	   *cexpr;
-
-			/*
-			 * If this constraint hasn't been fully validated yet, we must
-			 * ignore it here.
-			 */
-			if (!attachrel_constr->check[i].ccvalid)
-				continue;
-
-			cexpr = stringToNode(attachrel_constr->check[i].ccbin);
-
-			/*
-			 * Run each expression through const-simplification and
-			 * canonicalization.  It is necessary, because we will be
-			 * comparing it to similarly-processed qual clauses, and may fail
-			 * to detect valid matches without this.
-			 */
-			cexpr = eval_const_expressions(NULL, cexpr);
-			cexpr = (Node *) canonicalize_qual((Expr *) cexpr);
-
-			existConstraint = list_concat(existConstraint,
-										  make_ands_implicit((Expr *) cexpr));
-		}
-
-		existConstraint = list_make1(make_ands_explicit(existConstraint));
-
-		/* And away we go ... */
-		if (predicate_implied_by(partConstraint, existConstraint, true))
-			skip_validate = true;
-	}
-
-	if (skip_validate)
+	if (PartConstraintImpliedByRelConstraint(attachrel, partConstraint))
 	{
 		/* No need to scan the table after all. */
 		ereport(INFO,
@@ -13715,9 +13636,18 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	}
 	else
 	{
-		/* Constraints proved insufficient, so we need to scan the table. */
 		ListCell   *lc;
 
+		/*
+		 * Constraints proved insufficient, so we need to scan the table.
+		 * However, if the table is partitioned, validation scans of the
+		 * individual leaf partitions may still be skipped if they have
+		 * constraints that would make scanning them unnecessary.
+		 *
+		 * Note that attachrel's OID is in the attachrel_children list.  Since
+		 * we already determined above that its validation scan cannot be
+		 * skipped, we need not check that again in the loop below.
+		 */
 		foreach(lc, attachrel_children)
 		{
 			AlteredTableInfo *tab;
@@ -13742,6 +13672,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 				continue;
 			}
 
+			/*
+			 * Check if the partition's existing constraints imply the
+			 * partition constraint and if so, skip the validation scan.
+			 */
 			if (part_rel != attachrel)
 			{
 				/*
@@ -13776,6 +13710,87 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 }
 
 /*
+ * PartConstraintImpliedByRelConstraint
+ *		Does partrel's existing constraints imply the partition constraint?
+ *
+ * Existing constraints includes its check constraints and column-level
+ * NOT NULL constraints and partConstraint describes the partition constraint.
+ */
+static bool
+PartConstraintImpliedByRelConstraint(Relation partrel, List *partConstraint)
+{
+	List *existConstraint = NIL;
+	TupleConstr *constr = RelationGetDescr(partrel)->constr;
+	int		num_check,
+			i;
+
+	if (constr && constr->has_not_null)
+	{
+		int		natts = partrel->rd_att->natts;
+
+		for (i = 1; i <= natts; i++)
+		{
+			Form_pg_attribute att = partrel->rd_att->attrs[i - 1];
+
+			if (att->attnotnull && !att->attisdropped)
+			{
+				NullTest   *ntest = makeNode(NullTest);
+
+				ntest->arg = (Expr *) makeVar(1,
+											  i,
+											  att->atttypid,
+											  att->atttypmod,
+											  att->attcollation,
+											  0);
+				ntest->nulltesttype = IS_NOT_NULL;
+
+				/*
+				 * 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.
+				 */
+				ntest->argisrow = false;
+				ntest->location = -1;
+				existConstraint = lappend(existConstraint, ntest);
+			}
+		}
+	}
+
+	num_check = (constr != NULL) ? constr->num_check : 0;
+	for (i = 0; i < num_check; i++)
+	{
+		Node	   *cexpr;
+
+		/*
+		 * If this constraint hasn't been fully validated yet, we must
+		 * ignore it here.
+		 */
+		if (!constr->check[i].ccvalid)
+			continue;
+
+		cexpr = stringToNode(constr->check[i].ccbin);
+
+		/*
+		 * Run each expression through const-simplification and
+		 * canonicalization.  It is necessary, because we will be comparing
+		 * it to similarly-processed partition constraint expressions, and
+		 * may fail to detect valid matches without this.
+		 */
+		cexpr = eval_const_expressions(NULL, cexpr);
+		cexpr = (Node *) canonicalize_qual((Expr *) cexpr);
+
+		existConstraint = list_concat(existConstraint,
+									  make_ands_implicit((Expr *) cexpr));
+	}
+
+	if (existConstraint != NIL)
+		existConstraint = list_make1(make_ands_explicit(existConstraint));
+
+	/* And away we go ... */
+	return predicate_implied_by(partConstraint, existConstraint, true);
+}
+
+/*
  * ALTER TABLE DETACH PARTITION
  *
  * Return the address of the relation that is no longer a partition of rel.
-- 
2.11.0