Clean up code, comments, and formatting for table partitioning.

Amit Langote, plus pgindent-ing by me.  Inspired in part by review
comments from Tomas Vondra.

src/backend/catalog/heap.c

@@ -1887,6 +1887,10 @@ heap_drop_with_catalog(Oid relid)
 
 	if (parent)
 	{
+		/*
+		 * Invalidate the parent's relcache so that the partition is no longer
+		 * included in its partition descriptor.
+		 */
 		CacheInvalidateRelcache(parent);
 		heap_close(parent, NoLock);		/* keep the lock */
 	}

src/backend/catalog/partition.c

@@ -1492,7 +1492,7 @@ generate_partition_qual(Relation rel, bool recurse)
  *			Construct values[] and isnull[] arrays for the partition key
  *			of a tuple.
  *
- *	pkinfo			partition key execution info
+ *	pd				Partition dispatch object of the partitioned table
  *	slot			Heap tuple from which to extract partition key
  *	estate			executor state for evaluating any partition key
  *					expressions (must be non-NULL)
@@ -1565,7 +1565,7 @@ FormPartitionKeyDatum(PartitionDispatch pd,
  * the latter case.
  */
 int
-get_partition_for_tuple(PartitionDispatch * pd,
+get_partition_for_tuple(PartitionDispatch *pd,
 						TupleTableSlot *slot,
 						EState *estate,
 						Oid *failed_at)

src/backend/commands/copy.c

@@ -1403,31 +1403,28 @@ BeginCopy(ParseState *pstate,
 					 errmsg("table \"%s\" does not have OIDs",
 							RelationGetRelationName(cstate->rel))));
 
-		/*
-		 * Initialize state for CopyFrom tuple routing.  Watch out for
-		 * any foreign partitions.
-		 */
+		/* Initialize state for CopyFrom tuple routing. */
 		if (is_from && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 		{
-			PartitionDispatch *pd;
 			List	   *leaf_parts;
 			ListCell   *cell;
 			int			i,
-							num_parted,
-							num_leaf_parts;
+						num_parted;
 			ResultRelInfo *leaf_part_rri;
 
 			/* Get the tuple-routing information and lock partitions */
-			pd = RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
-												  &num_parted, &leaf_parts);
-			num_leaf_parts = list_length(leaf_parts);
-			cstate->partition_dispatch_info = pd;
+			cstate->partition_dispatch_info =
+				RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
+												 &num_parted,
+												 &leaf_parts);
 			cstate->num_dispatch = num_parted;
-			cstate->num_partitions = num_leaf_parts;
-			cstate->partitions = (ResultRelInfo *) palloc(num_leaf_parts *
+			cstate->num_partitions = list_length(leaf_parts);
+			cstate->partitions = (ResultRelInfo *)
+				palloc(cstate->num_partitions *
 					   sizeof(ResultRelInfo));
 			cstate->partition_tupconv_maps = (TupleConversionMap **)
-						palloc0(num_leaf_parts * sizeof(TupleConversionMap *));
+				palloc0(cstate->num_partitions *
+						sizeof(TupleConversionMap *));
 
 			leaf_part_rri = cstate->partitions;
 			i = 0;
@@ -1438,8 +1435,8 @@ BeginCopy(ParseState *pstate,
 				/*
 				 * We locked all the partitions above including the leaf
 				 * partitions.  Note that each of the relations in
-				 * cstate->partitions will be closed by CopyFrom() after
-				 * it's finished with its processing.
+				 * cstate->partitions will be closed by CopyFrom() after it's
+				 * finished with its processing.
 				 */
 				partrel = heap_open(lfirst_oid(cell), NoLock);
 
@@ -1452,7 +1449,8 @@ BeginCopy(ParseState *pstate,
 				InitResultRelInfo(leaf_part_rri,
 								  partrel,
 								  1,	/* dummy */
-								  false, /* no partition constraint check */
+								  false,		/* no partition constraint
+												 * check */
 								  0);
 
 				/* Open partition indices */
@@ -2486,8 +2484,8 @@ CopyFrom(CopyState cstate)
 	 * BEFORE/INSTEAD OF triggers, or we need to evaluate volatile default
 	 * expressions. Such triggers or expressions might query the table we're
 	 * inserting to, and act differently if the tuples that have already been
-	 * processed and prepared for insertion are not there.  We also can't
-	 * do it if the table is partitioned.
+	 * processed and prepared for insertion are not there.  We also can't do
+	 * it if the table is partitioned.
 	 */
 	if ((resultRelInfo->ri_TrigDesc != NULL &&
 		 (resultRelInfo->ri_TrigDesc->trig_insert_before_row ||

src/backend/commands/tablecmds.c

@@ -791,8 +791,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	}
 
 	/*
-	 * Process the partitioning specification (if any) and store the
-	 * partition key information into the catalog.
+	 * Process the partitioning specification (if any) and store the partition
+	 * key information into the catalog.
 	 */
 	if (stmt->partspec)
 	{
@@ -827,7 +827,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 			for (i = 0; i < partnatts; i++)
 			{
 				AttrNumber	partattno = partattrs[i];
-				Form_pg_attribute attform = descriptor->attrs[partattno-1];
+				Form_pg_attribute attform = descriptor->attrs[partattno - 1];
 
 				if (partattno != 0 && !attform->attnotnull)
 				{
@@ -1613,8 +1613,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 						MaxHeapAttributeNumber)));
 
 	/*
-	 * In case of a partition, there are no new column definitions, only
-	 * dummy ColumnDefs created for column constraints.  We merge these
+	 * In case of a partition, there are no new column definitions, only dummy
+	 * ColumnDefs created for column constraints.  We merge them with the
 	 * constraints inherited from the parent.
 	 */
 	if (is_partition)
@@ -1704,9 +1704,9 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 		 * the parent's relhassubclass field, if its previous children were
 		 * recently dropped.
 		 *
-		 * If the child table is a partition, then we instead grab an exclusive
-		 * lock on the parent because its partition descriptor will be changed
-		 * by addition of the new partition.
+		 * If the child table is a partition, then we instead grab an
+		 * exclusive lock on the parent because its partition descriptor will
+		 * be changed by addition of the new partition.
 		 */
 		if (!is_partition)
 			relation = heap_openrv(parent, ShareUpdateExclusiveLock);
@@ -1714,8 +1714,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 			relation = heap_openrv(parent, AccessExclusiveLock);
 
 		/*
-		 * We do not allow partitioned tables and partitions to participate
-		 * in regular inheritance.
+		 * We do not allow partitioned tables and partitions to participate in
+		 * regular inheritance.
 		 */
 		if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
 			!is_partition)
@@ -2030,8 +2030,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 							newcollid;
 
 				/*
-				 * Partitions have only one parent, so conflict should never
-				 * occur
+				 * Partitions have only one parent and have no column
+				 * definitions of their own, so conflict should never occur.
 				 */
 				Assert(!is_partition);
 
@@ -2118,8 +2118,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 
 	/*
 	 * Now that we have the column definition list for a partition, we can
-	 * check whether the columns referenced in column option specifications
-	 * actually exist.  Also, we merge the options into the corresponding
+	 * check whether the columns referenced in the column constraint specs
+	 * actually exist.  Also, we merge the constraints into the corresponding
 	 * column definitions.
 	 */
 	if (is_partition && list_length(saved_schema) > 0)
@@ -2145,8 +2145,8 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
 			while (rest != NULL)
 			{
 				ColumnDef  *restdef = lfirst(rest);
-				ListCell   *next = lnext(rest);		/* need to save it in case
-													 * we delete it */
+				ListCell   *next = lnext(rest); /* need to save it in case we
+												 * delete it */
 
 				if (strcmp(coldef->colname, restdef->colname) == 0)
 				{
@@ -5602,8 +5602,8 @@ ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode)
 	}
 
 	/*
-	 * If the table is a range partitioned table, check that the column
-	 * is not in the partition key.
+	 * If the table is a range partitioned table, check that the column is not
+	 * in the partition key.
 	 */
 	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 	{
@@ -8258,9 +8258,9 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	}
 
 	/*
-	 * In case of a partitioned table, the constraint must be dropped from
-	 * the partitions too.  There is no such thing as NO INHERIT constraints
-	 * in case of partitioned tables.
+	 * In case of a partitioned table, the constraint must be dropped from the
+	 * partitions too.  There is no such thing as NO INHERIT constraints in
+	 * case of partitioned tables.
 	 */
 	if (!recurse && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 		ereport(ERROR,
@@ -10704,7 +10704,8 @@ CreateInheritance(Relation child_rel, Relation parent_rel)
 	/*
 	 * Check for duplicates in the list of parents, and determine the highest
 	 * inhseqno already present; we'll use the next one for the new parent.
-	 * Also, if proposed child is a partition, it cannot already be inheriting.
+	 * Also, if proposed child is a partition, it cannot already be
+	 * inheriting.
 	 *
 	 * Note: we do not reject the case where the child already inherits from
 	 * the parent indirectly; CREATE TABLE doesn't reject comparable cases.
@@ -11283,6 +11284,7 @@ RemoveInheritance(Relation child_rel, Relation parent_rel)
 	drop_parent_dependency(RelationGetRelid(child_rel),
 						   RelationRelationId,
 						   RelationGetRelid(parent_rel));
+
 	/*
 	 * Post alter hook of this inherits. Since object_access_hook doesn't take
 	 * multiple object identifiers, we relay oid of parent relation using
@@ -12896,8 +12898,8 @@ ComputePartitionAttrs(Relation rel, List *partParams, AttrNumber *partattrs,
 							 errmsg("functions in partition key expression must be marked IMMUTABLE")));
 
 				/*
-				 * While it is not exactly *wrong* for an expression to be
-				 * a constant value, it seems better to prevent such input.
+				 * While it is not exactly *wrong* for an expression to be a
+				 * constant value, it seems better to prevent such input.
 				 */
 				if (IsA(expr, Const))
 					ereport(ERROR,
@@ -12905,9 +12907,9 @@ ComputePartitionAttrs(Relation rel, List *partParams, AttrNumber *partattrs,
 							 errmsg("cannot use constant expression as partition key")));
 
 				/*
-				 * transformPartitionSpec() should have already rejected subqueries,
-				 * aggregates, window functions, and SRFs, based on the EXPR_KIND_
-				 * for partition expressions.
+				 * transformPartitionSpec() should have already rejected
+				 * subqueries, aggregates, window functions, and SRFs, based
+				 * on the EXPR_KIND_ for partition expressions.
 				 */
 
 				/* Cannot have expressions containing whole-row references */
@@ -12929,8 +12931,8 @@ ComputePartitionAttrs(Relation rel, List *partParams, AttrNumber *partattrs,
 		/*
 		 * Check we have a collation iff it's a collatable type.  The only
 		 * expected failures here are (1) COLLATE applied to a noncollatable
-		 * type, or (2) partition expression had an unresolved collation.
-		 * But we might as well code this to be a complete consistency check.
+		 * type, or (2) partition expression had an unresolved collation. But
+		 * we might as well code this to be a complete consistency check.
 		 */
 		if (type_is_collatable(atttype))
 		{
@@ -13053,8 +13055,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	heap_close(catalog, AccessShareLock);
 
 	/*
-	 * Prevent circularity by seeing if rel is a partition of attachRel.
-	 * (In particular, this disallows making a rel a partition of itself.)
+	 * Prevent circularity by seeing if rel is a partition of attachRel. (In
+	 * particular, this disallows making a rel a partition of itself.)
 	 */
 	childrels = find_all_inheritors(RelationGetRelid(attachRel),
 									AccessShareLock, NULL);
@@ -13132,8 +13134,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 
 	/*
 	 * Check that the new partition's bound is valid and does not overlap any
-	 * of existing partitions of the parent - note that it does not return
-	 * on error.
+	 * of existing partitions of the parent - note that it does not return on
+	 * error.
 	 */
 	check_new_partition_bound(RelationGetRelationName(attachRel), rel,
 							  cmd->bound);
@@ -13155,8 +13157,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	partConstraint = list_make1(make_ands_explicit(partConstraint));
 
 	/*
-	 * Check if we can do away with having to scan the table being attached
-	 * to validate the partition constraint, by *proving* that the existing
+	 * 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().
@@ -13243,14 +13245,14 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 			skip_validate = true;
 
 		/*
-		 * We choose to err on the safer side, ie, give up on skipping the
-		 * the validation scan, if the partition key column doesn't have
-		 * the NOT NULL constraint and the table is to become a list partition
-		 * that does not accept nulls.  In this case, the partition predicate
+		 * We choose to err on the safer side, ie, give up on skipping the the
+		 * validation scan, if the partition key column doesn't have the NOT
+		 * NULL constraint and the table is to become a list partition that
+		 * does not accept nulls.  In this case, the partition predicate
 		 * (partConstraint) does include an 'key IS NOT NULL' expression,
-		 * however, because of the way predicate_implied_by_simple_clause()
-		 * is designed to handle IS NOT NULL predicates in the absence of a
-		 * IS NOT NULL clause, we cannot rely on just the above proof.
+		 * however, because of the way predicate_implied_by_simple_clause() is
+		 * designed to handle IS NOT NULL predicates in the absence of a IS
+		 * NOT NULL clause, we cannot rely on just the above proof.
 		 *
 		 * That is not an issue in case of a range partition, because if there
 		 * were no NOT NULL constraint defined on the key columns, an error
@@ -13299,8 +13301,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 
 	/*
 	 * Set up to have the table to be scanned to validate the partition
-	 * constraint (see partConstraint above).  If it's a partitioned table,
-	 * we instead schdule its leaf partitions to be scanned instead.
+	 * constraint (see partConstraint above).  If it's a partitioned table, we
+	 * instead schdule its leaf partitions to be scanned instead.
 	 */
 	if (!skip_validate)
 	{
@@ -13351,8 +13353,8 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	}
 
 	/*
-	 * Invalidate the relcache so that the new partition is now included
-	 * in rel's partition descriptor.
+	 * Invalidate the parent's relcache so that the new partition is now
+	 * included its partition descriptor.
 	 */
 	CacheInvalidateRelcache(rel);
 
@@ -13414,8 +13416,8 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	heap_close(classRel, RowExclusiveLock);
 
 	/*
-	 * Invalidate the relcache so that the partition is no longer included
-	 * in our partition descriptor.
+	 * Invalidate the parent's relcache so that the partition is no longer
+	 * included in its partition descriptor.
 	 */
 	CacheInvalidateRelcache(rel);
 

src/backend/executor/nodeModifyTable.c

@@ -1718,26 +1718,26 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 	if (operation == CMD_INSERT &&
 		rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 	{
-		PartitionDispatch  *pd;
 		int					i,
 							j,
-							num_parted,
-							num_leaf_parts;
+							num_parted;
 		List			   *leaf_parts;
 		ListCell		   *cell;
 		ResultRelInfo	   *leaf_part_rri;
 
-		/* Form the partition node tree and lock partitions */
-		pd = RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
-											  &num_parted, &leaf_parts);
-		mtstate->mt_partition_dispatch_info = pd;
+		/* Get the tuple-routing information and lock partitions */
+		mtstate->mt_partition_dispatch_info =
+					RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
+													 &num_parted,
+													 &leaf_parts);
 		mtstate->mt_num_dispatch = num_parted;
-		num_leaf_parts = list_length(leaf_parts);
-		mtstate->mt_num_partitions = num_leaf_parts;
+		mtstate->mt_num_partitions = list_length(leaf_parts);
 		mtstate->mt_partitions = (ResultRelInfo *)
-						palloc0(num_leaf_parts * sizeof(ResultRelInfo));
+						palloc0(mtstate->mt_num_partitions *
+												sizeof(ResultRelInfo));
 		mtstate->mt_partition_tupconv_maps = (TupleConversionMap **)
-					palloc0(num_leaf_parts * sizeof(TupleConversionMap *));
+						palloc0(mtstate->mt_num_partitions *
+												sizeof(TupleConversionMap *));
 
 		leaf_part_rri = mtstate->mt_partitions;
 		i = j = 0;

src/include/catalog/pg_partitioned_table.h

@@ -7,7 +7,7 @@
  *
  * Portions Copyright (c) 1996-2016, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/catalog/pg_partitioned_table.h $
+ * src/include/catalog/pg_partitioned_table.h
  *
  * NOTES
  *	  the genbki.sh script reads this file and generates .bki
@@ -39,17 +39,15 @@ CATALOG(pg_partitioned_table,3350) BKI_WITHOUT_OIDS
 	 * field of a heap tuple can be reliably accessed using its C struct
 	 * offset, as previous fields are all non-nullable fixed-length fields.
 	 */
-	int2vector		partattrs;		/* each member of the array is the
-									 * attribute number of a partition key
-									 * column, or 0 if the column is actually
-									 * an expression */
+	int2vector	partattrs;		/* each member of the array is the attribute
+								 * number of a partition key column, or 0 if
+								 * the column is actually an expression */
 
 #ifdef CATALOG_VARLEN
 	oidvector	partclass;		/* operator class to compare keys */
 	oidvector	partcollation;	/* user-specified collation for keys */
-	pg_node_tree	partexprs;		/* list of expressions in the partitioning
-									 * key; one item for each zero entry in
-									 * partattrs[] */
+	pg_node_tree partexprs;		/* list of expressions in the partition key;
+								 * one item for each zero entry in partattrs[] */
 #endif
 } FormData_pg_partitioned_table;