thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-11-21 15:12:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #16774 from ucasFL/agg-func-setting-null-for-empty

Browse Source 
fix Aggregation Function not found error after #16123
This commit is contained in:
alexey-milovidov 2020-11-12 13:50:15 +03:00 committed by GitHub
commit 6e62108606
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 132 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
src/AggregateFunctions/AggregateFunctionFactory.cpp
View File

@ -56,11 +56,7 @@ static DataTypes convertLowCardinalityTypesToNested(const DataTypes & types)
}
AggregateFunctionPtr AggregateFunctionFactory::get(
const String & name,
const DataTypes & argument_types,
const Array & parameters,
AggregateFunctionProperties & out_properties,
int recursion_level) const
const String & name, const DataTypes & argument_types, const Array & parameters, AggregateFunctionProperties & out_properties) const
{
auto type_without_low_cardinality = convertLowCardinalityTypesToNested(argument_types);
@ -81,11 +77,11 @@ AggregateFunctionPtr AggregateFunctionFactory::get(
[](const auto & type) { return type->onlyNull(); });
AggregateFunctionPtr nested_function = getImpl(
name, nested_types, nested_parameters, out_properties, has_null_arguments, recursion_level);
name, nested_types, nested_parameters, out_properties, has_null_arguments);
return combinator->transformAggregateFunction(nested_function, out_properties, type_without_low_cardinality, parameters);
}
auto res = getImpl(name, type_without_low_cardinality, parameters, out_properties, false, recursion_level);
auto res = getImpl(name, type_without_low_cardinality, parameters, out_properties, false);
if (!res)
throw Exception("Logical error: AggregateFunctionFactory returned nullptr", ErrorCodes::LOGICAL_ERROR);
return res;
@ -97,8 +93,7 @@ AggregateFunctionPtr AggregateFunctionFactory::getImpl(
const DataTypes & argument_types,
const Array & parameters,
AggregateFunctionProperties & out_properties,
bool has_null_arguments,
int recursion_level) const
bool has_null_arguments) const
{
String name = getAliasToOrName(name_param);
Value found;
@ -108,13 +103,9 @@ AggregateFunctionPtr AggregateFunctionFactory::getImpl(
{
found = it->second;
}
/// Find by case-insensitive name.
/// Combinators cannot apply for case insensitive (SQL-style) aggregate function names. Only for native names.
else if (recursion_level == 0)
{
if (auto jt = case_insensitive_aggregate_functions.find(Poco::toLower(name)); jt != case_insensitive_aggregate_functions.end())
found = jt->second;
}
if (auto jt = case_insensitive_aggregate_functions.find(Poco::toLower(name)); jt != case_insensitive_aggregate_functions.end())
found = jt->second;
if (found.creator)
{
@ -140,7 +131,7 @@ AggregateFunctionPtr AggregateFunctionFactory::getImpl(
DataTypes nested_types = combinator->transformArguments(argument_types);
Array nested_parameters = combinator->transformParameters(parameters);
AggregateFunctionPtr nested_function = get(nested_name, nested_types, nested_parameters, out_properties, recursion_level + 1);
AggregateFunctionPtr nested_function = get(nested_name, nested_types, nested_parameters, out_properties);
return combinator->transformAggregateFunction(nested_function, out_properties, argument_types, parameters);
}
@ -162,7 +153,7 @@ AggregateFunctionPtr AggregateFunctionFactory::tryGet(
}
std::optional<AggregateFunctionProperties> AggregateFunctionFactory::tryGetPropertiesImpl(const String & name_param, int recursion_level) const
std::optional<AggregateFunctionProperties> AggregateFunctionFactory::tryGetPropertiesImpl(const String & name_param) const
{
String name = getAliasToOrName(name_param);
Value found;
@ -172,13 +163,9 @@ std::optional<AggregateFunctionProperties> AggregateFunctionFactory::tryGetPrope
{
found = it->second;
}
/// Find by case-insensitive name.
/// Combinators cannot apply for case insensitive (SQL-style) aggregate function names. Only for native names.
else if (recursion_level == 0)
{
if (auto jt = case_insensitive_aggregate_functions.find(Poco::toLower(name)); jt != case_insensitive_aggregate_functions.end())
found = jt->second;
}
if (auto jt = case_insensitive_aggregate_functions.find(Poco::toLower(name)); jt != case_insensitive_aggregate_functions.end())
found = jt->second;
if (found.creator)
return found.properties;
@ -195,7 +182,7 @@ std::optional<AggregateFunctionProperties> AggregateFunctionFactory::tryGetPrope
String nested_name = name.substr(0, name.size() - combinator->getName().size());
/// NOTE: It's reasonable to also allow to transform properties by combinator.
return tryGetPropertiesImpl(nested_name, recursion_level + 1);
return tryGetPropertiesImpl(nested_name);
}
return {};
@ -204,21 +191,21 @@ std::optional<AggregateFunctionProperties> AggregateFunctionFactory::tryGetPrope
std::optional<AggregateFunctionProperties> AggregateFunctionFactory::tryGetProperties(const String & name) const
{
return tryGetPropertiesImpl(name, 0);
return tryGetPropertiesImpl(name);
}
bool AggregateFunctionFactory::isAggregateFunctionName(const String & name, int recursion_level) const
bool AggregateFunctionFactory::isAggregateFunctionName(const String & name) const
{
if (aggregate_functions.count(name) || isAlias(name))
return true;
String name_lowercase = Poco::toLower(name);
if (recursion_level == 0 && (case_insensitive_aggregate_functions.count(name_lowercase) || isAlias(name_lowercase)))
if (case_insensitive_aggregate_functions.count(name_lowercase) || isAlias(name_lowercase))
return true;
if (AggregateFunctionCombinatorPtr combinator = AggregateFunctionCombinatorFactory::instance().tryFindSuffix(name))
return isAggregateFunctionName(name.substr(0, name.size() - combinator->getName().size()), recursion_level + 1);
return isAggregateFunctionName(name.substr(0, name.size() - combinator->getName().size()));
return false;
}

14
src/AggregateFunctions/AggregateFunctionFactory.h
View File

@ -59,12 +59,11 @@ public:
CaseSensitiveness case_sensitiveness = CaseSensitive);
/// Throws an exception if not found.
AggregateFunctionPtr get(
const String & name,
AggregateFunctionPtr
get(const String & name,
const DataTypes & argument_types,
const Array & parameters,
AggregateFunctionProperties & out_properties,
int recursion_level = 0) const;
AggregateFunctionProperties & out_properties) const;
/// Returns nullptr if not found.
AggregateFunctionPtr tryGet(
@ -76,7 +75,7 @@ public:
/// Get properties if the aggregate function exists.
std::optional<AggregateFunctionProperties> tryGetProperties(const String & name) const;
bool isAggregateFunctionName(const String & name, int recursion_level = 0) const;
bool isAggregateFunctionName(const String & name) const;
private:
AggregateFunctionPtr getImpl(
@ -84,10 +83,9 @@ private:
const DataTypes & argument_types,
const Array & parameters,
AggregateFunctionProperties & out_properties,
bool has_null_arguments,
int recursion_level) const;
bool has_null_arguments) const;
std::optional<AggregateFunctionProperties> tryGetPropertiesImpl(const String & name, int recursion_level) const;
std::optional<AggregateFunctionProperties> tryGetPropertiesImpl(const String & name) const;
private:
using AggregateFunctions = std::unordered_map<String, Value>;

8
tests/queries/0_stateless/01559_aggregate_null_for_empty_fix.reference Normal file
View File

@ -0,0 +1,8 @@
0
0
0
0
-1
-1
-1
\N

91
tests/queries/0_stateless/01559_aggregate_null_for_empty_fix.sql Normal file
View File

@ -0,0 +1,91 @@
SELECT MAX(aggr)
FROM
(
SELECT MAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT MAX(-1) AS aggr
FROM system.one
WHERE 1
);
SELECT MaX(aggr)
FROM
(
SELECT mAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT MAx(-1) AS aggr
FROM system.one
WHERE 1
);
SELECT MaX(aggr)
FROM
(
SELECT mAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT max(-1) AS aggr
FROM system.one
WHERE 1
);
SELECT MaX(aggr)
FROM
(
SELECT mAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT max(-1) AS aggr
FROM system.one
WHERE not 1
);
SET aggregate_functions_null_for_empty=1;
SELECT MAX(aggr)
FROM
(
SELECT MAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT MAX(-1) AS aggr
FROM system.one
WHERE 1
);
SELECT MaX(aggr)
FROM
(
SELECT mAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT MAx(-1) AS aggr
FROM system.one
WHERE 1
);
SELECT MaX(aggr)
FROM
(
SELECT mAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT max(-1) AS aggr
FROM system.one
WHERE 1
);
SELECT MaX(aggr)
FROM
(
SELECT mAX(-1) AS aggr
FROM system.one
WHERE NOT 1
UNION ALL
SELECT max(-1) AS aggr
FROM system.one
WHERE not 1
);

5
tests/queries/0_stateless/01560_cancel_agg_func_combinator_native_name_constraint.reference Normal file
View File

@ -0,0 +1,5 @@
1
1
1
1
\N

5
tests/queries/0_stateless/01560_cancel_agg_func_combinator_native_name_constraint.sql Normal file
View File

@ -0,0 +1,5 @@
SELECT Sum(1);
SELECT SumOrNull(1);
SELECT SUMOrNull(1);
SELECT SUMOrNullIf(1, 1);
SELECT SUMOrNullIf(1, 0);