kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/2] kudu git commit: [KuduScanBatch::const_iterator] a minor clean-up
Date Wed, 03 Aug 2016 22:59:32 GMT
[KuduScanBatch::const_iterator] a minor clean-up

More 'standardized' signature for the prefix increment operator.
Added postfix increment operator.  Changed the behavior of the
equality/non-equality operators: along with current row index,
also check whether iterators are built upon the same batch.
Added units tests to cover the modified code.

Change-Id: I3eeba46e5bc45c9dd7b96abad60173381d6c3e39
Reviewed-on: http://gerrit.cloudera.org:8080/3834
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/7ca72739
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/7ca72739
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/7ca72739

Branch: refs/heads/master
Commit: 7ca727396408b72e465ad2920beb85c6f8b01234
Parents: 7766e92
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Tue Aug 2 14:57:00 2016 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Aug 3 22:58:41 2016 +0000

----------------------------------------------------------------------
 src/kudu/client/client-test.cc | 97 +++++++++++++++++++++++++++++++++++++
 src/kudu/client/scan_batch.h   | 33 ++++++++++---
 2 files changed, 123 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/7ca72739/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index e50a1c8..dd242d6 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -3067,5 +3067,102 @@ TEST_F(ClientTest, TestNoDefaultPartitioning) {
     ASSERT_STR_CONTAINS(s.ToString(), "Table partitioning must be specified");
 }
 
+TEST_F(ClientTest, TestBatchScanConstIterator) {
+  // Check for iterator behavior for an empty batch.
+  {
+    KuduScanBatch empty_batch;
+    ASSERT_EQ(0, empty_batch.NumRows());
+    ASSERT_TRUE(empty_batch.begin() == empty_batch.end());
+  }
+
+  {
+    // Insert a few rows
+    const int ROW_NUM = 2;
+    ASSERT_NO_FATAL_FAILURE(InsertTestRows(client_table_.get(), ROW_NUM));
+
+    KuduScanner scanner(client_table_.get());
+    ASSERT_OK(scanner.Open());
+
+    // Do a scan
+    KuduScanBatch batch;
+    ASSERT_TRUE(scanner.HasMoreRows());
+    ASSERT_OK(scanner.NextBatch(&batch));
+    const int ref_count(batch.NumRows());
+    ASSERT_EQ(ROW_NUM, ref_count);
+
+    {
+      KuduScanBatch::const_iterator it_next = batch.begin();
+      std::advance(it_next, 1);
+
+      KuduScanBatch::const_iterator it = batch.begin();
+      ASSERT_TRUE(++it == it_next);
+      ASSERT_TRUE(it == it_next);
+    }
+
+    {
+      KuduScanBatch::const_iterator it_end = batch.begin();
+      std::advance(it_end, ROW_NUM);
+      ASSERT_TRUE(batch.end() == it_end);
+    }
+
+    {
+      KuduScanBatch::const_iterator it(batch.begin());
+      ASSERT_TRUE(it++ == batch.begin());
+
+      KuduScanBatch::const_iterator it_next(batch.begin());
+      ASSERT_TRUE(++it_next == it);
+    }
+
+    // Check the prefix increment iterator.
+    {
+      int count = 0;
+      for (KuduScanBatch::const_iterator it = batch.begin();
+           it != batch.end(); ++it) {
+          ++count;
+      }
+      CHECK_EQ(ref_count, count);
+    }
+
+    // Check the postfix increment iterator.
+    {
+      int count = 0;
+      for (KuduScanBatch::const_iterator it = batch.begin();
+           it != batch.end(); it++) {
+          ++count;
+      }
+      CHECK_EQ(ref_count, count);
+    }
+
+    {
+      KuduScanBatch::const_iterator it_pre(batch.begin());
+      KuduScanBatch::const_iterator it_post(batch.begin());
+      for (; it_pre != batch.end(); ++it_pre, it_post++) {
+          ASSERT_TRUE(it_pre == it_post);
+          ASSERT_FALSE(it_pre != it_post);
+      }
+    }
+
+    // Check that iterators which are going over different batches
+    // are different, even if they iterate over the same raw data.
+    {
+      KuduScanner other_scanner(client_table_.get());
+      ASSERT_OK(other_scanner.Open());
+
+      KuduScanBatch other_batch;
+      ASSERT_TRUE(other_scanner.HasMoreRows());
+      ASSERT_OK(other_scanner.NextBatch(&other_batch));
+      const int other_ref_count(other_batch.NumRows());
+      ASSERT_EQ(ROW_NUM, other_ref_count);
+
+      KuduScanBatch::const_iterator it(batch.begin());
+      KuduScanBatch::const_iterator other_it(other_batch.begin());
+      for (; it != batch.end(); ++it, ++other_it) {
+          ASSERT_FALSE(it == other_it);
+          ASSERT_TRUE(it != other_it);
+      }
+    }
+  }
+}
+
 } // namespace client
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/7ca72739/src/kudu/client/scan_batch.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_batch.h b/src/kudu/client/scan_batch.h
index 9ca593e..7808905 100644
--- a/src/kudu/client/scan_batch.h
+++ b/src/kudu/client/scan_batch.h
@@ -51,6 +51,14 @@ class KuduSchema;
 //
 // In C++03, you'll need to use a regular for loop:
 //
+//   for (KuduScanBatch::const_iterator it = batch.begin(), it != batch.end();
+//        ++i) {
+//     KuduScanBatch::RowPtr row(*it);
+//     ...
+//   }
+//
+//   or
+//
 //   for (int i = 0, num_rows = batch.NumRows();
 //        i < num_rows;
 //        i++) {
@@ -181,15 +189,26 @@ class KUDU_EXPORT KuduScanBatch::const_iterator
     return batch_->Row(idx_);
   }
 
-  void operator++() {
-    idx_++;
+  // Prefix increment operator: advances the iterator to the next position.
+  // Returns the reference to the incremented/advanced iterator.
+  const_iterator& operator++() {
+    ++idx_;
+    return *this;
+  }
+
+  // Postfix increment operator: advances the iterator to the next position.
+  // Returns a copy of the iterator pointing to the pre-incremented position.
+  const_iterator operator++(int) {
+    const_iterator tmp(batch_, idx_);
+    ++idx_;
+    return tmp;
   }
 
-  bool operator==(const const_iterator& other) {
-    return idx_ == other.idx_;
+  bool operator==(const const_iterator& other) const {
+    return (idx_ == other.idx_) && (batch_ == other.batch_);
   }
-  bool operator!=(const const_iterator& other) {
-    return idx_ != other.idx_;
+  bool operator!=(const const_iterator& other) const {
+    return !(*this == other);
   }
 
  private:
@@ -199,7 +218,7 @@ class KUDU_EXPORT KuduScanBatch::const_iterator
         idx_(idx) {
   }
 
-  const KuduScanBatch* batch_;
+  const KuduScanBatch* const batch_;
   int idx_;
 };
 


Mime
View raw message