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 <[email protected]> Tested-by: Kudu Jenkins Reviewed-by: Jordan Birdsell <[email protected]> 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 <[email protected]> Authored: Thu Sep 29 16:52:57 2016 -0700 Committer: Dan Burkert <[email protected]> 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()); }
