-
Notifications
You must be signed in to change notification settings - Fork 757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: histogram aggregate function #14839
feat: histogram aggregate function #14839
Conversation
d1ac941
to
31e8547
Compare
9441e43
to
fba1126
Compare
Is there anyway to cast |
fba1126
to
4675297
Compare
|
918b413
to
02daad4
Compare
Is there anyway to format |
a7ff579
to
1667ad2
Compare
1667ad2
to
363af45
Compare
= =, I found some small improvements. Should I submit a new PR directly or open an issue? diff --git a/src/query/functions/src/aggregates/aggregate_histogram.rs b/src/query/functions/src/aggregates/aggregate_histogram.rs
index b1935c0a1d..7c0e2c4169 100644
--- a/src/query/functions/src/aggregates/aggregate_histogram.rs
+++ b/src/query/functions/src/aggregates/aggregate_histogram.rs
@@ -145,8 +145,8 @@ where
&buckets
.drain(..)
.map(|raw| Bucket {
- lower: format_scalar(raw.lower.clone()),
- upper: format_scalar(raw.upper.clone()),
+ lower: format_scalar(raw.lower),
+ upper: format_scalar(raw.upper),
ndv: raw.ndv,
count: raw.count,
pre_sum: raw.pre_sum,
@@ -352,7 +352,7 @@ fn calculate_bucket_max_values<T: Ord>(value_map: &BTreeMap<T, u64>, num_buckets
// Assume that the value map is not empty
debug_assert!(!value_map.is_empty());
- // Calculate the total number of values in the map using std::accumulate()
+ // Calculate the total number of values in the map
let total_values = value_map.values().sum();
// If there is only one bucket, then all values will be assigned to that bucket
diff --git a/src/query/sql/src/planner/semantic/type_check.rs b/src/query/sql/src/planner/semantic/type_check.rs
index c691f9a450..d11fe9b734 100644
--- a/src/query/sql/src/planner/semantic/type_check.rs
+++ b/src/query/sql/src/planner/semantic/type_check.rs
@@ -1726,7 +1726,7 @@ impl<'a> TypeChecker<'a> {
} && arg_types[1].is_integer();
if !is_positive_integer {
return Err(ErrorCode::SemanticError(
- "The delimiter of `histogram` must be a constant positive int",
+ "The max_num_buckets of `histogram` must be a constant positive int",
));
}
|
Cool, only a new PR is ok. |
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
Briefly describe what this PR aims to solve. Include background context that will help reviewers understand the purpose of the PR.
This Draft PR is only used to display task disassembly, current progress, and exchange ideas. The quality of the code is currently low. Today I will improve it to a state that can be reviewed.
Tests
Type of change
Progress
RangeIndex::supported_type
date
date_time
decimal
readableFor Reviewers
The implementation refers to doris' histogram function, so these may be helpful:
WIP
This change is