fix for table syncing with different column order

From b3ba66ae249291cfd0e39d622af766a0f7e13e7e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Thu, 18 May 2017 14:16:16 -0400
Subject: [PATCH] Fix table syncing with different column order

Logical replication supports replicating between tables with different
column order.  But this failed for the initial table sync because of a
logic error in how the column list for the internal COPY command was
composed.  Fix that and also add a test.

Also fix a minor omission in the column name mapping cache.  When
creating the mapping list, it would not skip locally dropped columns.
So if a remote column had the same name as a locally dropped
column (...pg.dropped...), then the expected error would not occur.
---
 src/backend/replication/logical/relation.c  |  9 +++++++--
 src/backend/replication/logical/tablesync.c | 16 +++-------------
 src/test/subscription/t/001_rep_changes.pl  | 15 +++++++++++++--
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 41eff8971a..e65f2865dd 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -277,8 +277,13 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 		found = 0;
 		for (i = 0; i < desc->natts; i++)
 		{
-			int			attnum = logicalrep_rel_att_by_name(remoterel,
-										   NameStr(desc->attrs[i]->attname));
+			int			attnum;
+
+			if (desc->attrs[i]->attisdropped)
+				continue;
+
+			attnum = logicalrep_rel_att_by_name(remoterel,
+												NameStr(desc->attrs[i]->attname));
 
 			entry->attrmap[i] = attnum;
 			if (attnum >= 0)
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 1e3753b8fe..50be6673b4 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -489,25 +489,15 @@ static List *
 make_copy_attnamelist(LogicalRepRelMapEntry *rel)
 {
 	List	   *attnamelist = NIL;
-	TupleDesc	desc = RelationGetDescr(rel->localrel);
 	int			i;
 
-	for (i = 0; i < desc->natts; i++)
+	for (i = 0; i < rel->remoterel.natts; i++)
 	{
-		int			remoteattnum = rel->attrmap[i];
-
-		/* Skip dropped attributes. */
-		if (desc->attrs[i]->attisdropped)
-			continue;
-
-		/* Skip attributes that are missing on remote side. */
-		if (remoteattnum < 0)
-			continue;
-
 		attnamelist = lappend(attnamelist,
-						  makeString(rel->remoterel.attnames[remoteattnum]));
+							  makeString(rel->remoterel.attnames[i]));
 	}
 
+
 	return attnamelist;
 }
 
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index 6d65388b22..e5638d3322 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -3,7 +3,7 @@
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 14;
+use Test::More tests => 15;
 
 # Initialize publisher node
 my $node_publisher = get_new_node('publisher');
@@ -24,6 +24,10 @@
 	"CREATE TABLE tab_full AS SELECT generate_series(1,10) AS a");
 $node_publisher->safe_psql('postgres',
 	"CREATE TABLE tab_rep (a int primary key)");
+$node_publisher->safe_psql('postgres',
+    "CREATE TABLE tab_mixed (a int primary key, b text)");
+$node_publisher->safe_psql('postgres',
+    "INSERT INTO tab_mixed (a, b) VALUES (1, 'foo')");
 
 # Setup structure on subscriber
 $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_notrep (a int)");
@@ -31,6 +35,9 @@
 $node_subscriber->safe_psql('postgres', "CREATE TABLE tab_full (a int)");
 $node_subscriber->safe_psql('postgres',
 	"CREATE TABLE tab_rep (a int primary key)");
+# different column count and order than on publisher
+$node_subscriber->safe_psql('postgres',
+    "CREATE TABLE tab_mixed (c text, b text, a int primary key)");
 
 # Setup logical replication
 my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
@@ -38,7 +45,7 @@
 $node_publisher->safe_psql('postgres',
 	"CREATE PUBLICATION tap_pub_ins_only WITH (publish = insert)");
 $node_publisher->safe_psql('postgres',
-	"ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full");
+	"ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_mixed");
 $node_publisher->safe_psql('postgres',
 	"ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_ins");
 
@@ -88,6 +95,10 @@
 	"SELECT count(*), min(a), max(a) FROM tab_rep");
 is($result, qq(20|-20|-1), 'check replicated changes on subscriber');
 
+$result = $node_subscriber->safe_psql('postgres',
+	"SELECT c, b, a FROM tab_mixed");
+is($result, qq(|foo|1), 'check replicated changes with different column order');
+
 # insert some duplicate rows
 $node_publisher->safe_psql('postgres',
 	"INSERT INTO tab_full SELECT generate_series(1,10)");
-- 
2.13.0