fix: sql and ns client desc result (#341)

vagetablechicken · web-flow · commit 838a14d2e76d · 2021-09-03T12:28:56.000+08:00
* fix sql/ns client desc result
diff --git a/src/catalog/schema_adapter.h b/src/catalog/schema_adapter.h
@@ -83,12 +83,12 @@ class SchemaAdapter {
         }
         for (int32_t i = 0; i < index.size(); i++) {
             const ::openmldb::common::ColumnKey& key = index.Get(i);
-            ::hybridse::type::IndexDef* index = output->Add();
-            index->set_name(key.index_name());
-            index->mutable_first_keys()->CopyFrom(key.col_name());
+            ::hybridse::type::IndexDef* index_def = output->Add();
+            index_def->set_name(key.index_name());
+            index_def->mutable_first_keys()->CopyFrom(key.col_name());
             if (key.has_ts_name() && !key.ts_name().empty()) {
-                index->set_second_key(key.ts_name());
-                index->set_ts_offset(0);
+                index_def->set_second_key(key.ts_name());
+                index_def->set_ts_offset(0);
             }
             if (key.has_ttl()) {
                 auto ttl_type = key.ttl().ttl_type();
@@ -97,14 +97,14 @@ class SchemaAdapter {
                     LOG(WARNING) << "not found " << ::openmldb::type::TTLType_Name(ttl_type);
                     return false;
                 }
-                index->set_ttl_type(it->second);
+                index_def->set_ttl_type(it->second);
                 if (ttl_type == ::openmldb::type::kAbsAndLat || ttl_type == ::openmldb::type::kAbsOrLat) {
-                    index->add_ttl(key.ttl().abs_ttl());
-                    index->add_ttl(key.ttl().lat_ttl());
+                    index_def->add_ttl(key.ttl().abs_ttl());
+                    index_def->add_ttl(key.ttl().lat_ttl());
                 } else if (ttl_type == ::openmldb::type::kAbsoluteTime) {
-                    index->add_ttl(key.ttl().abs_ttl());
+                    index_def->add_ttl(key.ttl().abs_ttl());
                 } else {
-                    index->add_ttl(key.ttl().lat_ttl());
+                    index_def->add_ttl(key.ttl().lat_ttl());
                 }
             }
         }
diff --git a/src/cmd/display.h b/src/cmd/display.h
@@ -25,6 +25,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/texttable.h"
 #include "cmd/sdk_iterator.h"
 #include "codec/flat_array.h"
 #include "codec/row_codec.h"
@@ -52,88 +53,61 @@ static void TransferString(std::vector<std::string>* vec) {
     });
 }
 
-__attribute__((unused)) static void PrintSchema(
-    const google::protobuf::RepeatedPtrField<::openmldb::common::ColumnDesc>& column_desc_field) {
-    std::vector<std::string> row;
-    row.push_back("#");
-    row.push_back("name");
-    row.push_back("type");
-    ::baidu::common::TPrinter tp(row.size(), FLAGS_max_col_display_length);
-    tp.AddRow(row);
-    uint32_t idx = 0;
-    for (const auto& column_desc : column_desc_field) {
-        row.clear();
-        row.push_back(std::to_string(idx));
-        row.push_back(column_desc.name());
-        auto iter = ::openmldb::codec::DATA_TYPE_STR_MAP.find(column_desc.data_type());
-        if (iter != ::openmldb::codec::DATA_TYPE_STR_MAP.end()) {
-            row.push_back(iter->second);
-        } else {
-            row.push_back("-");
-        }
-        tp.AddRow(row);
-        idx++;
-    }
-    tp.Print(true);
-}
-
 __attribute__((unused)) static void PrintSchema(
     const google::protobuf::RepeatedPtrField<::openmldb::common::ColumnDesc>& column_desc,
     const google::protobuf::RepeatedPtrField<::openmldb::common::ColumnDesc>& added_column_desc) {
-    std::vector<std::string> row;
-    row.push_back("#");
-    row.push_back("name");
-    row.push_back("type");
-    ::baidu::common::TPrinter tp(row.size(), FLAGS_max_col_display_length);
-    tp.AddRow(row);
-    uint32_t idx = 0;
-    for (const auto& column_desc : column_desc) {
-        row.clear();
-        row.push_back(std::to_string(idx));
-        row.push_back(column_desc.name());
-        auto iter = ::openmldb::codec::DATA_TYPE_STR_MAP.find(column_desc.data_type());
-        if (iter != ::openmldb::codec::DATA_TYPE_STR_MAP.end()) {
-            row.push_back(iter->second);
-        } else {
-            row.push_back("-");
-        }
-        tp.AddRow(row);
-        idx++;
+    ::hybridse::base::TextTable t('-', ' ', ' ');
+    t.add("#");
+    t.add("Field");
+    t.add("Type");
+    t.add("Null");
+    t.end_of_row();
+
+    for (int i = 0; i < column_desc.size(); i++) {
+        const auto& column = column_desc.Get(i);
+        t.add(std::to_string(i + 1));
+        t.add(column.name());
+        // kXXX discard k
+        t.add(DataType_Name(column.data_type()).substr(1));
+        t.add(column.not_null() ? "NO" : "YES");
+        t.end_of_row();
     }
-    for (const auto& column_desc : added_column_desc) {
-        row.clear();
-        row.push_back(std::to_string(idx));
-        row.push_back(column_desc.name());
-        auto iter = ::openmldb::codec::DATA_TYPE_STR_MAP.find(column_desc.data_type());
-        if (iter != ::openmldb::codec::DATA_TYPE_STR_MAP.end()) {
-            row.push_back(iter->second);
-        } else {
-            row.push_back("-");
-        }
-        tp.AddRow(row);
-        idx++;
+
+    for (int i = 0; i < added_column_desc.size(); i++) {
+        const auto& column = added_column_desc.Get(i);
+        t.add(std::to_string(i + 1));
+        t.add(column.name());
+        // kXXX discard k
+        t.add(DataType_Name(column.data_type()).substr(1));
+        t.add(column.not_null() ? "NO" : "YES");
+        t.end_of_row();
     }
-    tp.Print(true);
+    std::cout << t;
+}
+
+__attribute__((unused)) static void PrintSchema(
+    const google::protobuf::RepeatedPtrField<::openmldb::common::ColumnDesc>& column_desc_field) {
+    PrintSchema(column_desc_field, {});
 }
 
 __attribute__((unused)) static void PrintColumnKey(
     const google::protobuf::RepeatedPtrField<::openmldb::common::ColumnKey>& column_key_field) {
-    std::vector<std::string> row;
-    row.push_back("#");
-    row.push_back("index_name");
-    row.push_back("col_name");
-    row.push_back("ts_col");
-    row.push_back("ttl");
-    ::baidu::common::TPrinter tp(row.size(), FLAGS_max_col_display_length);
-    tp.AddRow(row);
-    uint32_t idx = 0;
-    for (const auto& column_key : column_key_field) {
+    ::hybridse::base::TextTable t('-', ' ', ' ');
+    t.add("#");
+    t.add("name");
+    t.add("keys");
+    t.add("ts");
+    t.add("ttl");
+    t.add("ttl_type");
+    t.end_of_row();
+
+    for (int i = 0; i < column_key_field.size(); i++) {
+        const auto& column_key = column_key_field.Get(i);
         if (column_key.flag() == 1) {
             continue;
         }
-        row.clear();
-        row.push_back(std::to_string(idx));
-        row.push_back(column_key.index_name());
+        t.add(std::to_string(i + 1));
+        t.add(column_key.index_name());
         std::string key;
         for (const auto& name : column_key.col_name()) {
             if (key.empty()) {
@@ -142,26 +116,29 @@ __attribute__((unused)) static void PrintColumnKey(
                 key += "|" + name;
             }
         }
-        if (key.empty()) {
-            key = column_key.index_name();
-        }
-        row.push_back(key);
+        t.add((key.empty() ? column_key.index_name() : key));
+
         if (!column_key.ts_name().empty()) {
-            row.push_back(column_key.ts_name());
+            t.add(column_key.ts_name());
         } else {
-            row.push_back("-");
+            t.add("-");
         }
+
         if (column_key.has_ttl()) {
-            ::openmldb::storage::TTLSt cur_ttl_st(column_key.ttl());
-            cur_ttl_st.abs_ttl = cur_ttl_st.abs_ttl / (60 * 1000);
-            row.push_back(cur_ttl_st.ToString());
+            std::ostringstream oss;
+            auto& ttl = column_key.ttl();
+            // TODO(hw): it's better to construct from common::TTLSt, but the abs will be measured in minutes. Needs fix
+            storage::TTLSt ttl_st(ttl.abs_ttl(), ttl.lat_ttl(), storage::TTLSt::ConvertTTLType(ttl.ttl_type()));
+            t.add(ttl_st.ToString());
+            t.add(TTLType_Name(ttl_st.GetProtoTTLType()));
         } else {
-            row.push_back("-");
+            t.add("-");  // ttl
+            t.add("-");  // ttl_type
         }
-        tp.AddRow(row);
-        idx++;
+
+        t.end_of_row();
     }
-    tp.Print(true);
+    std::cout << t;
 }
 
 __attribute__((unused)) static void ShowTableRows(bool is_compress, ::openmldb::codec::SDKCodec* codec,
diff --git a/src/cmd/sql_cmd.h b/src/cmd/sql_cmd.h
@@ -166,7 +166,7 @@ void PrintTableIndex(std::ostream &stream, const ::hybridse::vm::IndexList &inde
         t.add(std::to_string(i + 1));
         t.add(index.name());
         t.add(index.first_keys(0));
-        std::string ts_name = index.second_key();
+        const std::string &ts_name = index.second_key();
         if (ts_name.empty()) {
             t.add("-");
         } else {
@@ -228,7 +228,7 @@ void PrintItems(std::ostream &stream, const std::string &head, const std::vector
     ::hybridse::base::TextTable t('-', ' ', ' ');
     t.add(head);
     t.end_of_row();
-    for (auto item : items) {
+    for (const auto &item : items) {
         t.add(item);
         t.end_of_row();
     }
@@ -251,7 +251,7 @@ void PrintItems(const std::vector<std::pair<std::string, std::string>> &items, s
     t.add("DB");
     t.add("SP");
     t.end_of_row();
-    for (auto item : items) {
+    for (const auto &item : items) {
         t.add(item.first);
         t.add(item.second);
         t.end_of_row();
@@ -267,7 +267,7 @@ void PrintItems(const std::vector<std::pair<std::string, std::string>> &items, s
 
 void PrintProcedureSchema(const std::string &head, const ::hybridse::sdk::Schema &sdk_schema, std::ostream &stream) {
     try {
-        const ::hybridse::sdk::SchemaImpl &schema_impl = dynamic_cast<const ::hybridse::sdk::SchemaImpl &>(sdk_schema);
+        const auto &schema_impl = dynamic_cast<const ::hybridse::sdk::SchemaImpl &>(sdk_schema);
         auto &schema = schema_impl.GetSchema();
         if (schema.empty()) {
             stream << "Empty set" << std::endl;
@@ -283,15 +283,15 @@ void PrintProcedureSchema(const std::string &head, const ::hybridse::sdk::Schema
         t.end_of_row();
 
         for (uint32_t i = 0; i < items_size; i++) {
-            auto column = schema.Get(i);
+            const auto &column = schema.Get(i);
             t.add(std::to_string(i + 1));
             t.add(column.name());
             t.add(::hybridse::type::Type_Name(column.type()));
             t.add(column.is_constant() ? "YES" : "NO");
             t.end_of_row();
         }
         stream << t << std::endl;
-    } catch (std::bad_cast&) {
+    } catch (std::bad_cast &) {
         return;
     }
 }
@@ -346,12 +346,9 @@ void HandleCmd(const hybridse::node::CmdPlanNode *cmd_node) {
                 std::cerr << "table " << cmd_node->GetArgs()[0] << " does not exist" << std::endl;
                 return;
             }
-            ::hybridse::vm::Schema output_schema;
-            ::openmldb::catalog::SchemaAdapter::ConvertSchema(table->column_desc(), &output_schema);
-            PrintTableSchema(std::cout, output_schema);
-            ::hybridse::vm::IndexList index_list;
-            ::openmldb::catalog::SchemaAdapter::ConvertIndex(table->column_key(), &index_list);
-            PrintTableIndex(std::cout, index_list);
+
+            PrintSchema(table->column_desc());
+            PrintColumnKey(table->column_key());
             break;
         }
 
@@ -461,8 +458,7 @@ void HandleCmd(const hybridse::node::CmdPlanNode *cmd_node) {
             std::string error;
             std::vector<std::shared_ptr<hybridse::sdk::ProcedureInfo>> sp_infos = cs->GetProcedureInfo(&error);
             std::vector<std::pair<std::string, std::string>> pairs;
-            for (uint32_t i = 0; i < sp_infos.size(); i++) {
-                auto &sp_info = sp_infos.at(i);
+            for (auto &sp_info : sp_infos) {
                 pairs.push_back(std::make_pair(sp_info->GetDbName(), sp_info->GetSpName()));
             }
             PrintItems(pairs, std::cout);
@@ -542,7 +538,7 @@ void HandleSQL(const std::string &sql) {
     hybridse::node::PlanNode *node = plan_trees[0];
     switch (node->GetType()) {
         case hybridse::node::kPlanTypeCmd: {
-            hybridse::node::CmdPlanNode *cmd = dynamic_cast<hybridse::node::CmdPlanNode *>(node);
+            auto *cmd = dynamic_cast<hybridse::node::CmdPlanNode *>(node);
             HandleCmd(cmd);
             return;
         }
@@ -579,8 +575,7 @@ void HandleSQL(const std::string &sql) {
                 std::cout << "please use database first" << std::endl;
                 return;
             }
-            hybridse::node::CreateIndexPlanNode *create_index_node =
-                dynamic_cast<hybridse::node::CreateIndexPlanNode *>(node);
+            auto *create_index_node = dynamic_cast<hybridse::node::CreateIndexPlanNode *>(node);
             HandleCreateIndex(create_index_node->create_index_node_);
             return;
         }
diff --git a/src/storage/schema.h b/src/storage/schema.h
@@ -28,17 +28,18 @@
 #include "proto/tablet.pb.h"
 #include "proto/type.pb.h"
 
-namespace openmldb {
-namespace storage {
+namespace openmldb::storage {
 
 static constexpr uint32_t MAX_INDEX_NUM = 200;
 
 enum TTLType { kAbsoluteTime = 1, kRelativeTime = 2, kLatestTime = 3, kAbsAndLat = 4, kAbsOrLat = 5 };
 
+// ttl unit: millisecond
 struct TTLSt {
     TTLSt() : abs_ttl(0), lat_ttl(0), ttl_type(::openmldb::storage::TTLType::kAbsoluteTime) {}
     TTLSt(uint64_t abs, uint64_t lat, ::openmldb::storage::TTLType type) : abs_ttl(abs), lat_ttl(lat), ttl_type(type) {}
 
+    // TODO(hw): needs fix, common::TTLSt::abs_ttl should be ms
     explicit TTLSt(const ::openmldb::common::TTLSt& ttl) : abs_ttl(ttl.abs_ttl() * 60 * 1000), lat_ttl(ttl.lat_ttl()) {
         ttl_type = ConvertTTLType(ttl.ttl_type());
     }
@@ -76,13 +77,13 @@ struct TTLSt {
     bool NeedGc() const {
         switch (ttl_type) {
             case TTLType::kAbsoluteTime:
-                return abs_ttl == 0 ? false : true;
+                return abs_ttl != 0;
             case TTLType::kLatestTime:
-                return lat_ttl == 0 ? false : true;
+                return lat_ttl != 0;
             case TTLType::kAbsAndLat:
-                return abs_ttl == 0 || lat_ttl == 0 ? false : true;
+                return !(abs_ttl == 0 || lat_ttl == 0);
             case TTLType::kAbsOrLat:
-                return abs_ttl == 0 && lat_ttl == 0 ? false : true;
+                return !(abs_ttl == 0 && lat_ttl == 0);
             default:
                 return true;
         }
@@ -119,7 +120,7 @@ struct TTLSt {
     std::string ToString() const {
         switch (ttl_type) {
             case TTLType::kAbsoluteTime:
-                return std::to_string(abs_ttl) + "min";
+                return std::to_string(abs_ttl / (60 * 1000)) + "min";
             case TTLType::kLatestTime:
                 return std::to_string(lat_ttl);
             case TTLType::kAbsAndLat:
@@ -319,7 +320,7 @@ class TableSt {
         if (pid < partitions->size()) {
             return partitions->at(pid);
         }
-        return PartitionSt();
+        return {};
     }
 
     bool SetPartition(const PartitionSt& partition_st);
@@ -344,5 +345,4 @@ class TableSt {
     std::shared_ptr<std::vector<PartitionSt>> partitions_;
 };
 
-}  // namespace storage
-}  // namespace openmldb
+}  // namespace openmldb::storage
diff --git a/src/storage/schema_test.cc b/src/storage/schema_test.cc
