zhiqiang-hhhh commented on code in PR #34391:
URL: https://github.com/apache/doris/pull/34391#discussion_r1590516413


##########
be/src/vec/functions/round.h:
##########
@@ -696,49 +679,134 @@ class FunctionRounding : public IFunction {
         return Status::OK();
     }
 
-    ColumnNumbers get_arguments_that_are_always_constant() const override { 
return {1}; }
-
+    //// We moved and optimized the execute_impl logic of function_truncate.h 
from PR#32746,
+    //// as well as make it suitable for all functions.
     Status execute_impl(FunctionContext* context, Block& block, const 
ColumnNumbers& arguments,
-                        size_t result, size_t /*input_rows_count*/) const 
override {
-        const ColumnWithTypeAndName& column = 
block.get_by_position(arguments[0]);
-        Int16 scale_arg = 0;
-        if (arguments.size() == 2) {
-            RETURN_IF_ERROR(get_scale_arg(block.get_by_position(arguments[1]), 
&scale_arg));
-        }
-
+                        size_t result, size_t input_rows_count) const override 
{
+        const ColumnWithTypeAndName& column_general = 
block.get_by_position(arguments[0]);
+        const auto* col_general = column_general.column.get();
         ColumnPtr res;
+
+        /// potential argument types(optimized from four types in previous PR 
to two):
+        /// if the SECOND argument is MISSING(would be considered as ZERO 
const) or CONST, then we have 1st type:
+        ///    1. func(Column), func(ColumnConst), func(Column, ColumnConst), 
func(ColumnConst, ColumnConst)
+        /// otherwise, the SECOND arugment is COLUMN, we have another type:
+        ///    2. func(Column, Column), func(ColumnConst, Column)
+
         auto call = [&](const auto& types) -> bool {
             using Types = std::decay_t<decltype(types)>;
             using DataType = typename Types::LeftType;
 
             if constexpr (IsDataTypeNumber<DataType> || 
IsDataTypeDecimal<DataType>) {
                 using FieldType = typename DataType::FieldType;
-                res = Dispatcher<FieldType, rounding_mode, 
tie_breaking_mode>::apply_vec_const(
-                        column.column.get(), scale_arg);
+                // the SECOND argument is MISSING or CONST
+                if (arguments.size() == 1 ||
+                    
is_column_const(*block.get_by_position(arguments[1]).column)) {
+                    Int16 scale_arg = 0;
+                    if (arguments.size() == 2) {
+                        RETURN_IF_ERROR(
+                                
get_scale_arg(block.get_by_position(arguments[1]), &scale_arg));
+                    }
+                    res = Dispatcher<FieldType, rounding_mode, 
tie_breaking_mode>::apply_vec_const(

Review Comment:
   If column_general is const, we will get wrong result if we use this method.



-- 
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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to