Merge pull request #539 from citusdata/fix/fix_496_wrong_storage_type_for_foreign_tables

Fix wrong storage type for foreign tables

src/backend/distributed/master/master_create_shards.c

@@ -158,7 +158,15 @@ master_create_worker_shards(PG_FUNCTION_ARGS)
 	/* set shard storage type according to relation type */
 	if (relationKind == RELKIND_FOREIGN_TABLE)
 	{
-		shardStorageType = SHARD_STORAGE_FOREIGN;
+		bool cstoreTable = CStoreTable(distributedTableId);
+		if (cstoreTable)
+		{
+			shardStorageType = SHARD_STORAGE_COLUMNAR;
+		}
+		else
+		{
+			shardStorageType = SHARD_STORAGE_FOREIGN;
+		}
 	}
 	else
 	{

src/backend/distributed/master/master_stage_protocol.c

@@ -84,14 +84,30 @@ master_create_empty_shard(PG_FUNCTION_ARGS)
 	char storageType = SHARD_STORAGE_TABLE;
 
 	Oid relationId = ResolveRelationId(relationNameText);
+	char relationKind = get_rel_relkind(relationId);
 	char *relationOwner = TableOwner(relationId);
 
 	EnsureTablePermissions(relationId, ACL_INSERT);
 	CheckDistributedTable(relationId);
 
-	if (CStoreTable(relationId))
+	/*
+	 * We check whether the table is a foreign table or not. If it is, we set
+	 * storage type as foreign also. Only exception is if foreign table is a
+	 * foreign cstore table, in this case we set storage type as columnar.
+	 *
+	 * i.e. While setting storage type, columnar has priority over foreign.
+	 */
+	if (relationKind == RELKIND_FOREIGN_TABLE)
 	{
-		storageType = SHARD_STORAGE_COLUMNAR;
+		bool cstoreTable = cstoreTable = CStoreTable(relationId);
+		if (cstoreTable)
+		{
+			storageType = SHARD_STORAGE_COLUMNAR;
+		}
+		else
+		{
+			storageType = SHARD_STORAGE_FOREIGN;
+		}
 	}
 
 	partitionMethod = PartitionMethod(relationId);

src/test/regress/expected/multi_fdw_storage_type.out

@@ -0,0 +1,63 @@
+--
+-- MULTI_FDW_STORAGE_TYPE
+--
+-- Create two tables one regular and one foreign, then check whether
+-- shardstorage is correct
+-- explicitly set shard id
+ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 400000;
+-- create regular table
+CREATE TABLE people (
+	id bigint not null,
+	firstname char(10) not null,
+	lastname char(10) not null,
+	age integer not null);
+-- create distributed table
+SELECT master_create_distributed_table('people', 'id', 'append');
+ master_create_distributed_table 
+---------------------------------
+ 
+(1 row)
+
+-- create worker shards
+SELECT master_create_empty_shard('people');
+ master_create_empty_shard 
+---------------------------
+                    400000
+(1 row)
+
+-- check shardstorage
+SELECT shardstorage FROM pg_dist_shard WHERE shardid = 400000;
+ shardstorage 
+--------------
+ t
+(1 row)
+
+-- create foreign table
+CREATE FOREIGN TABLE people_foreign (
+	id bigint not null,
+	firstname char(10) not null,
+	lastname char(10) not null,
+	age integer not null)
+SERVER file_server 
+OPTIONS (format 'text', filename '', delimiter '|', null '');
+-- create distributed table
+SELECT master_create_distributed_table('people_foreign', 'id', 'append');
+ master_create_distributed_table 
+---------------------------------
+ 
+(1 row)
+
+-- create worker shards
+SELECT master_create_empty_shard('people_foreign');
+ master_create_empty_shard 
+---------------------------
+                    400001
+(1 row)
+
+-- check shardstorage
+SELECT shardstorage FROM pg_dist_shard WHERE shardid = 400001;
+ shardstorage 
+--------------
+ f
+(1 row)
+

src/test/regress/multi_fdw_schedule

@@ -11,6 +11,7 @@
 test: multi_fdw_create_table
 test: multi_fdw_master_protocol
 test: multi_fdw_stage_data
+test: multi_fdw_storage_type
 
 # ----------
 # Parallel TPC-H tests to check our distributed execution behavior

src/test/regress/sql/multi_fdw_storage_type.sql

@@ -0,0 +1,43 @@
+--
+-- MULTI_FDW_STORAGE_TYPE
+--
+
+-- Create two tables one regular and one foreign, then check whether
+-- shardstorage is correct
+
+-- explicitly set shard id
+ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 400000;
+
+-- create regular table
+CREATE TABLE people (
+	id bigint not null,
+	firstname char(10) not null,
+	lastname char(10) not null,
+	age integer not null);
+
+-- create distributed table
+SELECT master_create_distributed_table('people', 'id', 'append');
+
+-- create worker shards
+SELECT master_create_empty_shard('people');
+
+-- check shardstorage
+SELECT shardstorage FROM pg_dist_shard WHERE shardid = 400000;
+
+-- create foreign table
+CREATE FOREIGN TABLE people_foreign (
+	id bigint not null,
+	firstname char(10) not null,
+	lastname char(10) not null,
+	age integer not null)
+SERVER file_server 
+OPTIONS (format 'text', filename '', delimiter '|', null '');
+
+-- create distributed table
+SELECT master_create_distributed_table('people_foreign', 'id', 'append');
+
+-- create worker shards
+SELECT master_create_empty_shard('people_foreign');
+
+-- check shardstorage
+SELECT shardstorage FROM pg_dist_shard WHERE shardid = 400001;