diff --git a/src/AggregateFunctions/AggregateFunctionIf.cpp b/src/AggregateFunctions/AggregateFunctionIf.cpp index 276abb90920..5e7e3844956 100644 --- a/src/AggregateFunctions/AggregateFunctionIf.cpp +++ b/src/AggregateFunctions/AggregateFunctionIf.cpp @@ -53,17 +53,35 @@ class AggregateFunctionIfNullUnary final private: size_t num_arguments; + /// The name of the nested function, including combinators (i.e. *If) + /// + /// getName() from the nested_function cannot be used because in case of *If combinator + /// with Nullable argument nested_function will point to the function w/o combinator. + /// (I.e. sumIf(Nullable, 1) -> sum()), and distributed query processing will fail. + /// + /// And nested_function cannot point to the function with *If since + /// due to optimization in the add() which pass only one column with the result, + /// and so AggregateFunctionIf::add() cannot be called this way + /// (it write to the last argument -- num_arguments-1). + /// + /// And to avoid extra level of indirection, the name of function is cached: + /// + /// AggregateFunctionIfNullUnary::add -> [ AggregateFunctionIf::add -> ] AggregateFunctionSum::add + String name; + using Base = AggregateFunctionNullBase>; public: String getName() const override { - return Base::getName(); + return name; } - AggregateFunctionIfNullUnary(AggregateFunctionPtr nested_function_, const DataTypes & arguments, const Array & params) - : Base(std::move(nested_function_), arguments, params), num_arguments(arguments.size()) + AggregateFunctionIfNullUnary(const String & name_, AggregateFunctionPtr nested_function_, const DataTypes & arguments, const Array & params) + : Base(std::move(nested_function_), arguments, params) + , num_arguments(arguments.size()) + , name(name_) { if (num_arguments == 0) throw Exception("Aggregate function " + getName() + " require at least one argument", @@ -174,14 +192,14 @@ AggregateFunctionPtr AggregateFunctionIf::getOwnNullAdapter( { if (return_type_is_nullable) { - return std::make_shared>(nested_func, arguments, params); + return std::make_shared>(nested_function->getName(), nested_func, arguments, params); } else { if (serialize_flag) - return std::make_shared>(nested_func, arguments, params); + return std::make_shared>(nested_function->getName(), nested_func, arguments, params); else - return std::make_shared>(nested_func, arguments, params); + return std::make_shared>(nested_function->getName(), nested_func, arguments, params); } } else diff --git a/tests/queries/0_stateless/01642_if_nullable_regression.reference b/tests/queries/0_stateless/01642_if_nullable_regression.reference new file mode 100644 index 00000000000..66430601da2 --- /dev/null +++ b/tests/queries/0_stateless/01642_if_nullable_regression.reference @@ -0,0 +1,5 @@ +\N +\N +\N +0 +90 diff --git a/tests/queries/0_stateless/01642_if_nullable_regression.sql b/tests/queries/0_stateless/01642_if_nullable_regression.sql new file mode 100644 index 00000000000..9b307cf667a --- /dev/null +++ b/tests/queries/0_stateless/01642_if_nullable_regression.sql @@ -0,0 +1,7 @@ +SELECT sumIf(dummy, dummy) FROM remote('127.0.0.{1,2}', view(SELECT cast(Null AS Nullable(UInt8)) AS dummy FROM system.one)); +SELECT sumIf(dummy, 1) FROM remote('127.0.0.{1,2}', view(SELECT cast(Null AS Nullable(UInt8)) AS dummy FROM system.one)); +-- Before #16610 it returns 0 while with this patch it will return NULL +SELECT sumIf(dummy, dummy) FROM remote('127.0.0.{1,2}', view(SELECT cast(dummy AS Nullable(UInt8)) AS dummy FROM system.one)); +SELECT sumIf(dummy, 1) FROM remote('127.0.0.{1,2}', view(SELECT cast(dummy AS Nullable(UInt8)) AS dummy FROM system.one)); + +SELECT sumIf(n, 1) FROM remote('127.0.0.{1,2}', view(SELECT cast(* AS Nullable(UInt8)) AS n FROM system.numbers limit 10))