From 10daacb18f51974f89739067ace1d862afc1ff28 Mon Sep 17 00:00:00 2001
From: lgbo-ustc <lgbo.ustc@gmail.com>
Date: Wed, 19 Apr 2023 19:22:36 +0800
Subject: [PATCH] updated

---
 .../reference/first_value.md                  |  2 +-
 .../reference/last_value.md                   |  2 +-
 .../AggregateFunctionAny.cpp                  | 18 ++--
 .../AggregateFunctionFactory.cpp              |  2 +-
 .../AggregateFunctionMinMaxAny.h              | 83 ++++---------------
 src/AggregateFunctions/HelpersMinMaxAny.h     | 16 +---
 src/Parsers/ExpressionListParsers.cpp         | 16 ++--
 7 files changed, 39 insertions(+), 100 deletions(-)

diff --git a/docs/en/sql-reference/aggregate-functions/reference/first_value.md b/docs/en/sql-reference/aggregate-functions/reference/first_value.md
index 12dcf179f84..e163bd62a45 100644
--- a/docs/en/sql-reference/aggregate-functions/reference/first_value.md
+++ b/docs/en/sql-reference/aggregate-functions/reference/first_value.md
@@ -9,7 +9,7 @@ Selects the first encountered value, similar to `any`, but could accept NULL.
 
 ## examples
 
-```sq;
+```sql
 insert into test_data (a,b) values (1,null), (2,3), (4, 5), (6,null)
 ```
 
diff --git a/docs/en/sql-reference/aggregate-functions/reference/last_value.md b/docs/en/sql-reference/aggregate-functions/reference/last_value.md
index 05894c333f6..ebf002e6ae2 100644
--- a/docs/en/sql-reference/aggregate-functions/reference/last_value.md
+++ b/docs/en/sql-reference/aggregate-functions/reference/last_value.md
@@ -10,7 +10,7 @@ Selects the last encountered value, similar to `anyLast`, but could accept NULL.
 
 ## examples
 
-```sq;
+```sql
 insert into test_data (a,b) values (1,null), (2,3), (4, 5), (6,null)
 ```
 
diff --git a/src/AggregateFunctions/AggregateFunctionAny.cpp b/src/AggregateFunctions/AggregateFunctionAny.cpp
index 460ae3262b8..7f57062126b 100644
--- a/src/AggregateFunctions/AggregateFunctionAny.cpp
+++ b/src/AggregateFunctions/AggregateFunctionAny.cpp
@@ -14,11 +14,12 @@ AggregateFunctionPtr createAggregateFunctionAny(const std::string & name, const
     return AggregateFunctionPtr(createAggregateFunctionSingleValue<AggregateFunctionsSingleValue, AggregateFunctionAnyData>(name, argument_types, parameters, settings));
 }
 
-template <bool RespectNulls = false, bool NullIsGreater = false>
-AggregateFunctionPtr createAggregateFunctionNullableAny(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings)
+template <bool RespectNulls = false>
+AggregateFunctionPtr createAggregateFunctionNullableAny(
+    const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings)
 {
     return AggregateFunctionPtr(
-        createAggregateFunctionSingleNullableValue<AggregateFunctionsSingleValue, AggregateFunctionAnyData, RespectNulls, NullIsGreater>(
+        createAggregateFunctionSingleNullableValue<AggregateFunctionsSingleValue, AggregateFunctionAnyData, RespectNulls>(
             name, argument_types, parameters, settings));
 }
 
@@ -27,14 +28,13 @@ AggregateFunctionPtr createAggregateFunctionAnyLast(const std::string & name, co
     return AggregateFunctionPtr(createAggregateFunctionSingleValue<AggregateFunctionsSingleValue, AggregateFunctionAnyLastData>(name, argument_types, parameters, settings));
 }
 
-template <bool RespectNulls = false, bool NullIsGreater = false>
+template <bool RespectNulls = false>
 AggregateFunctionPtr createAggregateFunctionNullableAnyLast(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings)
 {
     return AggregateFunctionPtr(createAggregateFunctionSingleNullableValue<
                                 AggregateFunctionsSingleValue,
                                 AggregateFunctionAnyLastData,
-                                RespectNulls,
-                                NullIsGreater>(name, argument_types, parameters, settings));
+                                RespectNulls>(name, argument_types, parameters, settings));
 }
 
 AggregateFunctionPtr createAggregateFunctionAnyHeavy(const std::string & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings)
@@ -59,18 +59,12 @@ void registerAggregateFunctionsAny(AggregateFunctionFactory & factory)
     factory.registerFunction("first_value_respect_nulls",
         { createAggregateFunctionNullableAny<true>, properties },
         AggregateFunctionFactory::CaseInsensitive);
-    factory.registerFunction("first_value_ignore_nulls",
-        { createAggregateFunctionNullableAny<false>, properties },
-        AggregateFunctionFactory::CaseInsensitive);
     factory.registerFunction("last_value",
         { createAggregateFunctionAnyLast, properties },
         AggregateFunctionFactory::CaseInsensitive);
     factory.registerFunction("last_value_respect_nulls",
         { createAggregateFunctionNullableAnyLast<true>, properties },
         AggregateFunctionFactory::CaseInsensitive);
-    factory.registerFunction("last_value_ignore_nulls",
-        { createAggregateFunctionNullableAnyLast<false>, properties },
-        AggregateFunctionFactory::CaseInsensitive);
 }
 
 }
diff --git a/src/AggregateFunctions/AggregateFunctionFactory.cpp b/src/AggregateFunctions/AggregateFunctionFactory.cpp
index 9c60160f3ee..6cacf66500f 100644
--- a/src/AggregateFunctions/AggregateFunctionFactory.cpp
+++ b/src/AggregateFunctions/AggregateFunctionFactory.cpp
@@ -106,7 +106,7 @@ AggregateFunctionPtr AggregateFunctionFactory::get(
         // nullability themselves. Another special case is functions from Nothing
         // that are rewritten to AggregateFunctionNothing, in this case
         // nested_function is nullptr.
-        if ((!nested_function || !nested_function->isOnlyWindowFunction()))
+        if (!nested_function || !nested_function->isOnlyWindowFunction())
             return combinator->transformAggregateFunction(nested_function, out_properties, types_without_low_cardinality, parameters);
     }
 
diff --git a/src/AggregateFunctions/AggregateFunctionMinMaxAny.h b/src/AggregateFunctions/AggregateFunctionMinMaxAny.h
index dc6a822d85c..51eec1689ae 100644
--- a/src/AggregateFunctions/AggregateFunctionMinMaxAny.h
+++ b/src/AggregateFunctions/AggregateFunctionMinMaxAny.h
@@ -766,7 +766,7 @@ static_assert(
 
 
 /// For any other value types.
-template <bool IS_NULLABLE = false, bool IS_NULL_GREATER = false>
+template <bool IS_NULLABLE = false>
 struct SingleValueDataGeneric
 {
 private:
@@ -778,7 +778,6 @@ private:
 public:
     static constexpr bool is_nullable = IS_NULLABLE;
     static constexpr bool is_any = false;
-    static constexpr bool is_null_greater = IS_NULL_GREATER;
 
     bool has() const
     {
@@ -881,26 +880,13 @@ public:
             {
                 Field new_value;
                 column.get(row_num, new_value);
-                if constexpr (!is_null_greater)
+                if (!value.isNull() && (new_value.isNull() || new_value < value))
                 {
-                    if (!value.isNull() && (new_value.isNull() || new_value < value))
-                    {
-                        value = new_value;
-                        return true;
-                    }
-                    else
-                        return false;
+                    value = new_value;
+                    return true;
                 }
                 else
-                {
-                    if ((value.isNull() && !new_value.isNull()) || (!new_value.isNull() && new_value < value))
-                    {
-                        value = new_value;
-                        return true;
-                    }
-
                     return false;
-                }
             }
             else
             {
@@ -928,24 +914,12 @@ public:
                 change(to, arena);
                 return true;
             }
-            if constexpr (!is_null_greater)
+            if (to.value.isNull() || (!value.isNull() && to.value < value))
             {
-                if (to.value.isNull() || (!value.isNull() && to.value < value))
-                {
-                    value = to.value;
-                    return true;
-                }
-                return false;
-            }
-            else
-            {
-                if ((value.isNull() && !to.value.isNull()) || (!to.value.isNull() || to.value < value))
-                {
-                    value = to.value;
-                    return true;
-                }
-                return false;
+                value = to.value;
+                return true;
             }
+            return false;
         }
         else
         {
@@ -972,24 +946,12 @@ public:
             {
                 Field new_value;
                 column.get(row_num, new_value);
-                if constexpr (is_null_greater)
+                if (!value.isNull() && (new_value.isNull() || value < new_value))
                 {
-                    if (!value.isNull() && (new_value.isNull() || value < new_value))
-                    {
-                        value = new_value;
-                        return true;
-                    }
-                    return false;
-                }
-                else
-                {
-                    if ((value.isNull() && !new_value.isNull()) || (!new_value.isNull() && value < new_value))
-                    {
-                        value = new_value;
-                        return true;
-                    }
-                    return false;
+                    value = new_value;
+                    return true;
                 }
+                return false;
             }
             else
             {
@@ -1012,25 +974,12 @@ public:
             return false;
         if constexpr (is_nullable)
         {
-            if constexpr (is_null_greater)
+            if (!value.isNull() && (to.value.isNull() || value < to.value))
             {
-                if (!value.isNull() && (to.value.isNull() || value < to.value))
-                {
-                    value = to.value;
-                    return true;
-                }
-                return false;
-            }
-            else
-            {
-                if ((value.isNull() && !to.value.isNull()) || (!to.value.isNull() && value < to.value))
-                {
-                    value = to.value;
-                    return true;
-                }
-                return false;
-
+                value = to.value;
+                return true;
             }
+            return false;
         }
         else
         {
diff --git a/src/AggregateFunctions/HelpersMinMaxAny.h b/src/AggregateFunctions/HelpersMinMaxAny.h
index ea67e2f072e..31ae5fdd59a 100644
--- a/src/AggregateFunctions/HelpersMinMaxAny.h
+++ b/src/AggregateFunctions/HelpersMinMaxAny.h
@@ -11,10 +11,6 @@
 
 namespace DB
 {
-namespace ErrorCodes
-{
-    extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
-}
 struct Settings;
 
 /// min, max, any, anyLast, anyHeavy, etc...
@@ -51,29 +47,23 @@ static IAggregateFunction * createAggregateFunctionSingleValue(const String & na
     return new AggregateFunctionTemplate<Data<SingleValueDataGeneric<>>>(argument_type);
 }
 
-template <template <typename> class AggregateFunctionTemplate, template <typename> class Data, bool RespectNulls = false, bool NullIsGreater = false>
+template <template <typename> class AggregateFunctionTemplate, template <typename> class Data, bool RespectNulls = false>
 static IAggregateFunction * createAggregateFunctionSingleNullableValue(const String & name, const DataTypes & argument_types, const Array & parameters, const Settings * settings)
 {
     assertNoParameters(name, parameters);
     assertUnary(name, argument_types);
-    if (parameters.size() > 2)
-        throw Exception(ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, "name's parameter size must not be larger  then 2");
 
     const DataTypePtr & argument_type = argument_types[0];
     WhichDataType which(argument_type);
-    // if the resule value could be null(excluding the case that no row is matched),
+    // If the result value could be null (excluding the case that no row is matched),
     // use SingleValueDataGeneric.
     if constexpr (!RespectNulls)
     {
         return createAggregateFunctionSingleValue<AggregateFunctionTemplate, Data>(name, argument_types, Array(), settings);
     }
-    else if constexpr (NullIsGreater)
-    {
-        return new AggregateFunctionTemplate<Data<SingleValueDataGeneric<true, true>>>(argument_type);
-    }
     else
     {
-        return new AggregateFunctionTemplate<Data<SingleValueDataGeneric<true, false>>>(argument_type);
+        return new AggregateFunctionTemplate<Data<SingleValueDataGeneric<true>>>(argument_type);
     }
     UNREACHABLE();
 }
diff --git a/src/Parsers/ExpressionListParsers.cpp b/src/Parsers/ExpressionListParsers.cpp
index 7fec7f4c5db..1477f3d000d 100644
--- a/src/Parsers/ExpressionListParsers.cpp
+++ b/src/Parsers/ExpressionListParsers.cpp
@@ -1104,7 +1104,7 @@ public:
                     return false;
             }
 
-            NullsAction nulls_action = NullsAction::DEFAULT;
+            NullsAction nulls_action = NullsAction::EMPTY;
             if (respect_nulls.ignore(pos, expected))
             {
                 nulls_action = NullsAction::RESPECT_NULLS;
@@ -1150,19 +1150,25 @@ private:
 
     enum NullsAction
     {
-        DEFAULT = 0,
+        EMPTY = 0,
         RESPECT_NULLS = 1,
         IGNORE_NULLS = 2,
     };
     static String transformFunctionNameForRepectNulls(const String & original_function_name, NullsAction nulls_action)
     {
         static std::unordered_map<String, std::vector<String>> renamed_functions_with_nulls = {
-            {"first_value", {"first_value_ignore_nulls", "first_value_respect_nulls", "first_value_ignore_nulls"}},
-            {"last_value", {"last_value_ignore_nulls", "last_value_respect_nulls", "last_value_ignore_nulls"}},
+            {"first_value", {"first_value", "first_value_respect_nulls", "first_value"}},
+            {"last_value", {"last_value", "last_value_respect_nulls", "last_value"}},
         };
         auto it = renamed_functions_with_nulls.find(original_function_name);
         if (it == renamed_functions_with_nulls.end())
-            return original_function_name;
+        {
+            if (nulls_action == NullsAction::EMPTY)
+                return original_function_name;
+            else
+                throw Exception(
+                    ErrorCodes::SYNTAX_ERROR, "Function {} does not support RESPECT NULLS or IGNORE NULLS", original_function_name);
+        }
         return it->second[nulls_action];
     }
 };