This is an automated email from the ASF dual-hosted git repository.
mrhhsg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 49f122698de [fix](aggregate) Keep embedded NUL bytes in the JSON keys
emitted by topn() (#68243)
49f122698de is described below
commit 49f122698deff5b0a31f8033afd5ac8fb6f38cf7
Author: Jerry Hu <[email protected]>
AuthorDate: Thu Oct 8 15:14:27 2026 +0800
[fix](aggregate) Keep embedded NUL bytes in the JSON keys emitted by topn()
(#68243)
### What problem does this PR solve?
Issue Number: None
Problem Summary: A Doris STRING value is binary, so it may contain a NUL
byte. `topn()` keeps the whole value in its state, but writes the result
JSON with `writer.Key(element.second.c_str())`, which has no length
argument and therefore stops at the first NUL. `topn(concat('a',
unhex('00'), 'b'), 1)` returns `{"a":1}` instead of the real key, and
two values that only differ after the NUL collapse into the same
duplicated key, e.g. `{"a":1,"a":1}`, so the caller cannot recover the
top-N values at all.
Pass the key together with its length so rapidjson escapes the NUL as
`\u0000` and emits the whole key.
Before:
```
mysql> SELECT topn(s, 2) FROM (SELECT concat('a', unhex('00'), 'b') AS s
-> UNION ALL SELECT concat('a', unhex('00'),
'c')) t;
+---------------------+
| {"a":1,"a":1} |
+---------------------+
```
After:
```
+-------------------------------+
| {"a\u0000b":1,"a\u0000c":1} |
+-------------------------------+
```
### Release note
None
### Check List (For Author)
- Test:
- Unit Test: Yes, `AggTopNTest.test_string_key_with_embedded_nul` covers
a string key with an embedded NUL and two keys that only differ after
it.
- Regression test: Yes, `test_topn_embedded_nul` checks `topn()` on such
keys and keeps `topn_array()` as a reference.
- Behavior changed: Yes, `topn()` now emits the complete key with the
NUL escaped as `\u0000` instead of a key truncated at the NUL.
- Does this need documentation: No
https://claude.ai/code/session_01RZ36Pij3o8fGnYYg33PKnq
---
be/src/exprs/aggregate/aggregate_function_topn.h | 3 +-
be/test/exprs/aggregate/agg_topn_test.cpp | 60 ++++++++++++++++++++--
.../aggregate_functions/test_topn_embedded_nul.out | 10 ++++
.../test_topn_embedded_nul.groovy | 39 ++++++++++++++
4 files changed, 108 insertions(+), 4 deletions(-)
diff --git a/be/src/exprs/aggregate/aggregate_function_topn.h
b/be/src/exprs/aggregate/aggregate_function_topn.h
index 062b0e8eefe..82216314cbc 100644
--- a/be/src/exprs/aggregate/aggregate_function_topn.h
+++ b/be/src/exprs/aggregate/aggregate_function_topn.h
@@ -30,6 +30,7 @@
#include <utility>
#include <vector>
+#include "common/cast_set.h"
#include "common/exception.h"
#include "core/assert_cast.h"
#include "core/column/column.h"
@@ -184,7 +185,7 @@ struct AggregateFunctionTopNData {
writer.StartObject();
for (int i = 0; i < std::min((int)counter_vector.size(), top_num);
i++) {
const auto& element = counter_vector[i];
- writer.Key(element.second.c_str());
+ writer.Key(element.second.data(),
cast_set<rapidjson::SizeType>(element.second.size()));
writer.Uint64(element.first);
}
writer.EndObject();
diff --git a/be/test/exprs/aggregate/agg_topn_test.cpp
b/be/test/exprs/aggregate/agg_topn_test.cpp
index 324bb7272d9..f8c5c758459 100644
--- a/be/test/exprs/aggregate/agg_topn_test.cpp
+++ b/be/test/exprs/aggregate/agg_topn_test.cpp
@@ -18,15 +18,71 @@
#include <gtest/gtest.h>
#include <cstdint>
+#include <memory>
#include <string>
+#include "core/arena.h"
#include "core/column/column_string.h"
#include "core/column/column_vector.h"
+#include "core/data_type/data_type.h"
+#include "core/data_type/data_type_number.h"
+#include "core/data_type/data_type_string.h"
#include "core/string_buffer.hpp"
+#include "exprs/aggregate/aggregate_function.h"
+#include "exprs/aggregate/aggregate_function_simple_factory.h"
#include "exprs/aggregate/aggregate_function_topn.h"
namespace doris {
-namespace {
+
+void register_aggregate_function_topn(AggregateFunctionSimpleFactory& factory);
+
+class AggTopNTest : public testing::Test {
+public:
+ void SetUp() override {
+ AggregateFunctionSimpleFactory factory =
AggregateFunctionSimpleFactory::instance();
+ register_aggregate_function_topn(factory);
+ }
+
+protected:
+ Arena _agg_arena_pool;
+};
+
+// String keys are binary, so an embedded NUL must not terminate the JSON key.
+TEST_F(AggTopNTest, test_string_key_with_embedded_nul) {
+ DataTypes data_types = {std::make_shared<DataTypeString>(),
std::make_shared<DataTypeInt32>()};
+ auto agg_function =
+ AggregateFunctionSimpleFactory::instance().get("topn", data_types,
nullptr, false, -1);
+ ASSERT_NE(agg_function, nullptr);
+
+ const std::string frequent("a\0b", 3);
+ const std::string rare("a\0c", 3);
+
+ auto value_column = ColumnString::create();
+ value_column->insert_data(frequent.data(), frequent.size());
+ value_column->insert_data(frequent.data(), frequent.size());
+ value_column->insert_data(rare.data(), rare.size());
+
+ auto top_num_column = ColumnInt32::create();
+ for (size_t i = 0; i < value_column->size(); ++i) {
+ top_num_column->insert_value(2);
+ }
+
+ const IColumn* columns[2] = {value_column.get(), top_num_column.get()};
+
+ std::unique_ptr<char[]> memory(new char[agg_function->size_of_data()]);
+ AggregateDataPtr place = memory.get();
+ agg_function->create(place);
+ for (size_t i = 0; i < value_column->size(); ++i) {
+ agg_function->add(place, columns, i, _agg_arena_pool);
+ }
+
+ auto result_column = ColumnString::create();
+ agg_function->insert_result_into(place, *result_column);
+ agg_function->destroy(place);
+
+ ASSERT_EQ(result_column->size(), 1);
+ EXPECT_EQ(result_column->get_data_at(0).to_string(),
R"({"a\u0000b":2,"a\u0000c":1})");
+}
template <PrimitiveType T>
AggregateFunctionTopNData<T> round_trip(const AggregateFunctionTopNData<T>&
state) {
@@ -120,6 +176,4 @@ TEST(AggregateFunctionTopNTest,
PositiveRateStillLimitsSerializedCandidates) {
state.set_paramenters(1);
EXPECT_EQ(round_trip(state).counter_map, state.counter_map);
}
-
-} // namespace
} // namespace doris
diff --git
a/regression-test/data/query_p0/sql_functions/aggregate_functions/test_topn_embedded_nul.out
b/regression-test/data/query_p0/sql_functions/aggregate_functions/test_topn_embedded_nul.out
new file mode 100644
index 00000000000..0c5edd72fcf
--- /dev/null
+++
b/regression-test/data/query_p0/sql_functions/aggregate_functions/test_topn_embedded_nul.out
@@ -0,0 +1,10 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !topn_single_key --
+{"a\\u0000b":1}
+
+-- !topn_distinct_keys --
+{"a\\u0000b":2,"a\\u0000c":1}
+
+-- !topn_array_element_hex --
+610062
+
diff --git
a/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_topn_embedded_nul.groovy
b/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_topn_embedded_nul.groovy
new file mode 100644
index 00000000000..19bd0628404
--- /dev/null
+++
b/regression-test/suites/query_p0/sql_functions/aggregate_functions/test_topn_embedded_nul.groovy
@@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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.
+
+suite("test_topn_embedded_nul") {
+ // A STRING value is binary, so a NUL byte inside a key must not truncate
+ // the JSON key that topn() emits, and two keys differing after the NUL
+ // must stay distinct in the result.
+ qt_topn_single_key """
+ SELECT topn(s, 1) FROM (SELECT concat('a', unhex('00'), 'b') AS s) t
+ """
+
+ qt_topn_distinct_keys """
+ SELECT topn(s, 2) FROM (
+ SELECT concat('a', unhex('00'), 'b') AS s
+ UNION ALL SELECT concat('a', unhex('00'), 'b')
+ UNION ALL SELECT concat('a', unhex('00'), 'c')
+ ) t
+ """
+
+ qt_topn_array_element_hex """
+ SELECT hex(element_at(topn_array(s, 1), 1)) FROM (
+ SELECT concat('a', unhex('00'), 'b') AS s
+ ) t
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
