tp: allow conversion from int to double in runtime tables
It's not uncommon for tables to queries which populate tables to
start off containing ints and then switch to doubles or vice versa.
Account for this case by adding code which converts ints to doubles
if there is a mixed column containing both. Still ensure that all ints
are abs(i) <= 2^53 as this is the maximum number densely representible
by doubles
Bug: 296599532
Change-Id: I816202c464a0b99cd02b8b5ddb3c4b372fabed10
diff --git a/Android.bp b/Android.bp
index e254977..347e723 100644
--- a/Android.bp
+++ b/Android.bp
@@ -9798,6 +9798,7 @@
"src/trace_processor/db/column_storage_overlay_unittest.cc",
"src/trace_processor/db/compare_unittest.cc",
"src/trace_processor/db/query_executor_unittest.cc",
+ "src/trace_processor/db/runtime_table_unittest.cc",
"src/trace_processor/db/view_unittest.cc",
],
}
diff --git a/src/trace_processor/db/BUILD.gn b/src/trace_processor/db/BUILD.gn
index 50f572c..067d768 100644
--- a/src/trace_processor/db/BUILD.gn
+++ b/src/trace_processor/db/BUILD.gn
@@ -59,6 +59,7 @@
"column_storage_overlay_unittest.cc",
"compare_unittest.cc",
"query_executor_unittest.cc",
+ "runtime_table_unittest.cc",
"view_unittest.cc",
]
deps = [
diff --git a/src/trace_processor/db/runtime_table.cc b/src/trace_processor/db/runtime_table.cc
index a982b8a..f2559f9 100644
--- a/src/trace_processor/db/runtime_table.cc
+++ b/src/trace_processor/db/runtime_table.cc
@@ -15,11 +15,29 @@
*/
#include "src/trace_processor/db/runtime_table.h"
+#include <cstdint>
+#include <optional>
+#include "perfetto/base/status.h"
namespace perfetto {
namespace trace_processor {
+namespace {
-RuntimeTable::~RuntimeTable() = default;
+template <typename T, typename U>
+T Fill(uint32_t leading_nulls, U value) {
+ T res;
+ for (uint32_t i = 0; i < leading_nulls; ++i) {
+ res.Append(value);
+ }
+ return res;
+}
+
+bool IsPerfectlyRepresentableAsDouble(int64_t res) {
+ static constexpr int64_t kMaxDoubleRepresentible = 1ull << 53;
+ return res >= -kMaxDoubleRepresentible && res <= kMaxDoubleRepresentible;
+}
+
+} // namespace
RuntimeTable::RuntimeTable(StringPool* pool, std::vector<std::string> col_names)
: Table(pool), col_names_(col_names), storage_(col_names_.size()) {
@@ -27,6 +45,8 @@
storage_[i] = std::make_unique<VariantStorage>();
}
+RuntimeTable::~RuntimeTable() = default;
+
base::Status RuntimeTable::AddNull(uint32_t idx) {
auto* col = storage_[idx].get();
if (auto* leading_nulls = std::get_if<uint32_t>(col)) {
@@ -46,11 +66,21 @@
base::Status RuntimeTable::AddInteger(uint32_t idx, int64_t res) {
auto* col = storage_[idx].get();
if (auto* leading_nulls_ptr = std::get_if<uint32_t>(col)) {
- RETURN_IF_ERROR(Fill<IntStorage>(col, *leading_nulls_ptr, std::nullopt));
+ *col = Fill<IntStorage>(*leading_nulls_ptr, std::nullopt);
+ }
+ if (auto* doubles = std::get_if<DoubleStorage>(col)) {
+ if (!IsPerfectlyRepresentableAsDouble(res)) {
+ return base::ErrStatus("Column %s contains %" PRId64
+ " which cannot be represented as a double",
+ col_names_[idx].c_str(), res);
+ }
+ doubles->Append(static_cast<double>(res));
+ return base::OkStatus();
}
auto* ints = std::get_if<IntStorage>(col);
if (!ints) {
- return base::ErrStatus("Column %u does not have consistent types", idx);
+ return base::ErrStatus("Column %s does not have consistent types",
+ col_names_[idx].c_str());
}
ints->Append(res);
return base::OkStatus();
@@ -59,11 +89,29 @@
base::Status RuntimeTable::AddFloat(uint32_t idx, double res) {
auto* col = storage_[idx].get();
if (auto* leading_nulls_ptr = std::get_if<uint32_t>(col)) {
- RETURN_IF_ERROR(Fill<DoubleStorage>(col, *leading_nulls_ptr, std::nullopt));
+ *col = Fill<DoubleStorage>(*leading_nulls_ptr, std::nullopt);
+ }
+ if (auto* ints = std::get_if<IntStorage>(col)) {
+ DoubleStorage storage;
+ for (uint32_t i = 0; i < ints->size(); ++i) {
+ std::optional<int64_t> int_val = ints->Get(i);
+ if (!int_val) {
+ storage.Append(std::nullopt);
+ continue;
+ }
+ if (int_val && !IsPerfectlyRepresentableAsDouble(*int_val)) {
+ return base::ErrStatus("Column %s contains %" PRId64
+ " which cannot be represented as a double",
+ col_names_[idx].c_str(), *int_val);
+ }
+ storage.Append(static_cast<double>(*int_val));
+ }
+ *col = std::move(storage);
}
auto* doubles = std::get_if<DoubleStorage>(col);
if (!doubles) {
- return base::ErrStatus("Column %u does not have consistent types", idx);
+ return base::ErrStatus("Column %s does not have consistent types",
+ col_names_[idx].c_str());
}
doubles->Append(res);
return base::OkStatus();
@@ -72,12 +120,12 @@
base::Status RuntimeTable::AddText(uint32_t idx, const char* ptr) {
auto* col = storage_[idx].get();
if (auto* leading_nulls_ptr = std::get_if<uint32_t>(col)) {
- RETURN_IF_ERROR(
- Fill<StringStorage>(col, *leading_nulls_ptr, StringPool::Id::Null()));
+ *col = Fill<StringStorage>(*leading_nulls_ptr, StringPool::Id::Null());
}
auto* strings = std::get_if<StringStorage>(col);
if (!strings) {
- return base::ErrStatus("Column %u does not have consistent types", idx);
+ return base::ErrStatus("Column %s does not have consistent types",
+ col_names_[idx].c_str());
}
strings->Append(string_pool_->InternString(ptr));
return base::OkStatus();
@@ -88,15 +136,19 @@
for (uint32_t i = 0; i < col_names_.size(); ++i) {
auto* col = storage_[i].get();
if (auto* leading_nulls = std::get_if<uint32_t>(col)) {
- RETURN_IF_ERROR(Fill<IntStorage>(col, *leading_nulls, std::nullopt));
+ PERFETTO_CHECK(*leading_nulls == rows);
+ *col = Fill<IntStorage>(*leading_nulls, std::nullopt);
}
if (auto* ints = std::get_if<IntStorage>(col)) {
+ PERFETTO_CHECK(ints->size() == rows);
columns_.push_back(Column(col_names_[i].c_str(), ints,
Column::Flag::kNoFlag, this, i, 0));
} else if (auto* strings = std::get_if<StringStorage>(col)) {
+ PERFETTO_CHECK(strings->size() == rows);
columns_.push_back(Column(col_names_[i].c_str(), strings,
Column::Flag::kNonNull, this, i, 0));
} else if (auto* doubles = std::get_if<DoubleStorage>(col)) {
+ PERFETTO_CHECK(doubles->size() == rows);
columns_.push_back(Column(col_names_[i].c_str(), doubles,
Column::Flag::kNoFlag, this, i, 0));
} else {
diff --git a/src/trace_processor/db/runtime_table.h b/src/trace_processor/db/runtime_table.h
index 6e14a53..cef31a3 100644
--- a/src/trace_processor/db/runtime_table.h
+++ b/src/trace_processor/db/runtime_table.h
@@ -55,15 +55,6 @@
base::Status AddColumnsAndOverlays(uint32_t rows);
private:
- template <typename T, typename U>
- base::Status Fill(VariantStorage* col, uint32_t leading_nulls, U value) {
- *col = T();
- auto* storage = std::get_if<T>(col);
- for (uint32_t i = 0; i < leading_nulls; ++i) {
- storage->Append(value);
- }
- return base::OkStatus();
- }
std::vector<std::string> col_names_;
std::vector<std::unique_ptr<VariantStorage>> storage_;
};
diff --git a/src/trace_processor/db/runtime_table_unittest.cc b/src/trace_processor/db/runtime_table_unittest.cc
new file mode 100644
index 0000000..85ca8ec
--- /dev/null
+++ b/src/trace_processor/db/runtime_table_unittest.cc
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "src/trace_processor/db/runtime_table.h"
+
+#include "test/gtest_and_gmock.h"
+
+namespace perfetto {
+namespace trace_processor {
+namespace {
+
+class RuntimeTableTest : public ::testing::Test {
+ protected:
+ StringPool pool_;
+ std::vector<std::string> names_{{"foo"}};
+ RuntimeTable table_{&pool_, names_};
+};
+
+TEST_F(RuntimeTableTest, DoubleThenIntValid) {
+ ASSERT_TRUE(table_.AddFloat(0, 1024.3).ok());
+ ASSERT_TRUE(table_.AddInteger(0, 1ll << 53).ok());
+ ASSERT_TRUE(table_.AddColumnsAndOverlays(2).ok());
+
+ const auto& col = table_.columns()[0];
+ ASSERT_EQ(col.Get(0).AsDouble(), 1024.3);
+ ASSERT_EQ(col.Get(1).AsDouble(), static_cast<double>(1ll << 53));
+}
+
+TEST_F(RuntimeTableTest, DoubleThenIntInvalid) {
+ ASSERT_TRUE(table_.AddFloat(0, 1024.0).ok());
+ ASSERT_FALSE(table_.AddInteger(0, (1ll << 53) + 1).ok());
+ ASSERT_FALSE(table_.AddInteger(0, -(1ll << 53) - 1).ok());
+}
+
+TEST_F(RuntimeTableTest, IntThenDouble) {
+ ASSERT_TRUE(table_.AddInteger(0, 1024).ok());
+ ASSERT_TRUE(table_.AddFloat(0, 1.3).ok());
+ ASSERT_TRUE(table_.AddColumnsAndOverlays(2).ok());
+
+ const auto& col = table_.columns()[0];
+ ASSERT_EQ(col.Get(0).AsDouble(), 1024.0);
+ ASSERT_EQ(col.Get(1).AsDouble(), 1.3);
+}
+
+} // namespace
+} // namespace trace_processor
+} // namespace perfetto
