You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/08/03 22:59:32 UTC
[2/2] kudu git commit: [KuduScanBatch::const_iterator] a minor
clean-up
[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 <to...@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 <as...@cloudera.com>
Authored: Tue Aug 2 14:57:00 2016 -0700
Committer: Todd Lipcon <to...@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_;
};