You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/05/06 04:05:22 UTC

[GitHub] [incubator-doris] HappenLee commented on a diff in pull request #9376: [BUG][Vectorized] fix `replace_if_not_null` in vectorized compaction

HappenLee commented on code in PR #9376:
URL: https://github.com/apache/incubator-doris/pull/9376#discussion_r866474902


##########
be/src/vec/aggregate_functions/aggregate_function_window.h:
##########
@@ -408,61 +489,78 @@ class WindowFunctionData final
 };
 
 template <template <typename> class AggregateFunctionTemplate, template <typename> class Data,
-          bool is_nullable, bool is_copy = false>
+          bool is_nullable, bool is_copy = false, bool init_with_null = false>
 static IAggregateFunction* create_function_single_value(const String& name,
                                                         const DataTypes& argument_types,
                                                         const Array& parameters) {
     using StoreType = std::conditional_t<is_copy, CopiedValue, Value>;
 
     assert_arity_at_most<3>(name, argument_types);
 
-    auto type = argument_types[0].get();
-    if (type->is_nullable()) {
-        type = assert_cast<const DataTypeNullable*>(type)->get_nested_type().get();
-    }
+    auto type = remove_nullable(argument_types[0]);
     WhichDataType which(*type);
 
-#define DISPATCH(TYPE)                        \
-    if (which.idx == TypeIndex::TYPE)         \
-        return new AggregateFunctionTemplate< \
-                Data<LeadAndLagData<TYPE, is_nullable, false, StoreType>>>(argument_types);
+#define DISPATCH(TYPE)                                                                      \
+    if (which.idx == TypeIndex::TYPE)                                                       \
+        return new AggregateFunctionTemplate<                                               \
+                Data<LeadAndLagData<TYPE, is_nullable, false, StoreType, init_with_null>>>( \
+                argument_types);
     FOR_NUMERIC_TYPES(DISPATCH)
 #undef DISPATCH
 
     if (which.is_decimal()) {
         return new AggregateFunctionTemplate<
-                Data<LeadAndLagData<Int128, is_nullable, false, StoreType>>>(argument_types);
+                Data<LeadAndLagData<Int128, is_nullable, false, StoreType, init_with_null>>>(
+                argument_types);
     }
     if (which.is_date_or_datetime()) {
         return new AggregateFunctionTemplate<
-                Data<LeadAndLagData<Int64, is_nullable, false, StoreType>>>(argument_types);
+                Data<LeadAndLagData<Int64, is_nullable, false, StoreType, init_with_null>>>(
+                argument_types);
     }
     if (which.is_string_or_fixed_string()) {
         return new AggregateFunctionTemplate<
-                Data<LeadAndLagData<StringRef, is_nullable, true, StoreType>>>(argument_types);
+                Data<LeadAndLagData<StringRef, is_nullable, true, StoreType, init_with_null>>>(
+                argument_types);
     }
     DCHECK(false) << "with unknowed type, failed in  create_aggregate_function_leadlag";
     return nullptr;
 }
 
-template <bool is_nullable, bool is_copy>
+template <bool is_nullable, bool is_copy, bool replace_if_not_null = false>
 AggregateFunctionPtr create_aggregate_function_first(const std::string& name,
                                                      const DataTypes& argument_types,
                                                      const Array& parameters,
                                                      bool result_is_nullable) {
-    return AggregateFunctionPtr(
-            create_function_single_value<WindowFunctionData, WindowFunctionFirstData, is_nullable,
-                                         is_copy>(name, argument_types, parameters));
+    if constexpr (replace_if_not_null) {
+        return AggregateFunctionPtr(
+                create_function_single_value<WindowFunctionData, WindowFunctionFirstNonNullData,
+                                             is_nullable, is_copy, true>(name, argument_types,
+                                                                         parameters));
+    } else {
+        return AggregateFunctionPtr(
+                create_function_single_value<WindowFunctionData, WindowFunctionFirstData,
+                                             is_nullable, is_copy>(name, argument_types,
+                                                                   parameters));
+    }
 }
 
-template <bool is_nullable, bool is_copy>
+template <bool is_nullable, bool is_copy, bool replace_if_not_null = false>
 AggregateFunctionPtr create_aggregate_function_last(const std::string& name,

Review Comment:
   maybe should split the register `last_value` and `last_not_null_value`



##########
be/src/vec/aggregate_functions/aggregate_function_window.h:
##########
@@ -192,6 +194,9 @@ struct LeadAndLagData {
         _default_value.reset();
         _is_init = false;
         _has_value = false;
+        if constexpr (init_with_null) {

Review Comment:
   the logic only use in child class, should not do in parent.



-- 
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-unsubscribe@doris.apache.org

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


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