[5/6] kudu git commit: KuduPredicate::Clone on IN list predicate causes segfault

[da...@apache.org]

KuduPredicate::Clone on IN list predicate causes segfault

Jordan Birdsell uncovered this issue where a cloned IN list predicate
causes a segfault when applied to a scanner. During the clone, the
values list was being 'preallocated' using the vector(int) constructor,
which adds a bunch of default constructed values (in this case, nullptrs
since the type is const void*). We previously didn't have any test
coverage of the Clone method, so I added a check to every column
predicate test to make sure that scans with cloned predicates behave the
same as the original predicates. The test segfaults without the fix.

Change-Id: I977691390535273c37aa90c3a3663a7c09ea67fc
Reviewed-on: http://gerrit.cloudera.org:8080/4568
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Jordan Birdsell <jo...@gmail.com>


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

Branch: refs/heads/master
Commit: 74c9e67ce326e851c39677e44cca7ab7d4955e0a
Parents: 5385af8
Author: Dan Burkert <da...@cloudera.com>
Authored: Thu Sep 29 16:52:57 2016 -0700
Committer: Dan Burkert <da...@cloudera.com>
Committed: Fri Sep 30 02:55:49 2016 +0000

----------------------------------------------------------------------
 src/kudu/client/predicate-test.cc         | 21 +++++++++++++++++++--
 src/kudu/client/scan_predicate-internal.h |  3 ++-
 2 files changed, 21 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/74c9e67c/src/kudu/client/predicate-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/predicate-test.cc b/src/kudu/client/predicate-test.cc
index 4f01219..cc0cdbb 100644
--- a/src/kudu/client/predicate-test.cc
+++ b/src/kudu/client/predicate-test.cc
@@ -85,8 +85,8 @@ class PredicateTest : public KuduTest {
   }
 
   // Count the rows in a table which satisfy the specified predicates.
-  int CountRows(const shared_ptr<KuduTable>& table,
-                const vector<KuduPredicate*>& predicates) {
+  int DoCountRows(const shared_ptr<KuduTable>& table,
+                  const vector<KuduPredicate*>& predicates) {
     KuduScanner scanner(table.get());
     for (KuduPredicate* predicate : predicates) {
       CHECK_OK(scanner.AddConjunctPredicate(predicate));
@@ -102,6 +102,23 @@ class PredicateTest : public KuduTest {
     return rows;
   }
 
+  // Count the rows in a table which satisfy the specified predicates. This
+  // also does a separate scan with cloned predicates, in order to test that
+  // cloned predicates behave exactly as the original.
+  int CountRows(const shared_ptr<KuduTable>& table,
+                const vector<KuduPredicate*>& predicates) {
+
+    vector<KuduPredicate*> cloned_predicates;
+    for (KuduPredicate* pred : predicates) {
+      cloned_predicates.push_back(pred->Clone());
+    }
+
+    int cloned_count = DoCountRows(table, cloned_predicates);
+    int count = DoCountRows(table, predicates);
+    CHECK_EQ(count, cloned_count);
+    return count;
+  }
+
   template <typename T>
   int CountMatchedRows(const vector<T>& values, const vector<T>& test_values) {
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/74c9e67c/src/kudu/client/scan_predicate-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/scan_predicate-internal.h b/src/kudu/client/scan_predicate-internal.h
index 05585f6..e0757ae 100644
--- a/src/kudu/client/scan_predicate-internal.h
+++ b/src/kudu/client/scan_predicate-internal.h
@@ -100,7 +100,8 @@ class InListPredicateData : public KuduPredicate::Data {
   Status AddToScanSpec(ScanSpec* spec, Arena* arena) override;
 
   InListPredicateData* Clone() const override {
-    std::vector<KuduValue*> values(vals_.size());
+    std::vector<KuduValue*> values;
+    values.reserve(vals_.size());
     for (KuduValue* val : vals_) {
       values.push_back(val->Clone());
     }