From 323f1151e05b21c337a7a686b12292fb4a9efa55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Y=C3=BCcesoy?= Date: Thu, 19 May 2016 15:17:59 +0300 Subject: [PATCH] Fix wrong storage type for foreign tables Fixes #496 Previously we do not check whether table is foreign or not while creating empty shards, and set storage type to 't'(Standard table) or 'c'(Columnar table). Now if the table is foreign table(but not CStore foreign table) we set storage type to 'f'(Foreign table). If it is CStore foreign table, we set its storage type to 'c', i.e. columnar table have priority over foreign table. Please note that 'c' is only used for CStore tables not for other possible columnar stores at the moment. Possible improvement could be checking for other columnar stores, though I am not sure if there is a way to check it for all other columnar stores. --- .../distributed/master/master_create_shards.c | 10 ++- .../master/master_stage_protocol.c | 20 +++++- .../expected/multi_fdw_storage_type.out | 63 +++++++++++++++++++ src/test/regress/multi_fdw_schedule | 1 + .../regress/sql/multi_fdw_storage_type.sql | 43 +++++++++++++ 5 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 src/test/regress/expected/multi_fdw_storage_type.out create mode 100644 src/test/regress/sql/multi_fdw_storage_type.sql diff --git a/src/backend/distributed/master/master_create_shards.c b/src/backend/distributed/master/master_create_shards.c index 7dd8008cf..f91093d05 100644 --- a/src/backend/distributed/master/master_create_shards.c +++ b/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 { diff --git a/src/backend/distributed/master/master_stage_protocol.c b/src/backend/distributed/master/master_stage_protocol.c index 8b11d3733..cd930a369 100644 --- a/src/backend/distributed/master/master_stage_protocol.c +++ b/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); diff --git a/src/test/regress/expected/multi_fdw_storage_type.out b/src/test/regress/expected/multi_fdw_storage_type.out new file mode 100644 index 000000000..06fb95e3d --- /dev/null +++ b/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) + diff --git a/src/test/regress/multi_fdw_schedule b/src/test/regress/multi_fdw_schedule index 9165da4ac..e9c861022 100644 --- a/src/test/regress/multi_fdw_schedule +++ b/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 diff --git a/src/test/regress/sql/multi_fdw_storage_type.sql b/src/test/regress/sql/multi_fdw_storage_type.sql new file mode 100644 index 000000000..be4e99fcd --- /dev/null +++ b/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;