-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Feature] Add width_bucket function #22787
base: main
Are you sure you want to change the base?
Conversation
docs/sql-reference/sql-functions/math-functions/width_bucket.md
Outdated
Show resolved
Hide resolved
run all |
@Mergifyio rebase |
✅ Branch has been successfully rebased |
95088ac
to
c90ab51
Compare
Please fix the problems based on the ci reports. At least those marked required. |
c90ab51
to
a4234b5
Compare
[FE PR Coverage Check]😍 pass : 0 / 0 (0%) |
done |
LGTM |
gensrc/script/functions.py
Outdated
[10324, "width_bucket", "BIGINT", ["TINYINT", "TINYINT", "TINYINT", "BIGINT"], "MathFunctions::width_bucket<TYPE_TINYINT>"], | ||
[103241, "width_bucket", "BIGINT", ["SMALLINT", "SMALLINT", "SMALLINT", "BIGINT"], "MathFunctions::width_bucket<TYPE_SMALLINT>"], | ||
[103242, "width_bucket", "BIGINT", ["INT", "INT", "INT", "BIGINT"], "MathFunctions::width_bucket<TYPE_INT>"], | ||
[103243, "width_bucket", "BIGINT", ["BIGINT", "BIGINT", "BIGINT", "BIGINT"], "MathFunctions::width_bucket<TYPE_BIGINT>"], | ||
[103244, "width_bucket", "BIGINT", ["FLOAT", "FLOAT", "FLOAT", "BIGINT"], "MathFunctions::width_bucket<TYPE_FLOAT>"], | ||
[103245, "width_bucket", "BIGINT", ["DOUBLE", "DOUBLE", "DOUBLE", "BIGINT"], "MathFunctions::width_bucket<TYPE_DOUBLE>"], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about decimal type?
select width_bucket(5.35, 0.024, 10.06, 5); | ||
-- result: | ||
3 | ||
-- !result | ||
select width_bucket(0, 0.024, 10.06, 5); | ||
-- result: | ||
0 | ||
-- !result | ||
select width_bucket(11, 0.024, 10.06, 5); | ||
-- result: | ||
6 | ||
-- !result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please some cases for null input
be/src/exprs/math_functions.h
Outdated
auto range_size = max_value - min_value; | ||
auto dist_from_min = input_value - min_value; | ||
int64_t ret = dist_from_min / (range_size / bucket_num); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shoud /
return double ? otherwise range_size/bucket_num would return 0.
what if max_value <= min_value?
a4234b5
to
4250da8
Compare
8154ca7
to
81fe5b3
Compare
@liuyehcf PTAL, I have fixed all issues above. And the CI failures seems not relevant to my pr. |
Signed-off-by: TBCCC <tbccc55555@outlook.com>
Signed-off-by: TBCCC <tbccc55555@outlook.com>
Signed-off-by: TBCCC <tbccc55555@outlook.com>
Signed-off-by: TBCCC <tbccc55555@outlook.com>
Signed-off-by: TBCCC <tbccc55555@outlook.com>
Signed-off-by: TBCCC <tbccc55555@outlook.com>
Signed-off-by: TBCCC <tbccc55555@outlook.com>
2772bbb
to
83877d7
Compare
Kudos, SonarCloud Quality Gate passed!
|
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]❌ fail : 0 / 40 (00.00%) file detail
|
What type of PR is this:
Which issues of this PR fixes:
Fixes #12990
Problem Summary(Required):
Checklist:
Bugfix cherry-pick branch check: