Refactor attribute mappings used in logical tuple conversion

Tuple conversion support in tupconvert.c is able to convert rowtypes
between two relations, inner and outer, which are logically equivalent
but have a different ordering or even dropped columns (used mainly for
inheritance tree and partitions).  This makes use of attribute mappings,
which are simple arrays made of AttrNumber elements with a length
matching the number of attributes of the outer relation.  The length of
the attribute mapping has been treated as completely independent of the
mapping itself until now, making it easy to pass down an incorrect
mapping length.

This commit refactors the code related to attribute mappings and moves
it into an independent facility called attmap.c, extracted from
tupconvert.c.  This merges the attribute mapping with its length,
avoiding to try to guess what is the length of a mapping to use as this
is computed once, when the map is built.

This will avoid mistakes like what has been fixed in dc816e58, which has
used an incorrect mapping length by matching it with the number of
attributes of an inner relation (a child partition) instead of an outer
relation (a partitioned table).

Author: Michael Paquier
Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/20191121042556.GD153437@paquier.xyz

src/backend/access/common/Makefile

@@ -13,6 +13,7 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
+	attmap.o \
 	bufmask.o \
 	detoast.o \
 	heaptuple.o \

src/backend/access/common/attmap.c

+/*-------------------------------------------------------------------------
+ *
+ * attmap.c
+ *	  Attribute mapping support.
+ *
+ * This file provides utility routines to build and manage attribute
+ * mappings by comparing input and output TupleDescs.  Such mappings
+ * are typically used by DDL operating on inheritance and partition trees
+ * to do a conversion between rowtypes logically equivalent but with
+ * columns in a different order, taking into account dropped columns.
+ * They are also used by the tuple conversion routines in tupconvert.c.
+ *
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/access/common/attmap.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "access/attmap.h"
+#include "access/htup_details.h"
+#include "utils/builtins.h"
+
+
+static bool check_attrmap_match(TupleDesc indesc,
+								TupleDesc outdesc,
+								AttrMap *attrMap);
+
+/*
+ * make_attrmap
+ *
+ * Utility routine to allocate an attribute map in the current memory
+ * context.
+ */
+AttrMap *
+make_attrmap(int maplen)
+{
+	AttrMap    *res;
+
+	res = (AttrMap *) palloc0(sizeof(AttrMap));
+	res->maplen = maplen;
+	res->attnums = (AttrNumber *) palloc0(sizeof(AttrNumber) * maplen);
+	return res;
+}
+
+/*
+ * free_attrmap
+ *
+ * Utility routine to release an attribute map.
+ */
+void
+free_attrmap(AttrMap *map)
+{
+	pfree(map->attnums);
+	pfree(map);
+}
+
+/*
+ * build_attrmap_by_position
+ *
+ * Return a palloc'd bare attribute map for tuple conversion, matching input
+ * and output columns by position.  Dropped columns are ignored in both input
+ * and output, marked as 0.  This is normally a subroutine for
+ * convert_tuples_by_position in tupconvert.c, but it can be used standalone.
+ *
+ * Note: the errdetail messages speak of indesc as the "returned" rowtype,
+ * outdesc as the "expected" rowtype.  This is okay for current uses but
+ * might need generalization in future.
+ */
+AttrMap *
+build_attrmap_by_position(TupleDesc indesc,
+						  TupleDesc outdesc,
+						  const char *msg)
+{
+	AttrMap    *attrMap;
+	int			nincols;
+	int			noutcols;
+	int			n;
+	int			i;
+	int			j;
+	bool		same;
+
+	/*
+	 * The length is computed as the number of attributes of the expected
+	 * rowtype as it includes dropped attributes in its count.
+	 */
+	n = outdesc->natts;
+	attrMap = make_attrmap(n);
+
+	j = 0;						/* j is next physical input attribute */
+	nincols = noutcols = 0;		/* these count non-dropped attributes */
+	same = true;
+	for (i = 0; i < n; i++)
+	{
+		Form_pg_attribute att = TupleDescAttr(outdesc, i);
+		Oid			atttypid;
+		int32		atttypmod;
+
+		if (att->attisdropped)
+			continue;			/* attrMap->attnums[i] is already 0 */
+		noutcols++;
+		atttypid = att->atttypid;
+		atttypmod = att->atttypmod;
+		for (; j < indesc->natts; j++)
+		{
+			att = TupleDescAttr(indesc, j);
+			if (att->attisdropped)
+				continue;
+			nincols++;
+
+			/* Found matching column, now check type */
+			if (atttypid != att->atttypid ||
+				(atttypmod != att->atttypmod && atttypmod >= 0))
+				ereport(ERROR,
+						(errcode(ERRCODE_DATATYPE_MISMATCH),
+						 errmsg_internal("%s", _(msg)),
+						 errdetail("Returned type %s does not match expected type %s in column %d.",
+								   format_type_with_typemod(att->atttypid,
+															att->atttypmod),
+								   format_type_with_typemod(atttypid,
+															atttypmod),
+								   noutcols)));
+			attrMap->attnums[i] = (AttrNumber) (j + 1);
+			j++;
+			break;
+		}
+		if (attrMap->attnums[i] == 0)
+			same = false;		/* we'll complain below */
+	}
+
+	/* Check for unused input columns */
+	for (; j < indesc->natts; j++)
+	{
+		if (TupleDescAttr(indesc, j)->attisdropped)
+			continue;
+		nincols++;
+		same = false;			/* we'll complain below */
+	}
+
+	/* Report column count mismatch using the non-dropped-column counts */
+	if (!same)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATATYPE_MISMATCH),
+				 errmsg_internal("%s", _(msg)),
+				 errdetail("Number of returned columns (%d) does not match "
+						   "expected column count (%d).",
+						   nincols, noutcols)));
+
+	/* Check if the map has a one-to-one match */
+	if (check_attrmap_match(indesc, outdesc, attrMap))
+	{
+		/* Runtime conversion is not needed */
+		free_attrmap(attrMap);
+		return NULL;
+	}
+
+	return attrMap;
+}
+
+/*
+ * build_attrmap_by_name
+ *
+ * Return a palloc'd bare attribute map for tuple conversion, matching input
+ * and output columns by name.  (Dropped columns are ignored in both input and
+ * output.)  This is normally a subroutine for convert_tuples_by_name in
+ * tupconvert.c, but can be used standalone.
+ */
+AttrMap *
+build_attrmap_by_name(TupleDesc indesc,
+					  TupleDesc outdesc)
+{
+	AttrMap    *attrMap;
+	int			outnatts;
+	int			innatts;
+	int			i;
+	int			nextindesc = -1;
+
+	outnatts = outdesc->natts;
+	innatts = indesc->natts;
+
+	attrMap = make_attrmap(outnatts);
+	for (i = 0; i < outnatts; i++)
+	{
+		Form_pg_attribute outatt = TupleDescAttr(outdesc, i);
+		char	   *attname;
+		Oid			atttypid;
+		int32		atttypmod;
+		int			j;
+
+		if (outatt->attisdropped)
+			continue;			/* attrMap->attnums[i] is already 0 */
+		attname = NameStr(outatt->attname);
+		atttypid = outatt->atttypid;
+		atttypmod = outatt->atttypmod;
+
+		/*
+		 * Now search for an attribute with the same name in the indesc. It
+		 * seems likely that a partitioned table will have the attributes in
+		 * the same order as the partition, so the search below is optimized
+		 * for that case.  It is possible that columns are dropped in one of
+		 * the relations, but not the other, so we use the 'nextindesc'
+		 * counter to track the starting point of the search.  If the inner
+		 * loop encounters dropped columns then it will have to skip over
+		 * them, but it should leave 'nextindesc' at the correct position for
+		 * the next outer loop.
+		 */
+		for (j = 0; j < innatts; j++)
+		{
+			Form_pg_attribute inatt;
+
+			nextindesc++;
+			if (nextindesc >= innatts)
+				nextindesc = 0;
+
+			inatt = TupleDescAttr(indesc, nextindesc);
+			if (inatt->attisdropped)
+				continue;
+			if (strcmp(attname, NameStr(inatt->attname)) == 0)
+			{
+				/* Found it, check type */
+				if (atttypid != inatt->atttypid || atttypmod != inatt->atttypmod)
+					ereport(ERROR,
+							(errcode(ERRCODE_DATATYPE_MISMATCH),
+							 errmsg("could not convert row type"),
+							 errdetail("Attribute \"%s\" of type %s does not match corresponding attribute of type %s.",
+									   attname,
+									   format_type_be(outdesc->tdtypeid),
+									   format_type_be(indesc->tdtypeid))));
+				attrMap->attnums[i] = inatt->attnum;
+				break;
+			}
+		}
+		if (attrMap->attnums[i] == 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_DATATYPE_MISMATCH),
+					 errmsg("could not convert row type"),
+					 errdetail("Attribute \"%s\" of type %s does not exist in type %s.",
+							   attname,
+							   format_type_be(outdesc->tdtypeid),
+							   format_type_be(indesc->tdtypeid))));
+	}
+	return attrMap;
+}
+
+/*
+ * build_attrmap_by_name_if_req
+ *
+ * Returns mapping created by build_attrmap_by_name, or NULL if no
+ * conversion is required.  This is a convenience routine for
+ * convert_tuples_by_name() in tupconvert.c and other functions, but it
+ * can be used standalone.
+ */
+AttrMap *
+build_attrmap_by_name_if_req(TupleDesc indesc,
+							 TupleDesc outdesc)
+{
+	AttrMap    *attrMap;
+
+	/* Verify compatibility and prepare attribute-number map */
+	attrMap = build_attrmap_by_name(indesc, outdesc);
+
+	/* Check if the map has a one-to-one match */
+	if (check_attrmap_match(indesc, outdesc, attrMap))
+	{
+		/* Runtime conversion is not needed */
+		free_attrmap(attrMap);
+		return NULL;
+	}
+
+	return attrMap;
+}
+
+/*
+ * check_attrmap_match
+ *
+ * Check to see if the map is a one-to-one match, in which case we need
+ * not to do a tuple conversion, and the attribute map is not necessary.
+ */
+static bool
+check_attrmap_match(TupleDesc indesc,
+					TupleDesc outdesc,
+					AttrMap *attrMap)
+{
+	int			i;
+
+	/* no match if attribute numbers are not the same */
+	if (indesc->natts != outdesc->natts)
+		return false;
+
+	for (i = 0; i < attrMap->maplen; i++)
+	{
+		Form_pg_attribute inatt;
+		Form_pg_attribute outatt;
+
+		if (attrMap->attnums[i] == (i + 1))
+			continue;
+
+		/*
+		 * If it's a dropped column and the corresponding input column is also
+		 * dropped, we don't need a conversion.  However, attlen and attalign
+		 * must agree.
+		 */
+		inatt = TupleDescAttr(indesc, i);
+		outatt = TupleDescAttr(outdesc, i);
+		if (attrMap->attnums[i] == 0 &&
+			inatt->attisdropped &&
+			inatt->attlen == outatt->attlen &&
+			inatt->attalign == outatt->attalign)
+			continue;
+
+		return false;
+	}
+
+	return true;
+}

src/backend/catalog/index.c

@@ -2381,13 +2381,13 @@ BuildDummyIndexInfo(Relation index)
  * Note: passing collations and opfamilies separately is a kludge.  Adding
  * them to IndexInfo may result in better coding here and elsewhere.
  *
- * Use convert_tuples_by_name_map(index2, index1) to build the attmap.
+ * Use build_attrmap_by_name(index2, index1) to build the attmap.
  */
 bool
 CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
 				 Oid *collations1, Oid *collations2,
 				 Oid *opfamilies1, Oid *opfamilies2,
-				 AttrNumber *attmap, int maplen)
+				 AttrMap *attmap)
 {
 	int			i;
 
@@ -2414,12 +2414,12 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
 	 */
 	for (i = 0; i < info1->ii_NumIndexAttrs; i++)
 	{
-		if (maplen < info2->ii_IndexAttrNumbers[i])
+		if (attmap->maplen < info2->ii_IndexAttrNumbers[i])
 			elog(ERROR, "incorrect attribute map");
 
 		/* ignore expressions at this stage */
 		if ((info1->ii_IndexAttrNumbers[i] != InvalidAttrNumber) &&
-			(attmap[info2->ii_IndexAttrNumbers[i] - 1] !=
+			(attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] !=
 			 info1->ii_IndexAttrNumbers[i]))
 			return false;
 
@@ -2445,7 +2445,7 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
 		Node	   *mapped;
 
 		mapped = map_variable_attnos((Node *) info2->ii_Expressions,
-									 1, 0, attmap, maplen,
+									 1, 0, attmap,
 									 InvalidOid, &found_whole_row);
 		if (found_whole_row)
 		{
@@ -2469,7 +2469,7 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
 		Node	   *mapped;
 
 		mapped = map_variable_attnos((Node *) info2->ii_Predicate,
-									 1, 0, attmap, maplen,
+									 1, 0, attmap,
 									 InvalidOid, &found_whole_row);
 		if (found_whole_row)
 		{

src/backend/catalog/partition.c

@@ -14,11 +14,11 @@
 */
 #include "postgres.h"
 
+#include "access/attmap.h"
 #include "access/genam.h"
 #include "access/htup_details.h"
 #include "access/sysattr.h"
 #include "access/table.h"
-#include "access/tupconvert.h"
 #include "catalog/indexing.h"
 #include "catalog/partition.h"
 #include "catalog/pg_inherits.h"
@@ -206,14 +206,13 @@ map_partition_varattnos(List *expr, int fromrel_varno,
 
 	if (expr != NIL)
 	{
-		AttrNumber *part_attnos;
+		AttrMap    *part_attmap;
 
-		part_attnos = convert_tuples_by_name_map(RelationGetDescr(to_rel),
-												 RelationGetDescr(from_rel));
+		part_attmap = build_attrmap_by_name(RelationGetDescr(to_rel),
+											RelationGetDescr(from_rel));
 		expr = (List *) map_variable_attnos((Node *) expr,
 											fromrel_varno, 0,
-											part_attnos,
-											RelationGetDescr(from_rel)->natts,
+											part_attmap,
 											RelationGetForm(to_rel)->reltype,
 											&my_found_whole_row);
 	}

src/backend/catalog/pg_constraint.c

@@ -18,7 +18,6 @@
 #include "access/htup_details.h"
 #include "access/sysattr.h"
 #include "access/table.h"
-#include "access/tupconvert.h"
 #include "access/xact.h"
 #include "catalog/catalog.h"
 #include "catalog/dependency.h"

src/backend/commands/indexcmds.c

@@ -1060,9 +1060,8 @@ DefineIndex(Oid relationId,
 				Relation	childrel;
 				List	   *childidxs;
 				ListCell   *cell;
-				AttrNumber *attmap;
+				AttrMap    *attmap;
 				bool		found = false;
-				int			maplen;
 
 				childrel = table_open(childRelid, lockmode);
 
@@ -1087,9 +1086,8 @@ DefineIndex(Oid relationId,
 
 				childidxs = RelationGetIndexList(childrel);
 				attmap =
-					convert_tuples_by_name_map(RelationGetDescr(childrel),
-											   parentDesc);
-				maplen = parentDesc->natts;
+					build_attrmap_by_name(RelationGetDescr(childrel),
+										  parentDesc);
 
 				foreach(cell, childidxs)
 				{
@@ -1108,7 +1106,7 @@ DefineIndex(Oid relationId,
 										 collationObjectId,
 										 cldidx->rd_opfamily,
 										 opfamOids,
-										 attmap, maplen))
+										 attmap))
 					{
 						Oid			cldConstrOid = InvalidOid;
 
@@ -1193,7 +1191,7 @@ DefineIndex(Oid relationId,
 						{
 							ielem->expr =
 								map_variable_attnos((Node *) ielem->expr,
-													1, 0, attmap, maplen,
+													1, 0, attmap,
 													InvalidOid,
 													&found_whole_row);
 							if (found_whole_row)
@@ -1202,7 +1200,7 @@ DefineIndex(Oid relationId,
 					}
 					childStmt->whereClause =
 						map_variable_attnos(stmt->whereClause, 1, 0,
-											attmap, maplen,
+											attmap,
 											InvalidOid, &found_whole_row);
 					if (found_whole_row)
 						elog(ERROR, "cannot convert whole-row table reference");
@@ -1217,7 +1215,7 @@ DefineIndex(Oid relationId,
 
 				pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
 											 i + 1);
-				pfree(attmap);
+				free_attrmap(attmap);
 			}
 
 			/*

src/backend/executor/execMain.c

@@ -1843,14 +1843,14 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo,
 	if (resultRelInfo->ri_PartitionRoot)
 	{
 		TupleDesc	old_tupdesc;
-		AttrNumber *map;
+		AttrMap    *map;
 
 		root_relid = RelationGetRelid(resultRelInfo->ri_PartitionRoot);
 		tupdesc = RelationGetDescr(resultRelInfo->ri_PartitionRoot);
 
 		old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
 		/* a reverse map */
-		map = convert_tuples_by_name_map_if_req(old_tupdesc, tupdesc);
+		map = build_attrmap_by_name_if_req(old_tupdesc, tupdesc);
 
 		/*
 		 * Partition-specific slot's tupdesc can't be changed, so allocate a
@@ -1929,13 +1929,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 				 */
 				if (resultRelInfo->ri_PartitionRoot)
 				{
-					AttrNumber *map;
+					AttrMap    *map;
 
 					rel = resultRelInfo->ri_PartitionRoot;
 					tupdesc = RelationGetDescr(rel);
 					/* a reverse map */
-					map = convert_tuples_by_name_map_if_req(orig_tupdesc,
-															tupdesc);
+					map = build_attrmap_by_name_if_req(orig_tupdesc,
+													   tupdesc);
 
 					/*
 					 * Partition-specific slot's tupdesc can't be changed, so
@@ -1978,13 +1978,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
 			if (resultRelInfo->ri_PartitionRoot)
 			{
 				TupleDesc	old_tupdesc = RelationGetDescr(rel);
-				AttrNumber *map;
+				AttrMap    *map;
 
 				rel = resultRelInfo->ri_PartitionRoot;
 				tupdesc = RelationGetDescr(rel);
 				/* a reverse map */
-				map = convert_tuples_by_name_map_if_req(old_tupdesc,
-														tupdesc);
+				map = build_attrmap_by_name_if_req(old_tupdesc,
+												   tupdesc);
 
 				/*
 				 * Partition-specific slot's tupdesc can't be changed, so
@@ -2085,13 +2085,13 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
 					if (resultRelInfo->ri_PartitionRoot)
 					{
 						TupleDesc	old_tupdesc = RelationGetDescr(rel);
-						AttrNumber *map;
+						AttrMap    *map;
 
 						rel = resultRelInfo->ri_PartitionRoot;
 						tupdesc = RelationGetDescr(rel);
 						/* a reverse map */
-						map = convert_tuples_by_name_map_if_req(old_tupdesc,
-																tupdesc);
+						map = build_attrmap_by_name_if_req(old_tupdesc,
+														   tupdesc);
 
 						/*
 						 * Partition-specific slot's tupdesc can't be changed,

src/backend/executor/execPartition.c

@@ -143,7 +143,7 @@ typedef struct PartitionDispatchData
 	List	   *keystate;		/* list of ExprState */
 	PartitionDesc partdesc;
 	TupleTableSlot *tupslot;
-	AttrNumber *tupmap;
+	AttrMap    *tupmap;
 	int			indexes[FLEXIBLE_ARRAY_MEMBER];
 }			PartitionDispatchData;
 
@@ -298,7 +298,7 @@ ExecFindPartition(ModifyTableState *mtstate,
 	dispatch = pd[0];
 	while (true)
 	{
-		AttrNumber *map = dispatch->tupmap;
+		AttrMap    *map = dispatch->tupmap;
 		int			partidx = -1;
 
 		CHECK_FOR_INTERRUPTS();
@@ -511,7 +511,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
 	Relation	firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
 	ResultRelInfo *leaf_part_rri;
 	MemoryContext oldcxt;
-	AttrNumber *part_attnos = NULL;
+	AttrMap    *part_attmap = NULL;
 	bool		found_whole_row;
 
 	oldcxt = MemoryContextSwitchTo(proute->memcxt);
@@ -584,14 +584,13 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
 		/*
 		 * Convert Vars in it to contain this partition's attribute numbers.
 		 */
-		part_attnos =
-			convert_tuples_by_name_map(RelationGetDescr(partrel),
-									   RelationGetDescr(firstResultRel));
+		part_attmap =
+			build_attrmap_by_name(RelationGetDescr(partrel),
+								  RelationGetDescr(firstResultRel));
 		wcoList = (List *)
 			map_variable_attnos((Node *) wcoList,
 								firstVarno, 0,
-								part_attnos,
-								RelationGetDescr(firstResultRel)->natts,
+								part_attmap,
 								RelationGetForm(partrel)->reltype,
 								&found_whole_row);
 		/* We ignore the value of found_whole_row. */
@@ -642,15 +641,14 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
 		/*
 		 * Convert Vars in it to contain this partition's attribute numbers.
 		 */
-		if (part_attnos == NULL)
-			part_attnos =
-				convert_tuples_by_name_map(RelationGetDescr(partrel),
-										   RelationGetDescr(firstResultRel));
+		if (part_attmap == NULL)
+			part_attmap =
+				build_attrmap_by_name(RelationGetDescr(partrel),
+									  RelationGetDescr(firstResultRel));
 		returningList = (List *)
 			map_variable_attnos((Node *) returningList,
 								firstVarno, 0,
-								part_attnos,
-								RelationGetDescr(firstResultRel)->natts,
+								part_attmap,
 								RelationGetForm(partrel)->reltype,
 								&found_whole_row);
 		/* We ignore the value of found_whole_row. */
@@ -785,23 +783,21 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
 				 * target relation (firstVarno).
 				 */
 				onconflset = (List *) copyObject((Node *) node->onConflictSet);
-				if (part_attnos == NULL)
-					part_attnos =
-						convert_tuples_by_name_map(RelationGetDescr(partrel),
-												   RelationGetDescr(firstResultRel));
+				if (part_attmap == NULL)
+					part_attmap =
+						build_attrmap_by_name(RelationGetDescr(partrel),
+											  RelationGetDescr(firstResultRel));
 				onconflset = (List *)
 					map_variable_attnos((Node *) onconflset,
 										INNER_VAR, 0,
-										part_attnos,
-										RelationGetDescr(firstResultRel)->natts,
+										part_attmap,
 										RelationGetForm(partrel)->reltype,
 										&found_whole_row);
 				/* We ignore the value of found_whole_row. */
 				onconflset = (List *)
 					map_variable_attnos((Node *) onconflset,
 										firstVarno, 0,
-										part_attnos,
-										RelationGetDescr(firstResultRel)->natts,
+										part_attmap,
 										RelationGetForm(partrel)->reltype,
 										&found_whole_row);
 				/* We ignore the value of found_whole_row. */
@@ -835,16 +831,14 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate,
 					clause = (List *)
 						map_variable_attnos((Node *) clause,
 											INNER_VAR, 0,
-											part_attnos,
-											RelationGetDescr(firstResultRel)->natts,
+											part_attmap,
 											RelationGetForm(partrel)->reltype,
 											&found_whole_row);
 					/* We ignore the value of found_whole_row. */
 					clause = (List *)
 						map_variable_attnos((Node *) clause,
 											firstVarno, 0,
-											part_attnos,
-											RelationGetDescr(firstResultRel)->natts,
+											part_attmap,
 											RelationGetForm(partrel)->reltype,
 											&found_whole_row);
 					/* We ignore the value of found_whole_row. */
@@ -1036,8 +1030,8 @@ ExecInitPartitionDispatchInfo(EState *estate,
 		 * tuple descriptor when computing its partition key for tuple
 		 * routing.
 		 */
-		pd->tupmap = convert_tuples_by_name_map_if_req(RelationGetDescr(parent_pd->reldesc),
-													   tupdesc);
+		pd->tupmap = build_attrmap_by_name_if_req(RelationGetDescr(parent_pd->reldesc),
+												  tupdesc);
 		pd->tupslot = pd->tupmap ?
 			MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual) : NULL;
 	}
@@ -1434,15 +1428,16 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
 {
 	List	   *new_tlist = NIL;
 	TupleDesc	tupdesc = map->outdesc;
-	AttrNumber *attrMap = map->attrMap;
+	AttrMap    *attrMap = map->attrMap;
 	AttrNumber	attrno;
 
+	Assert(tupdesc->natts == attrMap->maplen);
 	for (attrno = 1; attrno <= tupdesc->natts; attrno++)
 	{
 		Form_pg_attribute att_tup = TupleDescAttr(tupdesc, attrno - 1);
 		TargetEntry *tle;
 
-		if (attrMap[attrno - 1] != InvalidAttrNumber)
+		if (attrMap->attnums[attrno - 1] != InvalidAttrNumber)
 		{
 			Assert(!att_tup->attisdropped);
 
@@ -1450,7 +1445,7 @@ adjust_partition_tlist(List *tlist, TupleConversionMap *map)
 			 * Use the corresponding entry from the parent's tlist, adjusting
 			 * the resno the match the partition's attno.
 			 */
-			tle = (TargetEntry *) list_nth(tlist, attrMap[attrno - 1] - 1);
+			tle = (TargetEntry *) list_nth(tlist, attrMap->attnums[attrno - 1] - 1);
 			tle->resno = attrno;
 		}
 		else

src/backend/jit/llvm/llvmjit_expr.c

@@ -20,7 +20,6 @@
 
 #include "access/htup_details.h"
 #include "access/nbtree.h"
-#include "access/tupconvert.h"
 #include "catalog/objectaccess.h"
 #include "catalog/pg_type.h"
 #include "executor/execExpr.h"

src/backend/parser/parse_utilcmd.c

@@ -917,7 +917,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 	Relation	relation;
 	TupleDesc	tupleDesc;
 	TupleConstr *constr;
-	AttrNumber *attmap;
+	AttrMap    *attmap;
 	AclResult	aclresult;
 	char	   *comment;
 	ParseCallbackState pcbstate;
@@ -974,7 +974,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 	 * since dropped columns in the source table aren't copied, so the new
 	 * table can have different column numbers.
 	 */
-	attmap = (AttrNumber *) palloc0(sizeof(AttrNumber) * tupleDesc->natts);
+	attmap = make_attrmap(tupleDesc->natts);
 
 	/*
 	 * Insert the copied attributes into the cxt for the new table definition.
@@ -1020,7 +1020,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 		 */
 		cxt->columns = lappend(cxt->columns, def);
 
-		attmap[parent_attno - 1] = list_length(cxt->columns);
+		attmap->attnums[parent_attno - 1] = list_length(cxt->columns);
 
 		/*
 		 * Copy default, if present and it should be copied.  We have separate
@@ -1051,7 +1051,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 
 			def->cooked_default = map_variable_attnos(this_default,
 													  1, 0,
-													  attmap, tupleDesc->natts,
+													  attmap,
 													  InvalidOid, &found_whole_row);
 
 			/*
@@ -1134,7 +1134,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 
 			ccbin_node = map_variable_attnos(stringToNode(ccbin),
 											 1, 0,
-											 attmap, tupleDesc->natts,
+											 attmap,
 											 InvalidOid, &found_whole_row);
 
 			/*
@@ -1200,7 +1200,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
 			/* Build CREATE INDEX statement to recreate the parent_index */
 			index_stmt = generateClonedIndexStmt(cxt->relation,
 												 parent_index,
-												 attmap, tupleDesc->natts,
+												 attmap,
 												 NULL);
 
 			/* Copy comment on index, if requested */
@@ -1332,7 +1332,7 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
  */
 IndexStmt *
 generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
-						const AttrNumber *attmap, int attmap_length,
+						const AttrMap *attmap,
 						Oid *constraintOid)
 {
 	Oid			source_relid = RelationGetRelid(source_idx);
@@ -1552,7 +1552,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
 			/* Adjust Vars to match new table's column numbering */
 			indexkey = map_variable_attnos(indexkey,
 										   1, 0,
-										   attmap, attmap_length,
+										   attmap,
 										   InvalidOid, &found_whole_row);
 
 			/* As in transformTableLikeClause, reject whole-row variables */
@@ -1659,7 +1659,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
 		/* Adjust Vars to match new table's column numbering */
 		pred_tree = map_variable_attnos(pred_tree,
 										1, 0,
-										attmap, attmap_length,
+										attmap,
 										InvalidOid, &found_whole_row);
 
 		/* As in transformTableLikeClause, reject whole-row variables */

src/backend/replication/logical/relation.c

@@ -281,7 +281,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 		 */
 		desc = RelationGetDescr(entry->localrel);
 		oldctx = MemoryContextSwitchTo(LogicalRepRelMapContext);
-		entry->attrmap = palloc(desc->natts * sizeof(AttrNumber));
+		entry->attrmap = make_attrmap(desc->natts);
 		MemoryContextSwitchTo(oldctx);
 
 		found = 0;
@@ -292,14 +292,14 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 
 			if (attr->attisdropped || attr->attgenerated)
 			{
-				entry->attrmap[i] = -1;
+				entry->attrmap->attnums[i] = -1;
 				continue;
 			}
 
 			attnum = logicalrep_rel_att_by_name(remoterel,
 												NameStr(attr->attname));
 
-			entry->attrmap[i] = attnum;
+			entry->attrmap->attnums[i] = attnum;
 			if (attnum >= 0)
 				found++;
 		}
@@ -354,8 +354,8 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 
 			attnum = AttrNumberGetAttrOffset(attnum);
 
-			if (entry->attrmap[attnum] < 0 ||
-				!bms_is_member(entry->attrmap[attnum], remoterel->attkeys))
+			if (entry->attrmap->attnums[attnum] < 0 ||
+				!bms_is_member(entry->attrmap->attnums[attnum], remoterel->attkeys))
 			{
 				entry->updatable = false;
 				break;

src/backend/replication/logical/worker.c

@@ -230,6 +230,7 @@ slot_fill_defaults(LogicalRepRelMapEntry *rel, EState *estate,
 	defmap = (int *) palloc(num_phys_attrs * sizeof(int));
 	defexprs = (ExprState **) palloc(num_phys_attrs * sizeof(ExprState *));
 
+	Assert(rel->attrmap->maplen == num_phys_attrs);
 	for (attnum = 0; attnum < num_phys_attrs; attnum++)
 	{
 		Expr	   *defexpr;
@@ -237,7 +238,7 @@ slot_fill_defaults(LogicalRepRelMapEntry *rel, EState *estate,
 		if (TupleDescAttr(desc, attnum)->attisdropped || TupleDescAttr(desc, attnum)->attgenerated)
 			continue;
 
-		if (rel->attrmap[attnum] >= 0)
+		if (rel->attrmap->attnums[attnum] >= 0)
 			continue;
 
 		defexpr = (Expr *) build_column_default(rel->localrel, attnum + 1);
@@ -319,10 +320,11 @@ slot_store_cstrings(TupleTableSlot *slot, LogicalRepRelMapEntry *rel,
 	error_context_stack = &errcallback;
 
 	/* Call the "in" function for each non-dropped attribute */
+	Assert(natts == rel->attrmap->maplen);
 	for (i = 0; i < natts; i++)
 	{
 		Form_pg_attribute att = TupleDescAttr(slot->tts_tupleDescriptor, i);
-		int			remoteattnum = rel->attrmap[i];
+		int			remoteattnum = rel->attrmap->attnums[i];
 
 		if (!att->attisdropped && remoteattnum >= 0 &&
 			values[remoteattnum] != NULL)
@@ -403,10 +405,11 @@ slot_modify_cstrings(TupleTableSlot *slot, TupleTableSlot *srcslot,
 	error_context_stack = &errcallback;
 
 	/* Call the "in" function for each replaced attribute */
+	Assert(natts == rel->attrmap->maplen);
 	for (i = 0; i < natts; i++)
 	{
 		Form_pg_attribute att = TupleDescAttr(slot->tts_tupleDescriptor, i);
-		int			remoteattnum = rel->attrmap[i];
+		int			remoteattnum = rel->attrmap->attnums[i];
 
 		if (remoteattnum < 0)
 			continue;

src/backend/rewrite/rewriteManip.c

@@ -1221,8 +1221,7 @@ typedef struct
 {
 	int			target_varno;	/* RTE index to search for */
 	int			sublevels_up;	/* (current) nesting depth */
-	const AttrNumber *attno_map;	/* map array for user attnos */
-	int			map_length;		/* number of entries in attno_map[] */
+	const AttrMap *attno_map;	/* map array for user attnos */
 	Oid			to_rowtype;		/* change whole-row Vars to this type */
 	bool	   *found_whole_row;	/* output flag */
 } map_variable_attnos_context;
@@ -1249,11 +1248,11 @@ map_variable_attnos_mutator(Node *node,
 			if (attno > 0)
 			{
 				/* user-defined column, replace attno */
-				if (attno > context->map_length ||
-					context->attno_map[attno - 1] == 0)
+				if (attno > context->attno_map->maplen ||
+					context->attno_map->attnums[attno - 1] == 0)
 					elog(ERROR, "unexpected varattno %d in expression to be mapped",
 						 attno);
-				newvar->varattno = newvar->varoattno = context->attno_map[attno - 1];
+				newvar->varattno = newvar->varoattno = context->attno_map->attnums[attno - 1];
 			}
 			else if (attno == 0)
 			{
@@ -1350,7 +1349,7 @@ map_variable_attnos_mutator(Node *node,
 Node *
 map_variable_attnos(Node *node,
 					int target_varno, int sublevels_up,
-					const AttrNumber *attno_map, int map_length,
+					const AttrMap *attno_map,
 					Oid to_rowtype, bool *found_whole_row)
 {
 	map_variable_attnos_context context;
@@ -1358,7 +1357,6 @@ map_variable_attnos(Node *node,
 	context.target_varno = target_varno;
 	context.sublevels_up = sublevels_up;
 	context.attno_map = attno_map;
-	context.map_length = map_length;
 	context.to_rowtype = to_rowtype;
 	context.found_whole_row = found_whole_row;
 

src/include/access/attmap.h

+/*-------------------------------------------------------------------------
+ *
+ * attmap.h
+ *	  Definitions for PostgreSQL attribute mappings
+ *
+ *
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/access/attmap.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef ATTMAP_H
+#define ATTMAP_H
+
+#include "access/attnum.h"
+#include "access/tupdesc.h"
+
+/*
+ * Attribute mapping structure
+ *
+ * This maps attribute numbers between a pair of relations, designated
+ * 'input' and 'output' (most typically inheritance parent and child
+ * relations), whose common columns may have different attribute numbers.
+ * Such difference may arise due to the columns being ordered differently
+ * in the two relations or the two relations having dropped columns at
+ * different positions.
+ *
+ * 'maplen' is set to the number of attributes of the 'output' relation,
+ * taking into account any of its dropped attributes, with the corresponding
+ * elements of the 'attnums' array set to 0.
+ */
+typedef struct AttrMap
+{
+	AttrNumber *attnums;
+	int			maplen;
+} AttrMap;
+
+extern AttrMap *make_attrmap(int maplen);
+extern void free_attrmap(AttrMap *map);
+
+/* Convertion routines to build mappings */
+extern AttrMap *build_attrmap_by_name(TupleDesc indesc,
+									  TupleDesc outdesc);
+extern AttrMap *build_attrmap_by_name_if_req(TupleDesc indesc,
+											 TupleDesc outdesc);
+extern AttrMap *build_attrmap_by_position(TupleDesc indesc,
+										  TupleDesc outdesc,
+										  const char *msg);
+
+#endif							/* ATTMAP_H */

src/include/access/tupconvert.h

@@ -14,6 +14,7 @@
 #ifndef TUPCONVERT_H
 #define TUPCONVERT_H
 
+#include "access/attmap.h"
 #include "access/htup.h"
 #include "access/tupdesc.h"
 #include "executor/tuptable.h"
@@ -23,7 +24,7 @@ typedef struct TupleConversionMap
 {
 	TupleDesc	indesc;			/* tupdesc for source rowtype */
 	TupleDesc	outdesc;		/* tupdesc for result rowtype */
-	AttrNumber *attrMap;		/* indexes of input fields, or 0 for null */
+	AttrMap    *attrMap;		/* indexes of input fields, or 0 for null */
 	Datum	   *invalues;		/* workspace for deconstructing source */
 	bool	   *inisnull;
 	Datum	   *outvalues;		/* workspace for constructing result */
@@ -38,14 +39,10 @@ extern TupleConversionMap *convert_tuples_by_position(TupleDesc indesc,
 extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc,
 												  TupleDesc outdesc);
 
-extern AttrNumber *convert_tuples_by_name_map(TupleDesc indesc,
-											  TupleDesc outdesc);
-extern AttrNumber *convert_tuples_by_name_map_if_req(TupleDesc indesc,
-													 TupleDesc outdesc);
-
 extern HeapTuple execute_attr_map_tuple(HeapTuple tuple, TupleConversionMap *map);
-extern TupleTableSlot *execute_attr_map_slot(AttrNumber *attrMap,
-											 TupleTableSlot *in_slot, TupleTableSlot *out_slot);
+extern TupleTableSlot *execute_attr_map_slot(AttrMap *attrMap,
+											 TupleTableSlot *in_slot,
+											 TupleTableSlot *out_slot);
 
 extern void free_conversion_map(TupleConversionMap *map);
 

src/include/catalog/index.h

@@ -111,7 +111,7 @@ extern IndexInfo *BuildDummyIndexInfo(Relation index);
 extern bool CompareIndexInfo(IndexInfo *info1, IndexInfo *info2,
 							 Oid *collations1, Oid *collations2,
 							 Oid *opfamilies1, Oid *opfamilies2,
-							 AttrNumber *attmap, int maplen);
+							 AttrMap *attmap);
 
 extern void BuildSpeculativeIndexInfo(Relation index, IndexInfo *ii);
 

src/include/parser/parse_utilcmd.h

@@ -16,6 +16,7 @@
 
 #include "parser/parse_node.h"
 
+typedef struct AttrMap AttrMap;
 
 extern List *transformCreateStmt(CreateStmt *stmt, const char *queryString);
 extern List *transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
@@ -29,7 +30,7 @@ extern PartitionBoundSpec *transformPartitionBound(ParseState *pstate, Relation
 												   PartitionBoundSpec *spec);
 extern IndexStmt *generateClonedIndexStmt(RangeVar *heapRel,
 										  Relation source_idx,
-										  const AttrNumber *attmap, int attmap_length,
+										  const AttrMap *attmap,
 										  Oid *constraintOid);
 
 #endif							/* PARSE_UTILCMD_H */

src/include/replication/logicalrelation.h

@@ -12,6 +12,7 @@
 #ifndef LOGICALRELATION_H
 #define LOGICALRELATION_H
 
+#include "access/attmap.h"
 #include "replication/logicalproto.h"
 
 typedef struct LogicalRepRelMapEntry
@@ -21,7 +22,7 @@ typedef struct LogicalRepRelMapEntry
 	/* Mapping to local relation, filled as needed. */
 	Oid			localreloid;	/* local relation id */
 	Relation	localrel;		/* relcache entry */
-	AttrNumber *attrmap;		/* map of local attributes to remote ones */
+	AttrMap    *attrmap;		/* map of local attributes to remote ones */
 	bool		updatable;		/* Can apply updates/deletes? */
 
 	/* Sync state. */

src/include/rewrite/rewriteManip.h

@@ -17,6 +17,7 @@
 #include "nodes/parsenodes.h"
 
 
+typedef struct AttrMap AttrMap;
 typedef struct replace_rte_variables_context replace_rte_variables_context;
 
 typedef Node *(*replace_rte_variables_callback) (Var *var,
@@ -71,7 +72,7 @@ extern Node *replace_rte_variables_mutator(Node *node,
 
 extern Node *map_variable_attnos(Node *node,
 								 int target_varno, int sublevels_up,
-								 const AttrNumber *attno_map, int map_length,
+								 const AttrMap *attno_map,
 								 Oid to_rowtype, bool *found_whole_row);
 
 extern Node *ReplaceVarsFromTargetList(Node *node,

src/tools/pgindent/typedefs.list

@@ -137,6 +137,7 @@ AttoptCacheEntry
 AttoptCacheKey
 AttrDefInfo
 AttrDefault
+AttrMap
 AttrMissing
 AttrNumber
 AttributeOpts