This is an automated email from the ASF dual-hosted git repository.
HappenLee 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 b79d65203ce [fix](be) Preserve the sample triggering QuantileState
digest conversion (#69037)
b79d65203ce is described below
commit b79d65203cef65c0c1a5a4e39f7f85772cac0949
Author: TengJianPing <[email protected]>
AuthorDate: Wed Oct 14 10:11:55 2026 +0800
[fix](be) Preserve the sample triggering QuantileState digest conversion
(#69037)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Adding a sample to a QuantileState with 2048 explicit
values converted the existing values to a TDigest but omitted the
incoming sample. Merging 2048 zeros with a singleton 100 consequently
returned a maximum of 0 and total weight 2048, while a different
grouping retained all 2049 inputs. Add the triggering sample before
clearing the explicit buffer. The shared add_value path fixes direct
additions and both EXPLICIT/SINGLE merge orders. Add unit coverage for
boundary counts, both extrema, compression limits, merge orders and the
EXPLICIT/EXPLICIT control grouping, plus SQL regression coverage for
stored partial states and ordered window merges.
### Release note
Fix lost samples and incorrect quantile results when QUANTILE_STATE
crosses the 2048-sample explicit representation limit.
### Check List (For Author)
- Test:
- Unit Test: ASAN run-be-ut.sh, all 48 QuantileState-related tests
passed. Reverting the fix makes both new boundary tests fail; the
control passes.
- Regression test: run-regression-test.sh,
test_quantile_state_explicit_boundary and test_quantile_state_function
passed; expected output was generated by the runner.
- Manual test: the reported SQL changes from maximum 0 / weight 2048 to
maximum 100 / weight 2049 using the same persisted partial states.
- ASAN BE build, clang-format 16, clang-tidy and build hygiene checks
passed.
- Behavior changed: Yes, retain the sample triggering
EXPLICIT-to-TDIGEST conversion.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
be/src/core/value/quantile_state.cpp | 2 +-
be/test/core/value/quantile_state_test.cpp | 60 +++++++++++++++++++
.../test_quantile_state_explicit_boundary.out | 22 +++++++
.../test_quantile_state_explicit_boundary.groovy | 70 ++++++++++++++++++++++
4 files changed, 153 insertions(+), 1 deletion(-)
diff --git a/be/src/core/value/quantile_state.cpp
b/be/src/core/value/quantile_state.cpp
index c5008f59616..c73ce277c2a 100644
--- a/be/src/core/value/quantile_state.cpp
+++ b/be/src/core/value/quantile_state.cpp
@@ -410,10 +410,10 @@ void QuantileState::add_value(const double& value) {
for (int i = 0; i < _explicit_data.size(); i++) {
_tdigest_ptr->digest.add((float)_explicit_data[i]);
}
+ _tdigest_ptr->digest.add((float)value);
_explicit_data.clear();
_explicit_data.shrink_to_fit();
_type = TDIGEST;
-
} else {
_explicit_data.emplace_back(value);
}
diff --git a/be/test/core/value/quantile_state_test.cpp
b/be/test/core/value/quantile_state_test.cpp
index 80ac1aa1ccf..defb69a62da 100644
--- a/be/test/core/value/quantile_state_test.cpp
+++ b/be/test/core/value/quantile_state_test.cpp
@@ -149,6 +149,66 @@ static long serialized_digest_weight(const QuantileState&
state) {
return digest.total_weight();
}
+static void expect_digest_samples(const QuantileState& state, int count,
double minimum,
+ double maximum) {
+ ASSERT_EQ(TDIGEST, state._type);
+ EXPECT_EQ(count, serialized_digest_weight(state));
+ EXPECT_EQ(minimum, state.get_value_by_percentile(0));
+ EXPECT_EQ(maximum, state.get_value_by_percentile(1));
+}
+
+TEST(QuantileStateTest, AddValueRetainsSampleAtExplicitLimit) {
+ for (float compression : {QUANTILE_STATE_COMPRESSION_MIN,
QUANTILE_STATE_COMPRESSION_MAX}) {
+ for (double value : {-100.0, 100.0}) {
+ QuantileState state(compression);
+ for (int i = 0; i < QUANTILE_STATE_EXPLICIT_NUM - 1; ++i) {
+ state.add_value(0);
+ }
+ ASSERT_EQ(EXPLICIT, state._type);
+ EXPECT_EQ(QUANTILE_STATE_EXPLICIT_NUM - 1,
state._explicit_data.size());
+ state.add_value(0);
+ ASSERT_EQ(EXPLICIT, state._type);
+ EXPECT_EQ(QUANTILE_STATE_EXPLICIT_NUM,
state._explicit_data.size());
+
+ state.add_value(value);
+ EXPECT_TRUE(state._explicit_data.empty());
+ expect_digest_samples(state, QUANTILE_STATE_EXPLICIT_NUM + 1,
std::min(0.0, value),
+ std::max(0.0, value));
+
+ state.add_value(0);
+ expect_digest_samples(state, QUANTILE_STATE_EXPLICIT_NUM + 2,
std::min(0.0, value),
+ std::max(0.0, value));
+ }
+ }
+}
+
+TEST(QuantileStateTest, MergeSingletonAndExplicitRetainsBoundarySample) {
+ for (double value : {-100.0, 100.0}) {
+ auto explicit_state = constant_state(0, QUANTILE_STATE_EXPLICIT_NUM);
+ auto singleton = constant_state(value, 1);
+
+ auto explicit_first = explicit_state;
+ explicit_first.merge(singleton);
+ auto singleton_first = singleton;
+ singleton_first.merge(explicit_state);
+ expect_digest_samples(explicit_first, QUANTILE_STATE_EXPLICIT_NUM + 1,
std::min(0.0, value),
+ std::max(0.0, value));
+ expect_digest_samples(singleton_first, QUANTILE_STATE_EXPLICIT_NUM + 1,
+ std::min(0.0, value), std::max(0.0, value));
+ EXPECT_EQ(QUANTILE_STATE_EXPLICIT_NUM,
explicit_state._explicit_data.size());
+ EXPECT_EQ(0, explicit_state.get_value_by_percentile(1));
+ EXPECT_EQ(value, singleton.get_value_by_percentile(1));
+ }
+}
+
+TEST(QuantileStateTest, ExplicitMergeGroupingRetainsAllSamples) {
+ auto state = constant_state(0, QUANTILE_STATE_EXPLICIT_NUM - 1);
+ auto other = constant_state(0, 1);
+ other.add_value(100);
+ state.merge(other);
+ expect_digest_samples(state, QUANTILE_STATE_EXPLICIT_NUM + 1, 0, 100);
+}
+
TEST(QuantileStateTest, SelfMergeAndSharedSourceMerge) {
for (int count : {2, 4096}) {
auto state = constant_state(10, count);
diff --git
a/regression-test/data/query_p0/sql_functions/quantile_state_functions/test_quantile_state_explicit_boundary.out
b/regression-test/data/query_p0/sql_functions/quantile_state_functions/test_quantile_state_explicit_boundary.out
new file mode 100644
index 00000000000..12546dfd2f2
--- /dev/null
+++
b/regression-test/data/query_p0/sql_functions/quantile_state_functions/test_quantile_state_explicit_boundary.out
@@ -0,0 +1,22 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !partial_extrema --
+0 0 0 0
+0 1 100 100
+1 0 100 100
+1 1 0 0
+2 0 0 0
+2 1 0 100
+
+-- !merged_extrema --
+0 0 100
+1 0 100
+2 0 100
+
+-- !ordered_merge_extrema --
+0 0 0 0
+0 1 0 100
+1 0 100 100
+1 1 0 100
+2 0 0 0
+2 1 0 100
+
diff --git
a/regression-test/suites/query_p0/sql_functions/quantile_state_functions/test_quantile_state_explicit_boundary.groovy
b/regression-test/suites/query_p0/sql_functions/quantile_state_functions/test_quantile_state_explicit_boundary.groovy
new file mode 100644
index 00000000000..94fae7ff8fe
--- /dev/null
+++
b/regression-test/suites/query_p0/sql_functions/quantile_state_functions/test_quantile_state_explicit_boundary.groovy
@@ -0,0 +1,70 @@
+// 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_quantile_state_explicit_boundary") {
+ sql "DROP TABLE IF EXISTS test_quantile_state_boundary_partials"
+ sql """
+ CREATE TABLE test_quantile_state_boundary_partials (
+ layout_id INT NOT NULL,
+ part INT NOT NULL,
+ s QUANTILE_STATE QUANTILE_UNION NOT NULL
+ ) AGGREGATE KEY(layout_id, part)
+ DISTRIBUTED BY HASH(layout_id) BUCKETS 1
+ PROPERTIES("replication_num" = "1")
+ """
+
+ // The same 2048 zeros and singleton 100 are grouped three ways:
+ // EXPLICIT + SINGLE, SINGLE + EXPLICIT, and EXPLICIT + EXPLICIT.
+ sql """
+ INSERT INTO test_quantile_state_boundary_partials
+ SELECT 0, CAST(number = 2048 AS INT),
+ quantile_union(to_quantile_state(IF(number = 2048, 100.0, 0.0),
2048))
+ FROM numbers("number" = "2049") GROUP BY 2
+ """
+ sql """
+ INSERT INTO test_quantile_state_boundary_partials
+ SELECT 1, CAST(number != 2048 AS INT),
+ quantile_union(to_quantile_state(IF(number = 2048, 100.0, 0.0),
2048))
+ FROM numbers("number" = "2049") GROUP BY 2
+ """
+ sql """
+ INSERT INTO test_quantile_state_boundary_partials
+ SELECT 2, CAST(number >= 2047 AS INT),
+ quantile_union(to_quantile_state(IF(number = 2048, 100.0, 0.0),
2048))
+ FROM numbers("number" = "2049") GROUP BY 2
+ """
+
+ qt_partial_extrema """
+ SELECT layout_id, part, quantile_percent(s, 0), quantile_percent(s, 1)
+ FROM test_quantile_state_boundary_partials ORDER BY layout_id, part
+ """
+ qt_merged_extrema """
+ SELECT layout_id, quantile_percent(quantile_union(s), 0),
+ quantile_percent(quantile_union(s), 1)
+ FROM test_quantile_state_boundary_partials GROUP BY layout_id ORDER BY
layout_id
+ """
+ // Window ordering guarantees both directions of the boundary conversion.
+ qt_ordered_merge_extrema """
+ SELECT layout_id, part, quantile_percent(s, 0), quantile_percent(s, 1)
+ FROM (
+ SELECT layout_id, part, quantile_union(s) OVER (
+ PARTITION BY layout_id ORDER BY part
+ ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS s
+ FROM test_quantile_state_boundary_partials
+ ) t ORDER BY layout_id, part
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
