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

6 files changed, 64 insertions, 12 deletions

diff --git a/src/include/access/attmap.h b/src/include/access/attmap.h
new file mode 100644
index 00000000000..57de8b65721
--- /dev/null
+++ b/src/include/access/attmap.h
@@ -0,0 +1,52 @@
+/*-------------------------------------------------------------------------
+ *
+ * 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 */
diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h
index 6d095f8e0d1..5ed7fe6d39a 100644
--- a/src/include/access/tupconvert.h
+++ b/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);
 
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 27d9e537d31..ea4ad1a2d1d 100644
--- a/src/include/catalog/index.h
+++ b/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);
 
diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h
index 1348064ad07..08dd0ce4ca7 100644
--- a/src/include/parser/parse_utilcmd.h
+++ b/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 */
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 2642a3f94ec..9922c2ed95a 100644
--- a/src/include/replication/logicalrelation.h
+++ b/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. */
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index 8d8fd17e41c..634cdc235da 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/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,