refactor(iterator): simplify virtual function list

hybridse/examples/toydb/src/tablet/tablet_catalog.cc

@@ -19,7 +19,6 @@
 #include <memory>
 #include <string>
 #include <utility>
-#include "codec/list_iterator_codec.h"
 #include "glog/logging.h"
 #include "storage/table_iterator.h"
 
@@ -99,13 +98,6 @@ bool TabletTableHandler::Init() {
     return true;
 }
 
-std::unique_ptr<RowIterator> TabletTableHandler::GetIterator() {
-    std::unique_ptr<storage::FullTableIterator> it(
-        new storage::FullTableIterator(table_->GetSegments(),
-                                       table_->GetSegCnt(), table_));
-    return std::move(it);
-}
-
 std::unique_ptr<WindowIterator> TabletTableHandler::GetWindowIterator(
     const std::string& idx_name) {
     auto iter = index_hint_.find(idx_name);
@@ -136,22 +128,6 @@ RowIterator* TabletTableHandler::GetRawIterator() {
     return new storage::FullTableIterator(table_->GetSegments(),
                                           table_->GetSegCnt(), table_);
 }
-const uint64_t TabletTableHandler::GetCount() {
-    auto iter = GetIterator();
-    uint64_t cnt = 0;
-    while (iter->Valid()) {
-        iter->Next();
-        cnt++;
-    }
-    return cnt;
-}
-Row TabletTableHandler::At(uint64_t pos) {
-    auto iter = GetIterator();
-    while (pos-- > 0 && iter->Valid()) {
-        iter->Next();
-    }
-    return iter->Valid() ? iter->GetValue() : Row();
-}
 
 TabletCatalog::TabletCatalog() : tables_(), db_() {}
 
@@ -249,22 +225,6 @@ std::unique_ptr<WindowIterator> TabletSegmentHandler::GetWindowIterator(
     const std::string& idx_name) {
     return std::unique_ptr<WindowIterator>();
 }
-const uint64_t TabletSegmentHandler::GetCount() {
-    auto iter = GetIterator();
-    uint64_t cnt = 0;
-    while (iter->Valid()) {
-        cnt++;
-        iter->Next();
-    }
-    return cnt;
-}
-Row TabletSegmentHandler::At(uint64_t pos) {
-    auto iter = GetIterator();
-    while (pos-- > 0 && iter->Valid()) {
-        iter->Next();
-    }
-    return iter->Valid() ? iter->GetValue() : Row();
-}
 
 const uint64_t TabletPartitionHandler::GetCount() {
     auto iter = GetWindowIterator();
@@ -275,5 +235,6 @@ const uint64_t TabletPartitionHandler::GetCount() {
     }
     return cnt;
 }
+
 }  // namespace tablet
 }  // namespace hybridse

hybridse/examples/toydb/src/tablet/tablet_catalog.h

@@ -21,7 +21,6 @@
 #include <memory>
 #include <string>
 #include <vector>
-#include "base/spin_lock.h"
 #include "storage/table_impl.h"
 #include "vm/catalog.h"
 
@@ -68,8 +67,6 @@
     std::unique_ptr<vm::RowIterator> GetIterator() override;
     RowIterator* GetRawIterator() override;
     std::unique_ptr<codec::WindowIterator> GetWindowIterator(const std::string& idx_name) override;
-    const uint64_t GetCount() override;
-    Row At(uint64_t pos) override;
     const std::string GetHandlerTypeName() override {
         return "TabletSegmentHandler";
     }
@@ -79,7 +76,7 @@
     std::string key_;
 };
 
-class TabletPartitionHandler
+class TabletPartitionHandler final
     : public PartitionHandler,
       public std::enable_shared_from_this<PartitionHandler> {
  public:
@@ -91,6 +88,8 @@
 
     ~TabletPartitionHandler() {}
 
+    RowIterator* GetRawIterator() override { return nullptr; }
+
     const OrderType GetOrderType() const override { return OrderType::kDescOrder; }
 
     const vm::Schema* GetSchema() override { return table_handler_->GetSchema(); }
@@ -104,6 +103,7 @@
     std::unique_ptr<codec::WindowIterator> GetWindowIterator() override {
         return table_handler_->GetWindowIterator(index_name_);
     }
+
     const uint64_t GetCount() override;
 
     std::shared_ptr<TableHandler> GetSegment(const std::string& key) override {
@@ -119,7 +119,7 @@
     vm::IndexHint index_hint_;
 };
 
-class TabletTableHandler
+class TabletTableHandler final
     : public vm::TableHandler,
       public std::enable_shared_from_this<vm::TableHandler> {
  public:
@@ -135,28 +135,23 @@
 
     bool Init();
 
-    inline const vm::Schema* GetSchema() { return &schema_; }
+    const vm::Schema* GetSchema() override { return &schema_; }
 
-    inline const std::string& GetName() { return name_; }
+    const std::string& GetName() override { return name_; }
 
-    inline const std::string& GetDatabase() { return db_; }
+    const std::string& GetDatabase() override { return db_; }
 
-    inline const vm::Types& GetTypes() { return types_; }
+    const vm::Types& GetTypes() override { return types_; }
 
-    inline const vm::IndexHint& GetIndex() { return index_hint_; }
+    const vm::IndexHint& GetIndex() override { return index_hint_; }
 
     const Row Get(int32_t pos);
 
-    inline std::shared_ptr<storage::Table> GetTable() { return table_; }
-    std::unique_ptr<RowIterator> GetIterator();
+    std::shared_ptr<storage::Table> GetTable() { return table_; }
     RowIterator* GetRawIterator() override;
-    std::unique_ptr<codec::WindowIterator> GetWindowIterator(
-        const std::string& idx_name);
-    virtual const uint64_t GetCount();
-    Row At(uint64_t pos) override;
+    std::unique_ptr<codec::WindowIterator> GetWindowIterator(const std::string& idx_name) override;
 
-    virtual std::shared_ptr<PartitionHandler> GetPartition(
-        const std::string& index_name) {
+    std::shared_ptr<PartitionHandler> GetPartition(const std::string& index_name) override {
         if (index_hint_.find(index_name) == index_hint_.cend()) {
             LOG(WARNING)
                 << "fail to get partition for tablet table handler, index name "
@@ -169,12 +164,12 @@
     const std::string GetHandlerTypeName() override {
         return "TabletTableHandler";
     }
-    virtual std::shared_ptr<hybridse::vm::Tablet> GetTablet(
-        const std::string& index_name, const std::string& pk) {
+    std::shared_ptr<hybridse::vm::Tablet> GetTablet(const std::string& index_name,
+                                                            const std::string& pk) override {
         return tablet_;
     }
-    virtual std::shared_ptr<hybridse::vm::Tablet> GetTablet(
-        const std::string& index_name, const std::vector<std::string>& pks) {
+    std::shared_ptr<hybridse::vm::Tablet> GetTablet(const std::string& index_name,
+                                                    const std::vector<std::string>& pks) override {
         return tablet_;
     }
 

hybridse/examples/toydb/src/testing/toydb_engine_test_base.cc

@@ -15,8 +15,9 @@
  */
 
 #include "testing/toydb_engine_test_base.h"
+
+#include "absl/strings/str_join.h"
 #include "gtest/gtest.h"
-#include "gtest/internal/gtest-param-util.h"
 
 using namespace llvm;       // NOLINT (build/namespaces)
 using namespace llvm::orc;  // NOLINT (build/namespaces)
@@ -141,18 +142,12 @@ std::shared_ptr<tablet::TabletCatalog> BuildOnePkTableStorage(
     }
     return catalog;
 }
-void BatchRequestEngineCheckWithCommonColumnIndices(
-    const SqlCase& sql_case, const EngineOptions options,
-    const std::set<size_t>& common_column_indices) {
-    std::ostringstream oss;
-    for (size_t index : common_column_indices) {
-        oss << index << ",";
-    }
-    LOG(INFO) << "BatchRequestEngineCheckWithCommonColumnIndices: "
-                 "common_column_indices = ["
-              << oss.str() << "]";
-    ToydbBatchRequestEngineTestRunner engine_test(sql_case, options,
-                                                  common_column_indices);
+// Run check with common column index info
+void BatchRequestEngineCheckWithCommonColumnIndices(const SqlCase& sql_case, const EngineOptions options,
+                                                    const std::set<size_t>& common_column_indices) {
+    LOG(INFO) << "BatchRequestEngineCheckWithCommonColumnIndices: common_column_indices = ["
+              << absl::StrJoin(common_column_indices, ",") << "]";
+    ToydbBatchRequestEngineTestRunner engine_test(sql_case, options, common_column_indices);
     engine_test.RunCheck();
 }
 

hybridse/include/codec/row.h

@@ -54,7 +54,7 @@ class Row {
 
     inline int32_t size() const { return slice_.size(); }
     inline int32_t size(int32_t pos) const {
-        return 0 == pos ? slice_.size() : slices_[pos - 1].size();
+        return 0 == pos ? slice_.size() : slices_.at(pos - 1).size();
     }
 
     // Return true if the length of the referenced data is zero

hybridse/include/codec/row_iterator.h

@@ -71,7 +71,14 @@
     virtual bool Valid() = 0;
     /// Return the RowIterator of current segment
     /// of dataset if Valid() return `true`.
-    virtual std::unique_ptr<RowIterator> GetValue() = 0;
+    virtual std::unique_ptr<RowIterator> GetValue() {
+        auto p = GetRawValue();
+        if (!p) {
+            return nullptr;
+        }
+
+        return std::unique_ptr<RowIterator>(p);
+    }
     /// Return the RowIterator of current segment
     /// of dataset if Valid() return `true`.
     virtual RowIterator *GetRawValue() = 0;

hybridse/include/codec/row_list.h

@@ -65,7 +65,13 @@ class ListV {
     ListV() {}
     virtual ~ListV() {}
     /// \brief Return the const iterator
-    virtual std::unique_ptr<ConstIterator<uint64_t, V>> GetIterator() = 0;
+    virtual std::unique_ptr<ConstIterator<uint64_t, V>> GetIterator() {
+        auto raw = GetRawIterator();
+        if (raw == nullptr) {
+            return {};
+        }
+        return std::unique_ptr<ConstIterator<uint64_t, V>>(raw);
+    }
 
     /// \brief Return the const iterator raw pointer
     virtual ConstIterator<uint64_t, V> *GetRawIterator() = 0;
@@ -76,7 +82,7 @@ class ListV {
     virtual const uint64_t GetCount() {
         auto iter = GetIterator();
         uint64_t cnt = 0;
-        while (iter->Valid()) {
+        while (iter && iter->Valid()) {
             iter->Next();
             cnt++;
         }

hybridse/include/vm/catalog.h

@@ -217,15 +217,15 @@
     virtual ~TableHandler() {}
 
     /// Return table column Types information.
+    /// TODO: rm it, never used
     virtual const Types& GetTypes() = 0;
 
     /// Return the index information
     virtual const IndexHint& GetIndex() = 0;
 
     /// Return WindowIterator
     /// so that user can use it to iterate datasets segment by segment.
-    virtual std::unique_ptr<WindowIterator> GetWindowIterator(
-        const std::string& idx_name) = 0;
+    virtual std::unique_ptr<WindowIterator> GetWindowIterator(const std::string& idx_name) { return nullptr; }
 
     /// Return the HandlerType of the dataset.
     /// Return HandlerType::kTableHandler by default
@@ -254,8 +254,7 @@
 
     /// Return Tablet binding to specify index and keys.
     /// Return `null` by default.
-    virtual std::shared_ptr<Tablet> GetTablet(
-        const std::string& index_name, const std::vector<std::string>& pks) {
+    virtual std::shared_ptr<Tablet> GetTablet(const std::string& index_name, const std::vector<std::string>& pks) {
         return std::shared_ptr<Tablet>();
     }
 };
@@ -286,27 +285,19 @@
     /// Return empty column Types.
     const Types& GetTypes() override { return types_; }
     /// Return empty table Schema.
-    inline const Schema* GetSchema() override { return schema_; }
+    const Schema* GetSchema() override { return schema_; }
     /// Return empty table name
-    inline const std::string& GetName() override { return table_name_; }
+    const std::string& GetName() override { return table_name_; }
     /// Return empty indexn information
-    inline const IndexHint& GetIndex() override { return index_hint_; }
+    const IndexHint& GetIndex() override { return index_hint_; }
     /// Return name of database
-    inline const std::string& GetDatabase() override { return db_; }
+    const std::string& GetDatabase() override { return db_; }
 
     /// Return null iterator
-    std::unique_ptr<RowIterator> GetIterator() {
-        return std::unique_ptr<RowIterator>();
-    }
-    /// Return null iterator
-    RowIterator* GetRawIterator() { return nullptr; }
-    /// Return null window iterator
-    std::unique_ptr<WindowIterator> GetWindowIterator(
-        const std::string& idx_name) {
-        return std::unique_ptr<WindowIterator>();
-    }
+    RowIterator* GetRawIterator() override { return nullptr; }
+
     /// Return empty row
-    virtual Row At(uint64_t pos) { return Row(); }
+    Row At(uint64_t pos) override { return Row(); }
 
     /// Return 0
     const uint64_t GetCount() override { return 0; }
@@ -317,7 +308,7 @@
     }
 
     /// Return status
-    virtual base::Status GetStatus() { return status_; }
+    base::Status GetStatus() override { return status_; }
 
  protected:
     base::Status status_;
@@ -340,16 +331,11 @@
     PartitionHandler() : TableHandler() {}
     ~PartitionHandler() {}
 
-    /// Return the iterator of row iterator.
-    /// Return null by default
-    virtual std::unique_ptr<RowIterator> GetIterator() {
-        return std::unique_ptr<RowIterator>();
-    }
-    /// Return the iterator of row iterator
-    /// Return null by default
+    // Return the iterator of row iterator
+    // Return null by default
     RowIterator* GetRawIterator() { return nullptr; }
-    virtual std::unique_ptr<WindowIterator> GetWindowIterator(
-        const std::string& idx_name) {
+
+    std::unique_ptr<WindowIterator> GetWindowIterator(const std::string& idx_name) override {
         return std::unique_ptr<WindowIterator>();
     }
 
@@ -361,18 +347,15 @@
     const HandlerType GetHandlerType() override { return kPartitionHandler; }
 
     /// Return empty row, cause partition dataset does not support At operation.
-    virtual Row At(uint64_t pos) { return Row(); }
+    // virtual Row At(uint64_t pos) { return Row(); }
 
     /// Return Return table handler of specific segment binding to given key.
     /// Return `null` by default.
-    virtual std::shared_ptr<TableHandler> GetSegment(const std::string& key) {
-        return std::shared_ptr<TableHandler>();
-    }
+    virtual std::shared_ptr<TableHandler> GetSegment(const std::string& key) = 0;
 
     /// Return a sequence of table handles of specify segments binding to given
     /// keys set.
-    virtual std::vector<std::shared_ptr<TableHandler>> GetSegments(
-        const std::vector<std::string>& keys) {
+    virtual std::vector<std::shared_ptr<TableHandler>> GetSegments(const std::vector<std::string>& keys) {
         std::vector<std::shared_ptr<TableHandler>> segments;
         for (auto key : keys) {
             segments.push_back(GetSegment(key));
@@ -383,9 +366,6 @@
     const std::string GetHandlerTypeName() override {
         return "PartitionHandler";
     }
-    /// Return order type of the dataset,
-    /// and return kNoneOrder by default.
-    const OrderType GetOrderType() const { return kNoneOrder; }
 };
 
 /// \brief A wrapper of table handler which is used as a asynchronous row