fix lagInFrame for nullable types

src/AggregateFunctions/AggregateFunctionFactory.cpp

@@ -71,9 +71,18 @@ AggregateFunctionPtr AggregateFunctionFactory::get(
 {
     auto type_without_low_cardinality = convertLowCardinalityTypesToNested(argument_types);
 
-    /// If one of the types is Nullable, we apply aggregate function combinator "Null".
+    auto with_original_parameters = getImpl(name, type_without_low_cardinality, parameters, out_properties, false);
 
-    if (std::any_of(type_without_low_cardinality.begin(), type_without_low_cardinality.end(),
+    // If one of the types is Nullable, we apply aggregate function combinator "Null".
+    // Pure window functions are not real aggregate functions. Applying
+    // combinators doesn't make sense for them, they must handle the
+    // nullability themselves. Another special case is functions from Nothing
+    // that are rewritten to AggregateFunctionNothing, in this case
+    // nested_function is nullptr.
+    if (!(with_original_parameters
+            && with_original_parameters->isOnlyWindowFunction())
+         && std::any_of(type_without_low_cardinality.begin(),
+            type_without_low_cardinality.end(),
             [](const auto & type) { return type->isNullable(); }))
     {
         AggregateFunctionCombinatorPtr combinator = AggregateFunctionCombinatorFactory::instance().tryFindSuffix("Null");
@@ -90,23 +99,12 @@ AggregateFunctionPtr AggregateFunctionFactory::get(
         AggregateFunctionPtr nested_function = getImpl(
             name, nested_types, nested_parameters, out_properties, has_null_arguments);
 
-        // Pure window functions are not real aggregate functions. Applying
-        // combinators doesn't make sense for them, they must handle the
-        // 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())
-        {
-            return nested_function;
-        }
-
         return combinator->transformAggregateFunction(nested_function, out_properties, type_without_low_cardinality, parameters);
     }
 
-    auto res = getImpl(name, type_without_low_cardinality, parameters, out_properties, false);
-    if (!res)
+    if (!with_original_parameters)
         throw Exception("Logical error: AggregateFunctionFactory returned nullptr", ErrorCodes::LOGICAL_ERROR);
-    return res;
+    return with_original_parameters;
 }
 
 

src/Processors/Transforms/WindowTransform.cpp

@@ -1475,10 +1475,10 @@ struct WindowFunctionLagLeadInFrame final : public WindowFunction
             return;
         }
 
-        if (!getLeastSupertype({argument_types[0], argument_types[2]}))
+        if (!argument_types[0]->equals(*argument_types[2]))
         {
             throw Exception(ErrorCodes::BAD_ARGUMENTS,
-                "The default value type '{}' is not convertible to the argument type '{}'",
+                "The default value type '{}' is not the same as the argument type '{}'",
                 argument_types[2]->getName(),
                 argument_types[0]->getName());
         }
@@ -1491,8 +1491,7 @@ struct WindowFunctionLagLeadInFrame final : public WindowFunction
         }
     }
 
-    DataTypePtr getReturnType() const override
-    { return argument_types[0]; }
+    DataTypePtr getReturnType() const override { return argument_types[0]; }
 
     bool allocatesMemoryInArena() const override { return false; }
 

tests/queries/0_stateless/01591_window_functions.reference

@@ -1056,10 +1056,25 @@ settings max_block_size = 3;
 15	3	15	0	15	15	15
 -- careful with auto-application of Null combinator
 select lagInFrame(toNullable(1)) over ();
-0
+\N
 select lagInFrameOrNull(1) over (); -- { serverError 36 }
-select intDiv(1, NULL) x, toTypeName(x), max(x) over ();
-\N	Nullable(Nothing)	\N
+-- this should give the same error as `select max(Null::Nullable(Nothing))`
+select intDiv(1, NULL) x, toTypeName(x), max(x) over (); -- { serverError 43 }
+-- to make lagInFrame return null for out-of-frame rows, cast the argument to
+-- Nullable; otherwise, it returns default values.
+SELECT
+    number,
+    lagInFrame(toNullable(number), 1) OVER w,
+    lagInFrame(toNullable(number), 2) OVER w,
+    lagInFrame(number, 1) OVER w,
+    lagInFrame(number, 2) OVER w
+FROM numbers(4)
+WINDOW w AS (ORDER BY number ASC)
+;
+0	\N	\N	0	0
+1	0	\N	0	0
+2	1	0	1	0
+3	2	1	2	1
 -- case-insensitive SQL-standard synonyms for any and anyLast
 select
     number,

tests/queries/0_stateless/01591_window_functions.sql

@@ -379,7 +379,19 @@ settings max_block_size = 3;
 -- careful with auto-application of Null combinator
 select lagInFrame(toNullable(1)) over ();
 select lagInFrameOrNull(1) over (); -- { serverError 36 }
-select intDiv(1, NULL) x, toTypeName(x), max(x) over ();
+-- this should give the same error as `select max(Null::Nullable(Nothing))`
+select intDiv(1, NULL) x, toTypeName(x), max(x) over (); -- { serverError 43 }
+-- to make lagInFrame return null for out-of-frame rows, cast the argument to
+-- Nullable; otherwise, it returns default values.
+SELECT
+    number,
+    lagInFrame(toNullable(number), 1) OVER w,
+    lagInFrame(toNullable(number), 2) OVER w,
+    lagInFrame(number, 1) OVER w,
+    lagInFrame(number, 2) OVER w
+FROM numbers(4)
+WINDOW w AS (ORDER BY number ASC)
+;
 
 -- case-insensitive SQL-standard synonyms for any and anyLast
 select