Skip to content

Commit

Permalink
Merge pull request ClickHouse#65519 from KevinyhZou/Fix_least_greast_…
Browse files Browse the repository at this point in the history
…diff

Make functions `least` and `greatest` ignore NULL arguments
  • Loading branch information
rschu1ze authored Nov 28, 2024
2 parents f00b0b0 + e557059 commit 4c36e8f
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 28 deletions.
4 changes: 2 additions & 2 deletions docs/en/sql-reference/functions/other-functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,7 @@ SELECT parseTimeDelta('1yr2mo')

## least

Returns the smaller value of a and b.
Returns the smallest arguments of one or more input arguments. NULL arguments are ignored.

**Syntax**

Expand All @@ -1223,7 +1223,7 @@ least(a, b)

## greatest

Returns the larger value of a and b.
Returns the largest arguments of one or more input arguments. NULL arguments are ignored.

**Syntax**

Expand Down
33 changes: 21 additions & 12 deletions src/Functions/LeastGreatestGeneric.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <Functions/FunctionFactory.h>
#include <base/map.h>


namespace DB
{

Expand Down Expand Up @@ -37,6 +36,7 @@ class FunctionLeastGreatestGeneric : public IFunction
size_t getNumberOfArguments() const override { return 0; }
bool isVariadic() const override { return true; }
bool useDefaultImplementationForConstants() const override { return true; }
bool useDefaultImplementationForNulls() const override { return false; }
bool isSuitableForShortCircuitArgumentsExecution(const DataTypesWithConstInfo & /*arguments*/) const override { return false; }

DataTypePtr getReturnTypeImpl(const DataTypes & types) const override
Expand All @@ -49,31 +49,40 @@ class FunctionLeastGreatestGeneric : public IFunction

ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr & result_type, size_t input_rows_count) const override
{
size_t num_arguments = arguments.size();
if (1 == num_arguments)
if (arguments.size() == 1)
return arguments[0].column;

Columns converted_columns(num_arguments);
for (size_t arg = 0; arg < num_arguments; ++arg)
converted_columns[arg] = castColumn(arguments[arg], result_type)->convertToFullColumnIfConst();
Columns converted_columns;
for (const auto & argument : arguments)
{
if (argument.type->onlyNull())
continue; /// ignore NULL arguments
auto converted_col = castColumn(argument, result_type)->convertToFullColumnIfConst();
converted_columns.emplace_back(converted_col);
}

if (converted_columns.empty())
return arguments[0].column;
else if (converted_columns.size() == 1)
return converted_columns[0];

auto result_column = result_type->createColumn();
result_column->reserve(input_rows_count);

for (size_t row_num = 0; row_num < input_rows_count; ++row_num)
{
size_t best_arg = 0;
for (size_t arg = 1; arg < num_arguments; ++arg)
for (size_t arg = 1; arg < converted_columns.size(); ++arg)
{
auto cmp_result = converted_columns[arg]->compareAt(row_num, row_num, *converted_columns[best_arg], 1);

if constexpr (kind == LeastGreatest::Least)
{
auto cmp_result = converted_columns[arg]->compareAt(row_num, row_num, *converted_columns[best_arg], 1);
if (cmp_result < 0)
best_arg = arg;
}
else
{
auto cmp_result = converted_columns[arg]->compareAt(row_num, row_num, *converted_columns[best_arg], -1);
if (cmp_result > 0)
best_arg = arg;
}
Expand All @@ -86,7 +95,6 @@ class FunctionLeastGreatestGeneric : public IFunction
}
};


template <LeastGreatest kind, typename SpecializedFunction>
class LeastGreatestOverloadResolver : public IFunctionOverloadResolver
{
Expand All @@ -103,6 +111,7 @@ class LeastGreatestOverloadResolver : public IFunctionOverloadResolver
String getName() const override { return name; }
size_t getNumberOfArguments() const override { return 0; }
bool isVariadic() const override { return true; }
bool useDefaultImplementationForNulls() const override { return false; }

FunctionBasePtr buildImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr & return_type) const override
{
Expand All @@ -111,7 +120,7 @@ class LeastGreatestOverloadResolver : public IFunctionOverloadResolver
argument_types.push_back(argument.type);

/// More efficient specialization for two numeric arguments.
if (arguments.size() == 2 && isNumber(removeNullable(arguments[0].type)) && isNumber(removeNullable(arguments[1].type)))
if (arguments.size() == 2 && isNumber(arguments[0].type) && isNumber(arguments[1].type))
return std::make_unique<FunctionToFunctionBaseAdaptor>(SpecializedFunction::create(context), argument_types, return_type);

return std::make_unique<FunctionToFunctionBaseAdaptor>(
Expand All @@ -123,7 +132,7 @@ class LeastGreatestOverloadResolver : public IFunctionOverloadResolver
if (types.empty())
throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, "Function {} cannot be called without arguments", getName());

if (types.size() == 2 && isNumber(removeNullable(types[0])) && isNumber(removeNullable(types[1])))
if (types.size() == 2 && isNumber(types[0]) && isNumber(types[1]))
return SpecializedFunction::create(context)->getReturnTypeImpl(types);

return getLeastSupertype(types);
Expand Down
10 changes: 0 additions & 10 deletions tests/performance/least_greatest.xml

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ z
hello
world
1
\N
\N
nan
1
inf
inf
inf
-0
123
Expand All @@ -20,4 +20,4 @@ inf
[]
[NULL]
[0]
[NULL]
[0]
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Test with one const argument
\N \N
Test with two const arguments
1 1
1 1
1.1 1.1
1.1 1.1
a a
a a
Test with one non-const argument
\N \N
Test with two non-const arguments
1 1
1 1
1.1 1.1
1.1 1.1
a a
a a
Special cases
2 1
1 1
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
-- Tests functions "greatest" and "least" with NULL arguments

SELECT 'Test with one const argument';
SELECT greatest(NULL), least(NULL);

SELECT 'Test with two const arguments';
SELECT greatest(1, NULL), least(1, NULL);
SELECT greatest(NULL, 1), least(NULL, 1);
SELECT greatest(NULL, 1.1), least(NULL, 1.1);
SELECT greatest(1.1, NULL), least(1.1, NULL);
SELECT greatest(NULL, 'a'), least(NULL, 'a');
SELECT greatest('a', NULL), least('a', NULL);

SELECT 'Test with one non-const argument';
SELECT greatest(materialize(NULL)), least(materialize(NULL));

SELECT 'Test with two non-const arguments';
SELECT greatest(materialize(1), NULL), least(materialize(1), NULL);
SELECT greatest(materialize(NULL), 1), least(materialize(NULL), 1);
SELECT greatest(materialize(NULL), 1.1), least(materialize(NULL), 1.1);
SELECT greatest(materialize(1.1), NULL), least(materialize(1.1), NULL);
SELECT greatest(materialize(NULL), 'a'), least(materialize(NULL), 'a');
SELECT greatest(materialize('a'), NULL), least(materialize('a'), NULL);

SELECT 'Special cases';
SELECT greatest(toNullable(1), 2), least(toNullable(1), 2);
SELECT greatest(toLowCardinality(1), NULL), least(toLowCardinality(1), NULL);

0 comments on commit 4c36e8f

Please sign in to comment.