Merge "tp: Fix StringStorage sort" into main
diff --git a/src/trace_processor/db/column/string_storage.cc b/src/trace_processor/db/column/string_storage.cc
index 07dfa76..9ab8c1f 100644
--- a/src/trace_processor/db/column/string_storage.cc
+++ b/src/trace_processor/db/column/string_storage.cc
@@ -620,20 +620,51 @@
SortToken* end,
SortDirection direction) const {
switch (direction) {
- case SortDirection::kAscending:
+ case SortDirection::kAscending: {
std::stable_sort(start, end,
- [this](const SortToken& a, const SortToken& b) {
- return string_pool_->Get((*data_)[a.index]) <
- string_pool_->Get((*data_)[b.index]);
+ [this](const SortToken& lhs, const SortToken& rhs) {
+ // If RHS is NULL, we know that LHS is not less than
+ // NULL, as nothing is less then null. This check is
+ // only required to keep the stability of the sort.
+ if ((*data_)[rhs.index] == StringPool::Id::Null()) {
+ return false;
+ }
+
+ // If LHS is NULL, it will always be smaller than any
+ // RHS value.
+ if ((*data_)[lhs.index] == StringPool::Id::Null()) {
+ return true;
+ }
+
+ // If neither LHS or RHS are NULL, we have to simply
+ // check which string is smaller.
+ return string_pool_->Get((*data_)[lhs.index]) <
+ string_pool_->Get((*data_)[rhs.index]);
});
return;
- case SortDirection::kDescending:
+ }
+ case SortDirection::kDescending: {
std::stable_sort(start, end,
- [this](const SortToken& a, const SortToken& b) {
- return string_pool_->Get((*data_)[a.index]) >
- string_pool_->Get((*data_)[b.index]);
+ [this](const SortToken& lhs, const SortToken& rhs) {
+ // If LHS is NULL, we know that it's not greater than
+ // any RHS. This check is only required to keep the
+ // stability of the sort.
+ if ((*data_)[lhs.index] == StringPool::Id::Null()) {
+ return false;
+ }
+
+ // If RHS is NULL, everything will be greater from it.
+ if ((*data_)[rhs.index] == StringPool::Id::Null()) {
+ return true;
+ }
+
+ // If neither LHS or RHS are NULL, we have to simply
+ // check which string is smaller.
+ return string_pool_->Get((*data_)[lhs.index]) >
+ string_pool_->Get((*data_)[rhs.index]);
});
return;
+ }
}
PERFETTO_FATAL("For GCC");
}
diff --git a/test/trace_processor/diff_tests/syntax/filtering_tests.py b/test/trace_processor/diff_tests/syntax/filtering_tests.py
index adf8c3e..22b4111 100644
--- a/test/trace_processor/diff_tests/syntax/filtering_tests.py
+++ b/test/trace_processor/diff_tests/syntax/filtering_tests.py
@@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from python.generators.diff_tests.testing import DataPath
+from python.generators.diff_tests.testing import DataPath, TextProto
from python.generators.diff_tests.testing import Csv
from python.generators.diff_tests.testing import DiffTestBlueprint
from python.generators.diff_tests.testing import TestSuite
@@ -189,3 +189,49 @@
"cnt"
0
"""))
+
+ def test_string_null_vs_empty(self):
+ return DiffTestBlueprint(
+ trace=TextProto(""),
+ query="""
+ CREATE PERFETTO TABLE foo AS
+ SELECT 0 as id, NULL AS strings
+ UNION ALL
+ SELECT 1, 'cheese'
+ UNION ALL
+ SELECT 2, NULL
+ UNION ALL
+ SELECT 3, '';
+
+ SELECT * FROM foo ORDER BY strings ASC;
+ """,
+ out=Csv("""
+ "id","strings"
+ 0,"[NULL]"
+ 2,"[NULL]"
+ 3,""
+ 1,"cheese"
+ """))
+
+ def test_string_null_vs_empty_desc(self):
+ return DiffTestBlueprint(
+ trace=TextProto(""),
+ query="""
+ CREATE PERFETTO TABLE foo AS
+ SELECT 0 as id, NULL AS strings
+ UNION ALL
+ SELECT 1, 'cheese'
+ UNION ALL
+ SELECT 2, NULL
+ UNION ALL
+ SELECT 3, '';
+
+ SELECT * FROM foo ORDER BY strings DESC;
+ """,
+ out=Csv("""
+ "id","strings"
+ 1,"cheese"
+ 3,""
+ 0,"[NULL]"
+ 2,"[NULL]"
+ """))
