Skip to content

Commit

Permalink
fix: create failed if partitionnum not equal distribution list size
Browse files Browse the repository at this point in the history
  • Loading branch information
@dl239
dl239 committed Sep 5, 2022
1 parent 3b3d206 commit 237f506
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 22 deletions.
27 changes: 20 additions & 7 deletions src/sdk/node_adapter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n
// different default value for cluster and standalone mode
int replica_num = 1;
int partition_num = 1;
bool setted_replica_num = false;
bool setted_partition_num = false;
if (is_cluster_mode) {
replica_num = default_replica_num;
partition_num = FLAGS_partition_num;
Expand All @@ -59,11 +61,13 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n
switch (table_option->GetType()) {
case hybridse::node::kReplicaNum: {
replica_num = dynamic_cast<hybridse::node::ReplicaNumNode *>(table_option)->GetReplicaNum();
setted_replica_num = true;
break;
}
case hybridse::node::kPartitionNum: {
partition_num =
dynamic_cast<hybridse::node::PartitionNumNode*>(table_option)->GetPartitionNum();
setted_partition_num = true;
break;
}
case hybridse::node::kStorageMode: {
Expand All @@ -82,11 +86,11 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n
}
}
}
if (replica_num == 0) {
if (replica_num <= 0) {
*status = {hybridse::common::kUnsupportSql, "replicanum should be great than 0"};
return false;
}
if (partition_num == 0) {
if (partition_num <= 0) {
*status = {hybridse::common::kUnsupportSql, "partitionnum should be great than 0"};
return false;
}
Expand Down Expand Up @@ -202,16 +206,15 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n
}
}
if (!distribution_list.empty()) {
table->set_partition_num(distribution_list.size());
int cur_replica_num = 0;
for (size_t idx = 0; idx < distribution_list.size(); idx++) {
auto table_partition = table->add_table_partition();
table_partition->set_pid(idx);
auto partition_mata_nodes = dynamic_cast<hybridse::node::SqlNodeList*>(distribution_list.at(idx));
if (idx == 0) {
table->set_replica_num(partition_mata_nodes->GetSize());
} else if (static_cast<int>(table->replica_num()) != partition_mata_nodes->GetSize()) {
status->msg = "CREATE common: replica num is inconsistency";
status->code = hybridse::common::kUnsupportSql;
cur_replica_num = partition_mata_nodes->GetSize();
} else if (cur_replica_num != partition_mata_nodes->GetSize()) {
*status = {hybridse::common::kUnsupportSql, "replica num is inconsistency"};
return false;
}
std::set<std::string> endpoint_set;
Expand Down Expand Up @@ -249,6 +252,16 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n
}
}
}
if (setted_partition_num && table->partition_num() != distribution_list.size()) {
*status = {hybridse::common::kUnsupportSql, "distribution_list size and partition_num is not match"};
return false;
}
table->set_partition_num(distribution_list.size());
if (setted_replica_num && static_cast<int>(table->replica_num()) != cur_replica_num) {
*status = {hybridse::common::kUnsupportSql, "replica in distribution_list and replica_num is not match"};
return false;
}
table->set_replica_num(cur_replica_num);
}
return true;
}
Expand Down
33 changes: 18 additions & 15 deletions src/sdk/node_adapter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void CheckTablePartition(const ::openmldb::nameserver::TableInfo& table_info,

TEST_P(NodeAdapterTest, TransformToTableInfo) {
std::string base_sql = "CREATE TABLE t1 (col0 STRING, col1 int, std_time TIMESTAMP, INDEX(KEY=col1, TS=std_time)) "
"OPTIONS (DISTRIBUTION = ";
"OPTIONS (";
auto& c = GetParam();
std::string sql = base_sql + c.distribution + ");";
hybridse::node::NodeManager node_manager;
Expand All @@ -94,25 +94,28 @@ TEST_P(NodeAdapterTest, TransformToTableInfo) {
}
}


static std::vector<TestInfo> cases = {
{ "[('127.0.0.1:6527')]", true, true, {{"127.0.0.1:6527"}} },
{ "[('127.0.0.1:6527', [])]", true, true, {{"127.0.0.1:6527"}} },
{ "[('127.0.0.1:6527', ['127.0.0.1:6528'])]", true, true, {{"127.0.0.1:6527", "127.0.0.1:6528"}}},
{ "[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6529'])]", true, true,
{ "DISTRIBUTION=[('127.0.0.1:6527')]", true, true, {{"127.0.0.1:6527"}} },
{ "DISTRIBUTION=[('127.0.0.1:6527', [])]", true, true, {{"127.0.0.1:6527"}} },
{ "DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6528'])]", true, true, {{"127.0.0.1:6527", "127.0.0.1:6528"}}},
{ "DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6529'])]", true, true,
{{"127.0.0.1:6527", "127.0.0.1:6528", "127.0.0.1:6529"}} },
{ "[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6529']), "
{ "DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6529']), "
"('127.0.0.1:6528', ['127.0.0.1:6527','127.0.0.1:6529'])]", true, true,
{{"127.0.0.1:6527", "127.0.0.1:6528", "127.0.0.1:6529"},
{"127.0.0.1:6528", "127.0.0.1:6527", "127.0.0.1:6529"}} },
{ "[('127.0.0.1:6527', ['127.0.0.1:6527'])]", true, false, {} },
{ "[('127.0.0.1:6527', ['127.0.0.1:6527')]", false, false, {} },
{ "[()]", false, false, {{}} },
{ "[]", false, false, {{}} },
{ "['127.0.0.1:6527']", true, true, {{"127.0.0.1:6527"}} },
{ "[('127.0.0.1:6527', '127.0.0.1:6527')]", false, false, {} },
{ "['127.0.0.1:6527', '127.0.0.1:6528']", true, true, {{"127.0.0.1:6527"}, {"127.0.0.1:6528"}} },
{ "[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6528'])]", true, false, {} },
{ "DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6527'])]", true, false, {} },
{ "DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6527')]", false, false, {} },
{ "DISTRIBUTION=[()]", false, false, {{}} },
{ "DISTRIBUTION=[]", false, false, {{}} },
{ "DISTRIBUTION=['127.0.0.1:6527']", true, true, {{"127.0.0.1:6527"}} },
{ "DISTRIBUTION=[('127.0.0.1:6527', '127.0.0.1:6527')]", false, false, {} },
{ "DISTRIBUTION=['127.0.0.1:6527', '127.0.0.1:6528']", true, true, {{"127.0.0.1:6527"}, {"127.0.0.1:6528"}} },
{ "DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6528'])]", true, false, {} },
{ "REPLICANUM=2, DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6529'])]", true, false, {} },
{ "PARTITIONNUM=2, DISTRIBUTION=[('127.0.0.1:6527', ['127.0.0.1:6528','127.0.0.1:6529'])]", true, false, {} },
{ "REPLICANUM=2, PARTITIONNUM=0", true, false, {} },
{ "REPLICANUM=0, PARTITIONNUM=8", true, false, {} },
};

INSTANTIATE_TEST_SUITE_P(NodeAdapter, NodeAdapterTest, testing::ValuesIn(cases));
Expand Down

0 comments on commit 237f506

Please sign in to comment.