tp: Move constraint validation to QueryExecutor level
Bug:307482437
Change-Id: If0111451a7999f123f3a1d2e9bec84de424270d6
diff --git a/src/trace_processor/containers/bit_vector.h b/src/trace_processor/containers/bit_vector.h
index 0e5e72b..243651c 100644
--- a/src/trace_processor/containers/bit_vector.h
+++ b/src/trace_processor/containers/bit_vector.h
@@ -351,8 +351,8 @@
return bv;
}
- // Creates a BitVector of size |end| bit the bits between |start| and |end|
- // filled with corresponding bits |this| BitVector.
+ // Creates a BitVector of size `min(range_end, size())` with bits between
+ // |start| and |end| filled with corresponding bits from |this| BitVector.
BitVector IntersectRange(uint32_t range_start, uint32_t range_end) const;
// Requests the removal of unused capacity.
diff --git a/src/trace_processor/db/query_executor.cc b/src/trace_processor/db/query_executor.cc
index 046aee4..056a8db 100644
--- a/src/trace_processor/db/query_executor.cc
+++ b/src/trace_processor/db/query_executor.cc
@@ -35,6 +35,7 @@
#include "src/trace_processor/db/storage/numeric_storage.h"
#include "src/trace_processor/db/storage/selector_storage.h"
#include "src/trace_processor/db/storage/set_id_storage.h"
+#include "src/trace_processor/db/storage/storage.h"
#include "src/trace_processor/db/storage/string_storage.h"
#include "src/trace_processor/db/storage/types.h"
#include "src/trace_processor/db/table.h"
@@ -64,6 +65,16 @@
return;
}
+ switch (storage.ValidateSearchConstraints(c.value, c.op)) {
+ case storage::Storage::SearchValidationResult::kAllData:
+ return;
+ case storage::Storage::SearchValidationResult::kNoData:
+ rm->Clear();
+ return;
+ case storage::Storage::SearchValidationResult::kOk:
+ break;
+ }
+
uint32_t rm_size = rm->size();
uint32_t rm_first = rm->Get(0);
uint32_t rm_last = rm->Get(rm_size - 1);
@@ -276,5 +287,37 @@
return rm;
}
+void QueryExecutor::BoundedColumnFilterForTesting(const Constraint& c,
+ const storage::Storage& col,
+ RowMap* rm) {
+ switch (col.ValidateSearchConstraints(c.value, c.op)) {
+ case storage::Storage::SearchValidationResult::kAllData:
+ return;
+ case storage::Storage::SearchValidationResult::kNoData:
+ rm->Clear();
+ return;
+ case storage::Storage::SearchValidationResult::kOk:
+ break;
+ }
+
+ LinearSearch(c, col, rm);
+}
+
+void QueryExecutor::IndexedColumnFilterForTesting(const Constraint& c,
+ const storage::Storage& col,
+ RowMap* rm) {
+ switch (col.ValidateSearchConstraints(c.value, c.op)) {
+ case storage::Storage::SearchValidationResult::kAllData:
+ return;
+ case storage::Storage::SearchValidationResult::kNoData:
+ rm->Clear();
+ return;
+ case storage::Storage::SearchValidationResult::kOk:
+ break;
+ }
+
+ IndexSearch(c, col, rm);
+}
+
} // namespace trace_processor
} // namespace perfetto
diff --git a/src/trace_processor/db/query_executor.h b/src/trace_processor/db/query_executor.h
index 013dbf2..0b171ca 100644
--- a/src/trace_processor/db/query_executor.h
+++ b/src/trace_processor/db/query_executor.h
@@ -63,18 +63,14 @@
}
// Used only in unittests. Exposes private function.
- static void BoundedColumnFilterForTesting(const Constraint& c,
- const storage::Storage& col,
- RowMap* rm) {
- LinearSearch(c, col, rm);
- }
+ static void BoundedColumnFilterForTesting(const Constraint&,
+ const storage::Storage&,
+ RowMap*);
// Used only in unittests. Exposes private function.
- static void IndexedColumnFilterForTesting(const Constraint& c,
- const storage::Storage& col,
- RowMap* rm) {
- IndexSearch(c, col, rm);
- }
+ static void IndexedColumnFilterForTesting(const Constraint&,
+ const storage::Storage&,
+ RowMap*);
private:
// Updates RowMap with result of filtering single column using the Constraint.
diff --git a/src/trace_processor/db/query_executor_unittest.cc b/src/trace_processor/db/query_executor_unittest.cc
index a2c05d8..1f73314 100644
--- a/src/trace_processor/db/query_executor_unittest.cc
+++ b/src/trace_processor/db/query_executor_unittest.cc
@@ -46,7 +46,7 @@
storage::NumericStorage<int64_t> storage(&storage_data, ColumnType::kInt64);
Constraint c{0, FilterOp::kGe, SqlValue::Long(3)};
- RowMap rm(0, 5);
+ RowMap rm(0, storage.size());
QueryExecutor::BoundedColumnFilterForTesting(c, storage, &rm);
ASSERT_EQ(rm.size(), 3u);
@@ -116,11 +116,12 @@
std::iota(storage_data.begin(), storage_data.end(), 0);
auto numeric = std::make_unique<storage::NumericStorage<int64_t>>(
&storage_data, ColumnType::kInt64);
+
BitVector bv{1, 1, 0, 1, 1, 0, 0, 0, 1, 0};
storage::NullStorage storage(std::move(numeric), &bv);
Constraint c{0, FilterOp::kIsNull, SqlValue()};
- RowMap rm(0, 10);
+ RowMap rm(0, storage.size());
QueryExecutor::BoundedColumnFilterForTesting(c, storage, &rm);
ASSERT_EQ(rm.size(), 5u);
diff --git a/src/trace_processor/db/storage/BUILD.gn b/src/trace_processor/db/storage/BUILD.gn
index bd80ec7..72d8d67 100644
--- a/src/trace_processor/db/storage/BUILD.gn
+++ b/src/trace_processor/db/storage/BUILD.gn
@@ -82,6 +82,7 @@
":storage",
"../../../../gn:default_deps",
"../../../../gn:gtest_and_gmock",
+ "../../../../include/perfetto/trace_processor:basic_types",
"../../containers",
]
}
diff --git a/src/trace_processor/db/storage/dense_null_storage.cc b/src/trace_processor/db/storage/dense_null_storage.cc
index 9f764a4..e1a6d69 100644
--- a/src/trace_processor/db/storage/dense_null_storage.cc
+++ b/src/trace_processor/db/storage/dense_null_storage.cc
@@ -35,6 +35,10 @@
Storage::SearchValidationResult DenseNullStorage::ValidateSearchConstraints(
SqlValue sql_val,
FilterOp op) const {
+ if (op == FilterOp::kIsNull) {
+ return Storage::SearchValidationResult::kOk;
+ }
+
return inner_->ValidateSearchConstraints(sql_val, op);
}
@@ -43,6 +47,23 @@
RowMap::Range in) const {
PERFETTO_TP_TRACE(metatrace::Category::DB, "DenseNullStorage::Search");
+ if (op == FilterOp::kIsNull) {
+ switch (inner_->ValidateSearchConstraints(sql_val, op)) {
+ case Storage::SearchValidationResult::kNoData: {
+ // There is no need to search in underlying storage. It's enough to
+ // intersect the |non_null_|.
+ BitVector res = non_null_->IntersectRange(in.start, in.end);
+ res.Not();
+ res.Resize(in.end, false);
+ return RangeOrBitVector(std::move(res));
+ }
+ case Storage::SearchValidationResult::kAllData:
+ return RangeOrBitVector(in);
+ case Storage::SearchValidationResult::kOk:
+ break;
+ }
+ }
+
RangeOrBitVector inner_res = inner_->Search(op, sql_val, in);
BitVector res;
if (inner_res.IsRange()) {
diff --git a/src/trace_processor/db/storage/id_storage.cc b/src/trace_processor/db/storage/id_storage.cc
index bec9b041..36418f9 100644
--- a/src/trace_processor/db/storage/id_storage.cc
+++ b/src/trace_processor/db/storage/id_storage.cc
@@ -158,16 +158,6 @@
PERFETTO_DCHECK(search_range.end <= size_);
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(search_range);
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
if (op == FilterOp::kNe) {
BitVector ret(search_range.start, false);
@@ -191,16 +181,6 @@
std::to_string(static_cast<uint32_t>(op)));
});
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, indices_size));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
switch (op) {
diff --git a/src/trace_processor/db/storage/id_storage_unittest.cc b/src/trace_processor/db/storage/id_storage_unittest.cc
index 0db3159..e792c01 100644
--- a/src/trace_processor/db/storage/id_storage_unittest.cc
+++ b/src/trace_processor/db/storage/id_storage_unittest.cc
@@ -16,6 +16,8 @@
#include "src/trace_processor/db/storage/id_storage.h"
#include <limits>
+#include "perfetto/trace_processor/basic_types.h"
+#include "src/trace_processor/db/storage/storage.h"
#include "src/trace_processor/db/storage/types.h"
#include "test/gtest_and_gmock.h"
@@ -33,79 +35,58 @@
TEST(IdStorageUnittest, InvalidSearchConstraints) {
IdStorage storage(100);
- Range test_range(10, 20);
- Range empty_range;
// NULL checks
- SqlValue val;
- val.type = SqlValue::kNull;
- Range search_result =
- storage.Search(FilterOp::kIsNull, val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kIsNotNull, val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue(), FilterOp::kIsNull),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue(), FilterOp::kIsNotNull),
+ Storage::SearchValidationResult::kAllData);
// FilterOp checks
- search_result =
- storage.Search(FilterOp::kGlob, SqlValue::Long(15), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kRegex, SqlValue::Long(15), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Long(15), FilterOp::kGlob),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Long(15), FilterOp::kRegex),
+ Storage::SearchValidationResult::kNoData);
// Type checks
- search_result =
- storage.Search(FilterOp::kGe, SqlValue::String("cheese"), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue::String("cheese"),
+ FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
// Value bounds
SqlValue max_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::max()) + 10);
- search_result =
- storage.Search(FilterOp::kGe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kGt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
- search_result =
- storage.Search(FilterOp::kLe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
- search_result =
- storage.Search(FilterOp::kLt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
- search_result =
- storage.Search(FilterOp::kNe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
SqlValue min_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::min()) - 1);
- search_result =
- storage.Search(FilterOp::kGe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
- search_result =
- storage.Search(FilterOp::kGt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
- search_result =
- storage.Search(FilterOp::kNe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, test_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
- search_result =
- storage.Search(FilterOp::kLe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kLt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
}
TEST(IdStorageUnittest, SearchEqSimple) {
@@ -179,13 +160,6 @@
ASSERT_EQ(bv.CountSetBits(), 39u);
}
-TEST(IdStorageUnittest, SearchNeInvalidNum) {
- IdStorage storage(100);
- Range r = storage.Search(FilterOp::kNe, SqlValue::Long(-1), Range(30, 70))
- .TakeIfRange();
- ASSERT_EQ(r.size(), 40u);
-}
-
TEST(IdStorageUnittest, IndexSearchEqSimple) {
IdStorage storage(12);
std::vector<uint32_t> indices{1, 3, 5, 7, 9, 11, 2, 4};
diff --git a/src/trace_processor/db/storage/null_storage.cc b/src/trace_processor/db/storage/null_storage.cc
index 6de2759..44e2061 100644
--- a/src/trace_processor/db/storage/null_storage.cc
+++ b/src/trace_processor/db/storage/null_storage.cc
@@ -17,11 +17,11 @@
#include "src/trace_processor/db/storage/null_storage.h"
#include <cstdint>
-#include <variant>
#include "protos/perfetto/trace_processor/serialization.pbzero.h"
#include "src/trace_processor/containers/bit_vector.h"
#include "src/trace_processor/containers/row_map.h"
+#include "src/trace_processor/db/storage/storage.h"
#include "src/trace_processor/db/storage/types.h"
#include "src/trace_processor/tp_metatrace.h"
@@ -76,6 +76,10 @@
Storage::SearchValidationResult NullStorage::ValidateSearchConstraints(
SqlValue sql_val,
FilterOp op) const {
+ if (op == FilterOp::kIsNull) {
+ return Storage::SearchValidationResult::kOk;
+ }
+
return storage_->ValidateSearchConstraints(sql_val, op);
}
@@ -90,6 +94,23 @@
RowMap::Range in) const {
PERFETTO_TP_TRACE(metatrace::Category::DB, "NullStorage::Search");
+ if (op == FilterOp::kIsNull) {
+ switch (storage_->ValidateSearchConstraints(sql_val, op)) {
+ case Storage::SearchValidationResult::kNoData: {
+ // There is no need to search in underlying storage. It's enough to
+ // intersect the |non_null_|.
+ BitVector res = non_null_->IntersectRange(in.start, in.end);
+ res.Not();
+ res.Resize(non_null_->size(), false);
+ return RangeOrBitVector(std::move(res));
+ }
+ case Storage::SearchValidationResult::kAllData:
+ return RangeOrBitVector(in);
+ case Storage::SearchValidationResult::kOk:
+ break;
+ }
+ }
+
// Figure out the bounds of the indices in the underlying storage and search
// it.
uint32_t start = non_null_->CountSetBits(in.start);
@@ -106,6 +127,24 @@
bool sorted) const {
PERFETTO_TP_TRACE(metatrace::Category::DB, "NullStorage::IndexSearch");
+ if (op == FilterOp::kIsNull) {
+ switch (storage_->ValidateSearchConstraints(sql_val, op)) {
+ case Storage::SearchValidationResult::kNoData: {
+ BitVector::Builder null_indices(indices_size);
+ for (uint32_t* it = indices; it != indices + indices_size; it++) {
+ null_indices.Append(!non_null_->IsSet(*it));
+ }
+ // There is no need to search in underlying storage. We should just
+ // check if the index is set in |non_null_|.
+ return RangeOrBitVector(std::move(null_indices).Build());
+ }
+ case Storage::SearchValidationResult::kAllData:
+ return RangeOrBitVector(Range(0, indices_size));
+ case Storage::SearchValidationResult::kOk:
+ break;
+ }
+ }
+
BitVector::Builder storage_non_null(indices_size);
std::vector<uint32_t> storage_iv;
storage_iv.reserve(indices_size);
diff --git a/src/trace_processor/db/storage/numeric_storage.cc b/src/trace_processor/db/storage/numeric_storage.cc
index 0b792de..1857eee 100644
--- a/src/trace_processor/db/storage/numeric_storage.cc
+++ b/src/trace_processor/db/storage/numeric_storage.cc
@@ -316,16 +316,6 @@
std::to_string(static_cast<uint32_t>(op)));
});
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, search_range.end));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
NumericValue val = GetNumericTypeVariant(type_, sql_val);
if (is_sorted_) {
@@ -356,15 +346,6 @@
std::to_string(static_cast<uint32_t>(op)));
});
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, indices_size));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
NumericValue val = GetNumericTypeVariant(type_, sql_val);
if (sorted) {
return RangeOrBitVector(
diff --git a/src/trace_processor/db/storage/numeric_storage_unittest.cc b/src/trace_processor/db/storage/numeric_storage_unittest.cc
index 36fa51a..aa2d2d7 100644
--- a/src/trace_processor/db/storage/numeric_storage_unittest.cc
+++ b/src/trace_processor/db/storage/numeric_storage_unittest.cc
@@ -14,6 +14,7 @@
* limitations under the License.
*/
#include "src/trace_processor/db/storage/numeric_storage.h"
+#include <cstdint>
#include "src/trace_processor/db/storage/types.h"
#include "test/gtest_and_gmock.h"
@@ -40,30 +41,23 @@
Range empty_range;
// NULL checks
- SqlValue val;
- val.type = SqlValue::kNull;
- Range search_result =
- storage.Search(FilterOp::kIsNull, val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kIsNotNull, val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue(), FilterOp::kIsNull),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue(), FilterOp::kIsNotNull),
+ Storage::SearchValidationResult::kAllData);
// FilterOp checks
- search_result =
- storage.Search(FilterOp::kGlob, SqlValue::Long(15), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kRegex, SqlValue::Long(15), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Long(15), FilterOp::kGlob),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Long(15), FilterOp::kRegex),
+ Storage::SearchValidationResult::kNoData);
// Type checks
- search_result =
- storage.Search(FilterOp::kGe, SqlValue::String("cheese"), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue::String("cheese"),
+ FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
}
TEST(NumericStorageUnittest, InvalidValueBoundsUint32) {
@@ -71,53 +65,37 @@
std::iota(data_vec.begin(), data_vec.end(), 0);
NumericStorage<uint32_t> storage(&data_vec, ColumnType::kUint32);
- Range test_range(20, 100);
- Range full_range(0, 100);
- Range empty_range;
-
SqlValue max_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::max()) + 10);
- Range search_result =
- storage.Search(FilterOp::kGe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kGt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
- search_result =
- storage.Search(FilterOp::kLe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kLt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kNe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
SqlValue min_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::min()) - 1);
- search_result =
- storage.Search(FilterOp::kGe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kGt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kNe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
- search_result =
- storage.Search(FilterOp::kLe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kLt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
}
TEST(NumericStorageUnittest, InvalidValueBoundsInt32) {
@@ -125,53 +103,37 @@
std::iota(data_vec.begin(), data_vec.end(), 0);
NumericStorage<int32_t> storage(&data_vec, ColumnType::kInt32);
- Range test_range(20, 100);
- Range full_range(0, 100);
- Range empty_range;
-
SqlValue max_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<int32_t>::max()) + 10);
- Range search_result =
- storage.Search(FilterOp::kGe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kGt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
- search_result =
- storage.Search(FilterOp::kLe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kLt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kNe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
SqlValue min_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<int32_t>::min()) - 1);
- search_result =
- storage.Search(FilterOp::kGe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kGt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kNe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
- search_result =
- storage.Search(FilterOp::kLe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kLt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
}
TEST(NumericStorageUnittest, StableSortTrivial) {
diff --git a/src/trace_processor/db/storage/set_id_storage.cc b/src/trace_processor/db/storage/set_id_storage.cc
index 94b94dc..b703644 100644
--- a/src/trace_processor/db/storage/set_id_storage.cc
+++ b/src/trace_processor/db/storage/set_id_storage.cc
@@ -141,16 +141,6 @@
PERFETTO_DCHECK(search_range.end <= size());
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, search_range.end));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
if (op == FilterOp::kNe) {
@@ -179,16 +169,6 @@
std::to_string(static_cast<uint32_t>(op)));
});
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, indices_size));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
uint32_t val = static_cast<uint32_t>(sql_val.AsLong());
BitVector::Builder builder(indices_size);
diff --git a/src/trace_processor/db/storage/set_id_storage_unittest.cc b/src/trace_processor/db/storage/set_id_storage_unittest.cc
index 5023345..b6f6cdb 100644
--- a/src/trace_processor/db/storage/set_id_storage_unittest.cc
+++ b/src/trace_processor/db/storage/set_id_storage_unittest.cc
@@ -32,81 +32,57 @@
TEST(SetIdStorageUnittest, InvalidSearchConstraints) {
std::vector<uint32_t> storage_data{0, 0, 0, 3, 3, 3, 6, 6, 6, 9, 9, 9};
SetIdStorage storage(&storage_data);
-
- Range test_range(3, 9);
- Range full_range(0, 9);
- Range empty_range;
-
// NULL checks
- SqlValue val;
- val.type = SqlValue::kNull;
- Range search_result =
- storage.Search(FilterOp::kIsNull, val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kIsNotNull, val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue(), FilterOp::kIsNull),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue(), FilterOp::kIsNotNull),
+ Storage::SearchValidationResult::kAllData);
// FilterOp checks
- search_result =
- storage.Search(FilterOp::kGlob, SqlValue::Long(15), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kRegex, SqlValue::Long(15), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Long(15), FilterOp::kGlob),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(
+ storage.ValidateSearchConstraints(SqlValue::Long(15), FilterOp::kRegex),
+ Storage::SearchValidationResult::kNoData);
// Type checks
- search_result =
- storage.Search(FilterOp::kGe, SqlValue::String("cheese"), test_range)
- .TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(SqlValue::String("cheese"),
+ FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
// Value bounds
SqlValue max_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::max()) + 10);
- search_result =
- storage.Search(FilterOp::kGe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kGt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
- search_result =
- storage.Search(FilterOp::kLe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kLt, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kNe, max_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(max_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
SqlValue min_val = SqlValue::Long(
static_cast<int64_t>(std::numeric_limits<uint32_t>::min()) - 1);
- search_result =
- storage.Search(FilterOp::kGe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kGt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
- search_result =
- storage.Search(FilterOp::kNe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, full_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGe),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kGt),
+ Storage::SearchValidationResult::kAllData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kNe),
+ Storage::SearchValidationResult::kAllData);
- search_result =
- storage.Search(FilterOp::kLe, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kLt, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
- search_result =
- storage.Search(FilterOp::kEq, min_val, test_range).TakeIfRange();
- ASSERT_EQ(search_result, empty_range);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLe),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kLt),
+ Storage::SearchValidationResult::kNoData);
+ ASSERT_EQ(storage.ValidateSearchConstraints(min_val, FilterOp::kEq),
+ Storage::SearchValidationResult::kNoData);
}
TEST(SetIdStorageUnittest, SearchEqSimple) {
diff --git a/src/trace_processor/db/storage/storage.h b/src/trace_processor/db/storage/storage.h
index 2a9c9fb..e2cf237 100644
--- a/src/trace_processor/db/storage/storage.h
+++ b/src/trace_processor/db/storage/storage.h
@@ -42,8 +42,6 @@
// the |Search| and |IndexSearch| in special cases.
//
// Notes for callers:
- // * This function is being called by Search and IndexSearch and there is no
- // need to call it before searches.
// * The SqlValue and FilterOp have to be valid in Sqlite: it will crash if
// either: value is NULL and operation is different than "IS NULL" and "IS
// NOT NULL" or the operation is "IS NULL" and "IS NOT NULL" and value is
@@ -58,6 +56,11 @@
// which match the constraint. If a BitVector is returned, it will be
// *precisely* as large as |range.end|.
//
+ // Notes for callers:
+ // * Should only be called if ValidateSearchContraints returned kOk.
+ // * Callers should note that the return value of this function corresponds
+ // to positions in |indices| *not* positions in the storage.
+ //
// Notes for implementors:
// * Implementations should ensure that the return value *only* includes
// positions in |range| as callers will expect this to be true and can
@@ -76,6 +79,7 @@
// be *precisely* as large as |indices_count|.
//
// Notes for callers:
+ // * Should only be called if ValidateSearchContraints returned kOk.
// * Callers should note that the return value of this function corresponds
// to positions in |indices| *not* positions in the storage.
//
diff --git a/src/trace_processor/db/storage/string_storage.cc b/src/trace_processor/db/storage/string_storage.cc
index d9c9b98..229e470 100644
--- a/src/trace_processor/db/storage/string_storage.cc
+++ b/src/trace_processor/db/storage/string_storage.cc
@@ -196,16 +196,6 @@
std::to_string(static_cast<uint32_t>(op)));
});
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, search_range.end));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
if (is_sorted_) {
if (op != FilterOp::kNe) {
return RangeOrBitVector(BinarySearchIntrinsic(op, sql_val, search_range));
@@ -233,16 +223,6 @@
std::to_string(static_cast<uint32_t>(op)));
});
- // After this switch we assume the search is valid.
- switch (ValidateSearchConstraints(sql_val, op)) {
- case SearchValidationResult::kOk:
- break;
- case SearchValidationResult::kAllData:
- return RangeOrBitVector(Range(0, indices_size));
- case SearchValidationResult::kNoData:
- return RangeOrBitVector(Range());
- }
-
if (indices_sorted) {
return RangeOrBitVector(
BinarySearchExtrinsic(op, sql_val, indices, indices_size));
