-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[opt](Nereids) aggregate function sum support string type as parameter #49954
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
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
d6bae2e to
5e471a0
Compare
|
run buildall |
TPC-H: Total hot run time: 34186 ms |
TPC-DS: Total hot run time: 187381 ms |
ClickBench: Total hot run time: 31.02 s |
5e471a0 to
f9567af
Compare
|
run buildall |
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.
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
Files not reviewed (1)
- regression-test/suites/nereids_function_p0/agg_function/agg.groovy: Language not supported
Comments suppressed due to low confidence (2)
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Sum0.java:124
- The searchSignature method does not explicitly handle string-like types. Consider adding an explicit else-if branch for string-like parameters to ensure consistent signature resolution for inputs that are cast from strings.
if (getArgument(0).getDataType() instanceof NullType) {
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/Sum.java:126
- Similar to Sum0, the searchSignature method in Sum does not explicitly address string-like types. Adding a dedicated branch for string-like inputs could improve clarity and maintain consistent type handling.
if (getArgument(0).getDataType() instanceof NullType) {
TPC-H: Total hot run time: 34500 ms |
TPC-DS: Total hot run time: 186316 ms |
ClickBench: Total hot run time: 30.87 s |
f9567af to
910efa0
Compare
morningman
left a comment
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.
LGTM
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
run buildall |
TPC-H: Total hot run time: 34331 ms |
TPC-DS: Total hot run time: 192075 ms |
ClickBench: Total hot run time: 31.37 s |
will cast string to double
910efa0 to
efdd39f
Compare
|
run buildall |
TPC-H: Total hot run time: 34163 ms |
TPC-DS: Total hot run time: 193380 ms |
ClickBench: Total hot run time: 31.33 s |
|
PR approved by at least one committer and no changes requested. |
apache#49954) ### What problem does this PR solve? will cast string to double ### Release note aggregate function sum and avg support string type as parameter
#49954) ### What problem does this PR solve? will cast string to double ### Release note aggregate function sum and avg support string type as parameter
#49954) ### What problem does this PR solve? will cast string to double ### Release note aggregate function sum and avg support string type as parameter
apache#4137) pick from apache#49954 picked to tmp branch tmp-selectdb-cloud-4.0.4.5-ave by apache#3936 Co-authored-by: starocean999 <lichi@selectdb.com>
What problem does this PR solve?
will cast string to double
Release note
aggregate function sum support string type as parameter
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)