trace_processor: fix multiple bugs with filtering and casting
When comparision against doubles was attempted to be fixed in aosp/821500,
we accidentally broke comparision of integers vs integers. Moreover,
by casting doubles to integers, we were losing precision in our
comparisions. This caused 119496959 which led to bugs in the timeline.
Fix these issues by choosing more carefully which type we want to
compare in - compare as integers if both the sqlite value and the value
in the table are integers. Otherwise, convert both to doubles and
compare there.
Also fix a minor bug where sqlite_value_int was used for int64 types.
Bug: 119496959
Change-Id: I6369abed91b9c97f58ec7da3f314d1ebbacdbab4
diff --git a/src/trace_processor/sqlite_utils.h b/src/trace_processor/sqlite_utils.h
index 4c700f9..0d3dc94 100644
--- a/src/trace_processor/sqlite_utils.h
+++ b/src/trace_processor/sqlite_utils.h
@@ -95,29 +95,29 @@
template <>
inline uint8_t ExtractSqliteValue(sqlite3_value* value) {
auto type = sqlite3_value_type(value);
- PERFETTO_DCHECK(type == SQLITE_INTEGER || type == SQLITE_FLOAT);
+ PERFETTO_DCHECK(type == SQLITE_INTEGER);
return static_cast<uint8_t>(sqlite3_value_int(value));
}
template <>
inline uint32_t ExtractSqliteValue(sqlite3_value* value) {
auto type = sqlite3_value_type(value);
- PERFETTO_DCHECK(type == SQLITE_INTEGER || type == SQLITE_FLOAT);
- return static_cast<uint32_t>(sqlite3_value_int(value));
+ PERFETTO_DCHECK(type == SQLITE_INTEGER);
+ return static_cast<uint32_t>(sqlite3_value_int64(value));
}
template <>
inline uint64_t ExtractSqliteValue(sqlite3_value* value) {
auto type = sqlite3_value_type(value);
- PERFETTO_DCHECK(type == SQLITE_INTEGER || type == SQLITE_FLOAT);
- return static_cast<uint64_t>(sqlite3_value_int(value));
+ PERFETTO_DCHECK(type == SQLITE_INTEGER);
+ return static_cast<uint64_t>(sqlite3_value_int64(value));
}
template <>
inline int64_t ExtractSqliteValue(sqlite3_value* value) {
auto type = sqlite3_value_type(value);
- PERFETTO_DCHECK(type == SQLITE_INTEGER || type == SQLITE_FLOAT);
- return static_cast<int64_t>(sqlite3_value_int(value));
+ PERFETTO_DCHECK(type == SQLITE_INTEGER);
+ return static_cast<int64_t>(sqlite3_value_int64(value));
}
template <>
diff --git a/src/trace_processor/storage_schema.h b/src/trace_processor/storage_schema.h
index d50efd4..ebca90f 100644
--- a/src/trace_processor/storage_schema.h
+++ b/src/trace_processor/storage_schema.h
@@ -131,11 +131,14 @@
}
Predicate Filter(int op, sqlite3_value* value) const override {
- auto binary_op = sqlite_utils::GetPredicateForOp<T>(op);
- T extracted = sqlite_utils::ExtractSqliteValue<T>(value);
- return [this, binary_op, extracted](uint32_t idx) {
- return binary_op((*deque_)[idx], extracted);
- };
+ auto type = sqlite3_value_type(value);
+ if (type == SQLITE_INTEGER && std::is_integral<T>::value) {
+ return FilterWithCast<int64_t>(op, value);
+ } else if (type == SQLITE_INTEGER || type == SQLITE_FLOAT) {
+ return FilterWithCast<double>(op, value);
+ } else {
+ PERFETTO_FATAL("Unexpected sqlite value to compare against");
+ }
}
Comparator Sort(const QueryConstraints::OrderBy& ob) const override {
@@ -172,6 +175,16 @@
}
private:
+ template <typename C>
+ Predicate FilterWithCast(int op, sqlite3_value* value) const {
+ auto binary_op = sqlite_utils::GetPredicateForOp<C>(op);
+ C extracted = sqlite_utils::ExtractSqliteValue<C>(value);
+ return [this, binary_op, extracted](uint32_t idx) {
+ auto val = static_cast<C>((*deque_)[idx]);
+ return binary_op(val, extracted);
+ };
+ }
+
const std::deque<T>* deque_ = nullptr;
bool is_naturally_ordered_ = false;
};
diff --git a/test/trace_processor/android_sched_and_ps_b119496959.out b/test/trace_processor/android_sched_and_ps_b119496959.out
new file mode 100644
index 0000000..480ada8
--- /dev/null
+++ b/test/trace_processor/android_sched_and_ps_b119496959.out
@@ -0,0 +1,11 @@
+"ts","cpu"
+81473797824982,3
+81473797942847,3
+81473798135399,0
+81473798786857,2
+81473798875451,3
+81473799019930,2
+81473799079982,0
+81473800089357,3
+81473800144461,3
+81473800441805,3
diff --git a/test/trace_processor/b119496959.sql b/test/trace_processor/b119496959.sql
new file mode 100644
index 0000000..be4460c
--- /dev/null
+++ b/test/trace_processor/b119496959.sql
@@ -0,0 +1 @@
+select ts, cpu from sched where ts >= 81473797418963 limit 10
diff --git a/test/trace_processor/index b/test/trace_processor/index
index ea24c2d..81dbe9f 100644
--- a/test/trace_processor/index
+++ b/test/trace_processor/index
@@ -5,6 +5,7 @@
../data/android_sched_and_ps.pb slice_span_join_b118665515.sql android_sched_and_ps_slice_span_join_b118665515.out
../data/android_sched_and_ps.pb b119301023.sql android_sched_and_ps_b119301023.out
../data/android_sched_and_ps.pb stats.sql android_sched_and_ps_stats.out
+../data/android_sched_and_ps.pb b119496959.sql android_sched_and_ps_b119496959.out
synth_1.py smoke.sql synth_1_smoke.out
synth_1.py filter_sched.sql synth_1_filter_sched.out
synth_1.py filter_counters.sql synth_1_filter_counters.out
