From 237f5068028ed957c65d9db7ebfd94d3d9cf501f Mon Sep 17 00:00:00 2001 From: dl239 Date: Mon, 5 Sep 2022 16:55:27 +0800 Subject: [PATCH] fix: create failed if partitionnum not equal distribution list size --- src/sdk/node_adapter.cc | 27 ++++++++++++++++++++------- src/sdk/node_adapter_test.cc | 33 ++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/sdk/node_adapter.cc b/src/sdk/node_adapter.cc index b45cd53c6e0..f37c4470880 100644 --- a/src/sdk/node_adapter.cc +++ b/src/sdk/node_adapter.cc @@ -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; @@ -59,11 +61,13 @@ bool NodeAdapter::TransformToTableDef(::hybridse::node::CreatePlanNode* create_n switch (table_option->GetType()) { case hybridse::node::kReplicaNum: { replica_num = dynamic_cast(table_option)->GetReplicaNum(); + setted_replica_num = true; break; } case hybridse::node::kPartitionNum: { partition_num = dynamic_cast(table_option)->GetPartitionNum(); + setted_partition_num = true; break; } case hybridse::node::kStorageMode: { @@ -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; } @@ -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(distribution_list.at(idx)); if (idx == 0) { - table->set_replica_num(partition_mata_nodes->GetSize()); - } else if (static_cast(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 endpoint_set; @@ -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(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; } diff --git a/src/sdk/node_adapter_test.cc b/src/sdk/node_adapter_test.cc index 89c8931572c..ded0277d0c2 100644 --- a/src/sdk/node_adapter_test.cc +++ b/src/sdk/node_adapter_test.cc @@ -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; @@ -94,25 +94,28 @@ TEST_P(NodeAdapterTest, TransformToTableInfo) { } } - static std::vector 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));