Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet merged PR #58666: URL: https://github.com/apache/doris/pull/58666 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
github-actions[bot] commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3635217001 PR approved by at least one committer and no changes requested. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3619649287 # BE Regression && UT Coverage Report Increment line coverage `90.91% (30/33)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/58666_7ec26c094bfb22e1813ef8a18d12a799e56f570f_merge/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/58666_7ec26c094bfb22e1813ef8a18d12a799e56f570f_merge/report/index.html) | Category | Coverage | |---|| | Function Coverage | 72.15% (24750/34303) | | Line Coverage | 58.86% (259619/441046) | | Region Coverage | 53.94% (216422/401211) | | Branch Coverage | 55.30% (92227/166776) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3619481840 # BE UT Coverage Report Increment line coverage `45.45% (15/33)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/7ec26c094bfb22e1813ef8a18d12a799e56f570f_7ec26c094bfb22e1813ef8a18d12a799e56f570f/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/7ec26c094bfb22e1813ef8a18d12a799e56f570f_7ec26c094bfb22e1813ef8a18d12a799e56f570f/report/index.html) | Category | Coverage | |---|| | Function Coverage | 53.24% (18625/34981) | | Line Coverage | 38.93% (171852/441412) | | Region Coverage | 33.56% (133047/396397) | | Branch Coverage | 34.50% (57223/165877) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3619411897 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610991148 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
HappenLee commented on code in PR #58666:
URL: https://github.com/apache/doris/pull/58666#discussion_r2588142131
##
be/src/vec/aggregate_functions/aggregate_function_avg.h:
##
@@ -112,31 +117,41 @@ class AggregateFunctionAvg final
UnaryExpression,
NullableAggregateFunction {
public:
-using ResultType = std::conditional_t<
-T == TYPE_DECIMALV2, Decimal128V2,
-std::conditional_t>;
-using ResultDataType = std::conditional_t<
-T == TYPE_DECIMALV2, DataTypeDecimalV2,
-std::conditional_t, DataTypeFloat64>>;
+using ResultDataType =
+std::conditional_t::DataType,
+ DataTypeFloat64>;
using ColVecType = typename PrimitiveTypeTraits::ColumnType;
-using ColVecResult = std::conditional_t<
-T == TYPE_DECIMALV2, ColumnDecimal128V2,
-std::conditional_t, ColumnFloat64>>;
+using ColVecResult =
+std::conditional_t::ColumnType,
+ ColumnFloat64>;
// The result calculated by PercentileApprox is an approximate value,
// so the underlying storage uses float. The following calls will involve
// an implicit cast to float.
using DataType = typename Data::ResultType;
+using ResultType = std::conditional_t;
/// ctor for native types
+// consistent with
fe/fe-common/src/main/java/org/apache/doris/catalog/ScalarType.java
+static constexpr uint32_t DEFAULT_MIN_AVG_DECIMAL128_SCALE = 4;
AggregateFunctionAvg(const DataTypes& argument_types_)
: IAggregateFunctionDataHelper>(argument_types_),
- scale(get_decimal_scale(*argument_types_[0])) {}
+ input_scale(get_decimal_scale(*argument_types_[0])),
+ output_scale(std::max(DEFAULT_MIN_AVG_DECIMAL128_SCALE,
input_scale)) {
+if constexpr (is_decimal(T)) {
+multiplier =
+
ResultType(ResultDataType::get_scale_multiplier(output_scale - input_scale));
+}
+}
String get_name() const override { return "avg"; }
DataTypePtr get_return_type() const override {
if constexpr (is_decimal(T)) {
-return
std::make_shared(ResultDataType::max_precision(), scale);
+return std::make_shared(
Review Comment:
we should calc the res in constructor and keep be member var
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
HappenLee commented on code in PR #58666: URL: https://github.com/apache/doris/pull/58666#discussion_r2588134874 ## be/src/vec/aggregate_functions/aggregate_function_avg.h: ## @@ -346,7 +365,9 @@ class AggregateFunctionAvg final } private: -UInt32 scale; +uint32_t input_scale; Review Comment: better be a tmp var -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
github-actions[bot] commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610928750 PR approved by anyone and no changes requested. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
github-actions[bot] commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610928605 PR approved by at least one committer and no changes requested. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610856494 # BE Regression && UT Coverage Report Increment line coverage `90.91% (30/33)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/58666_a2b39a41e883ec37e15a775e7a4b0f3c4f77e490_merge/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/58666_a2b39a41e883ec37e15a775e7a4b0f3c4f77e490_merge/report/index.html) | Category | Coverage | |---|| | Function Coverage | 72.20% (24730/34250) | | Line Coverage | 58.89% (259503/440689) | | Region Coverage | 53.74% (215561/401138) | | Branch Coverage | 55.29% (92178/166707) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610547488 # BE UT Coverage Report Increment line coverage `45.45% (15/33)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/a2b39a41e883ec37e15a775e7a4b0f3c4f77e490_a2b39a41e883ec37e15a775e7a4b0f3c4f77e490/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/a2b39a41e883ec37e15a775e7a4b0f3c4f77e490_a2b39a41e883ec37e15a775e7a4b0f3c4f77e490/report/index.html) | Category | Coverage | |---|| | Function Coverage | 53.39% (18651/34933) | | Line Coverage | 39.04% (172207/441079) | | Region Coverage | 33.65% (133354/396324) | | Branch Coverage | 34.61% (57389/165808) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610338057 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3610058963 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3606958434 # FE Regression Coverage Report Increment line coverage ` 100% (0/0)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/58666_3a29c9405d862ad159f134dc9023cb96a55c5824_merge_fe/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/58666_3a29c9405d862ad159f134dc9023cb96a55c5824_merge_fe/report/index.html) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3606903457 # BE Regression && UT Coverage Report Increment line coverage `90.91% (30/33)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/58666_3a29c9405d862ad159f134dc9023cb96a55c5824_merge/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/58666_3a29c9405d862ad159f134dc9023cb96a55c5824_merge/report/index.html) | Category | Coverage | |---|| | Function Coverage | 72.15% (24710/34247) | | Line Coverage | 58.83% (259253/440661) | | Region Coverage | 53.73% (215449/400969) | | Branch Coverage | 55.24% (92067/10) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3606605853 # BE UT Coverage Report Increment line coverage `45.45% (15/33)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/3a29c9405d862ad159f134dc9023cb96a55c5824_3a29c9405d862ad159f134dc9023cb96a55c5824/increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/3a29c9405d862ad159f134dc9023cb96a55c5824_3a29c9405d862ad159f134dc9023cb96a55c5824/report/index.html) | Category | Coverage | |---|| | Function Coverage | 53.36% (18638/34930) | | Line Coverage | 39.01% (172072/441053) | | Region Coverage | 33.63% (133226/396158) | | Branch Coverage | 34.59% (57330/165763) | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3606374201 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3606156361 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3605868455 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3605560943 # FE UT Coverage Report Increment line coverage ` 100% (0/0)` :tada: [Increment coverage report](http://coverage.selectdb-in.cc/coverage/58666_8652b5ea4658e3306c695e118fcb240e34c671f9/fe_increment_report/index.html) [Complete coverage report](http://coverage.selectdb-in.cc/coverage/58666_8652b5ea4658e3306c695e118fcb240e34c671f9/fe_report/index.html) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3605358140 run buildall -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
[PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
BiteThet opened a new pull request, #58666: URL: https://github.com/apache/doris/pull/58666 ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test - [ ] 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 - Behavior changed: - [ ] No. - [ ] Yes. - Does this need documentation? - [ ] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] [Improvement](function) reduce cast of input arg from decimal avg [doris]
hello-stephen commented on PR #58666: URL: https://github.com/apache/doris/pull/58666#issuecomment-3605357982 Thank you for your contribution to Apache Doris. Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR). Please clearly describe your PR: 1. What problem was fixed (it's best to include specific error reporting information). How it was fixed. 2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be. 3. What features were added. Why was this function added? 4. Which code was refactored and why was this part of the code refactored? 5. Which functions were optimized and what is the difference before and after the optimization? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
