From 4eb053a30360183211129f07727bbf1f74f68206 Mon Sep 17 00:00:00 2001 From: ryzuo Date: Thu, 8 Jul 2021 18:22:41 +0800 Subject: [PATCH 1/6] Implementaion of window function nth_value. 1. Implemented the window function nth_value 2. Make the return type of lag/lead, and nth_value functions to be ColumnNullable, thus the returned value can be null if it is out of frame. --- src/Processors/Transforms/WindowTransform.cpp | 89 ++++++++++++++++++- .../01591_window_functions.reference | 70 +++++++++------ .../0_stateless/01591_window_functions.sql | 47 +++++----- 3 files changed, 152 insertions(+), 54 deletions(-) diff --git a/src/Processors/Transforms/WindowTransform.cpp b/src/Processors/Transforms/WindowTransform.cpp index 7786c5c950a..41bf29e1b1c 100644 --- a/src/Processors/Transforms/WindowTransform.cpp +++ b/src/Processors/Transforms/WindowTransform.cpp @@ -1558,9 +1558,85 @@ struct WindowFunctionLagLeadInFrame final : public WindowFunction else { // Offset is inside the frame. - to.insertFrom(*transform->blockAt(target_row).input_columns[ - workspace.argument_column_indices[0]], - target_row.row); + auto ptr = ColumnNullable::create(transform->blockAt(target_row).input_columns[ + workspace.argument_column_indices[0]], + ColumnUInt8::create()); + to.insertFrom(*ptr, target_row.row); + } + } +}; + +struct WindowFunctionNthValue final : public WindowFunction +{ + WindowFunctionNthValue(const std::string & name_, + const DataTypes & argument_types_, const Array & parameters_) + : WindowFunction(name_, argument_types_, parameters_) + { + if (!parameters.empty()) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Function {} cannot be parameterized", name_); + } + + if (argument_types.empty()) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Function {} takes at least one argument", name_); + } + + if(argument_types.size() != 2) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Function '{}' accepts 2 arguments, {} given", + name_, argument_types.size()); + } + } + + DataTypePtr getReturnType() const override + { return argument_types[0]; } + + bool allocatesMemoryInArena() const override { return false; } + + void windowInsertResultInto(const WindowTransform * transform, + size_t function_index) override + { + const auto & current_block = transform->blockAt(transform->current_row); + IColumn & to = *(current_block.output_columns[function_index]); + const auto & workspace = transform->workspaces[function_index]; + + int64_t offset = (*current_block.input_columns[ + workspace.argument_column_indices[1]])[ + transform->current_row.row].get() - 1; + + if (offset < 0) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "The offset for function {} must be non-negative, {} given", + getName(), offset); + } + + if (offset > INT_MAX) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "The offset for function {} must be less than {}, {} given", + getName(), INT_MAX, offset); + } + + const auto [target_row, offset_left] = transform->moveRowNumber(transform->frame_start, offset); + if (offset_left != 0 + || target_row < transform->frame_start + || transform->frame_end <= target_row) + { + // Offset is outside the frame. + to.insertDefault(); + } + else + { + // Offset is inside the frame. + auto ptr = ColumnNullable::create(transform->blockAt(target_row).input_columns[ + workspace.argument_column_indices[0]], + ColumnUInt8::create()); + to.insertFrom(*ptr, target_row.row); } } }; @@ -1628,6 +1704,13 @@ void registerWindowFunctions(AggregateFunctionFactory & factory) return std::make_shared>( name, argument_types, parameters); }, properties}); + + factory.registerFunction("nth_value", {[](const std::string & name, + const DataTypes & argument_types, const Array & parameters, const Settings *) + { + return std::make_shared( + name, argument_types, parameters); + }, properties}); } } diff --git a/tests/queries/0_stateless/01591_window_functions.reference b/tests/queries/0_stateless/01591_window_functions.reference index 669a7e4f32f..3f02f1566c8 100644 --- a/tests/queries/0_stateless/01591_window_functions.reference +++ b/tests/queries/0_stateless/01591_window_functions.reference @@ -1095,6 +1095,48 @@ order by number 7 6 8 8 7 9 9 8 9 +-- nth_value without specific frame range given +select + number, + nth_value(number, 1) over w as firstValue, + nth_value(number, 2) over w as secondValue, + nth_value(number, 3) over w as thirdValue, + nth_value(number, 4) over w as fourthValue +from numbers(10) +window w as (order by number) +order by number +; +0 0 NULL NULL NULL +1 0 1 NULL NULL +2 0 1 2 NULL +3 0 1 2 3 +4 0 1 2 3 +5 0 1 2 3 +6 0 1 2 3 +7 0 1 2 3 +8 0 1 2 3 +9 0 1 2 3 +-- nth_value with frame range specified +select + number, + nth_value(number, 1) over w as firstValue, + nth_value(number, 2) over w as secondValue, + nth_value(number, 3) over w as thirdValue, + nth_value(number, 4) over w as fourthValue +from numbers(10) +window w as (order by number range between 1 preceding and 1 following) +order by number +; +0 0 1 NULL NULL +1 0 1 2 NULL +2 1 2 3 NULL +3 2 3 4 NULL +4 3 4 5 NULL +5 4 5 6 NULL +6 5 6 7 NULL +7 6 7 8 NULL +8 7 8 9 NULL +9 8 9 NULL NULL -- In this case, we had a problem with PartialSortingTransform returning zero-row -- chunks for input chunks w/o columns. select count() over () from numbers(4) where number < 2; @@ -1136,31 +1178,3 @@ select count() over (rows between 2147483648 preceding and 2147493648 following) select count() over () from (select 1 a) l inner join (select 2 a) r using a; -- This case works as expected, one empty input chunk marked as input end. select count() over () where null; --- Inheriting another window. -select number, count() over (w1 rows unbounded preceding) from numbers(10) -window - w0 as (partition by intDiv(number, 5) as p), - w1 as (w0 order by mod(number, 3) as o) -order by p, o, number -; -0 1 -3 2 -1 3 -4 4 -2 5 -6 1 -9 2 -7 3 -5 4 -8 5 --- can't redefine PARTITION BY -select count() over (w partition by number) from numbers(1) window w as (partition by intDiv(number, 5)); -- { serverError 36 } --- can't redefine existing ORDER BY -select count() over (w order by number) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3)); -- { serverError 36 } --- parent window can't have frame -select count() over (w range unbounded preceding) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3) rows unbounded preceding); -- { serverError 36 } --- looks weird but probably should work -- this is a window that inherits and changes nothing -select count() over (w) from numbers(1) window w as (); -1 --- nonexistent parent window -select count() over (w2 rows unbounded preceding); -- { serverError 36 } diff --git a/tests/queries/0_stateless/01591_window_functions.sql b/tests/queries/0_stateless/01591_window_functions.sql index b9456563c63..e6dcc48fece 100644 --- a/tests/queries/0_stateless/01591_window_functions.sql +++ b/tests/queries/0_stateless/01591_window_functions.sql @@ -403,6 +403,30 @@ window w as (order by number range between 1 preceding and 1 following) order by number ; +-- nth_value without specific frame range given +select + number, + nth_value(number, 1) over w as firstValue, + nth_value(number, 2) over w as secondValue, + nth_value(number, 3) over w as thirdValue, + nth_value(number, 4) over w as fourthValue +from numbers(10) +window w as (order by number) +order by number +; + +-- nth_value with frame range specified +select + number, + nth_value(number, 1) over w as firstValue, + nth_value(number, 2) over w as secondValue, + nth_value(number, 3) over w as thirdValue, + nth_value(number, 4) over w as fourthValue +from numbers(10) +window w as (order by number range between 1 preceding and 1 following) +order by number +; + -- In this case, we had a problem with PartialSortingTransform returning zero-row -- chunks for input chunks w/o columns. select count() over () from numbers(4) where number < 2; @@ -439,26 +463,3 @@ select count() over (rows between 2147483648 preceding and 2147493648 following) select count() over () from (select 1 a) l inner join (select 2 a) r using a; -- This case works as expected, one empty input chunk marked as input end. select count() over () where null; - --- Inheriting another window. -select number, count() over (w1 rows unbounded preceding) from numbers(10) -window - w0 as (partition by intDiv(number, 5) as p), - w1 as (w0 order by mod(number, 3) as o) -order by p, o, number -; - --- can't redefine PARTITION BY -select count() over (w partition by number) from numbers(1) window w as (partition by intDiv(number, 5)); -- { serverError 36 } - --- can't redefine existing ORDER BY -select count() over (w order by number) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3)); -- { serverError 36 } - --- parent window can't have frame -select count() over (w range unbounded preceding) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3) rows unbounded preceding); -- { serverError 36 } - --- looks weird but probably should work -- this is a window that inherits and changes nothing -select count() over (w) from numbers(1) window w as (); - --- nonexistent parent window -select count() over (w2 rows unbounded preceding); -- { serverError 36 } From fd0c016423563c300bc70f81252a01399882f17f Mon Sep 17 00:00:00 2001 From: ryzuo Date: Fri, 9 Jul 2021 01:47:08 +0800 Subject: [PATCH 2/6] Make tests of nth_value pass --- .../01591_window_functions.reference | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/queries/0_stateless/01591_window_functions.reference b/tests/queries/0_stateless/01591_window_functions.reference index 3f02f1566c8..a56e16e2b54 100644 --- a/tests/queries/0_stateless/01591_window_functions.reference +++ b/tests/queries/0_stateless/01591_window_functions.reference @@ -1106,16 +1106,16 @@ from numbers(10) window w as (order by number) order by number ; -0 0 NULL NULL NULL -1 0 1 NULL NULL -2 0 1 2 NULL -3 0 1 2 3 -4 0 1 2 3 -5 0 1 2 3 -6 0 1 2 3 -7 0 1 2 3 -8 0 1 2 3 -9 0 1 2 3 +0 0 \N \N \N +1 0 1 \N \N +2 0 1 2 \N +3 0 1 2 3 +4 0 1 2 3 +5 0 1 2 3 +6 0 1 2 3 +7 0 1 2 3 +8 0 1 2 3 +9 0 1 2 3 -- nth_value with frame range specified select number, @@ -1127,16 +1127,16 @@ from numbers(10) window w as (order by number range between 1 preceding and 1 following) order by number ; -0 0 1 NULL NULL -1 0 1 2 NULL -2 1 2 3 NULL -3 2 3 4 NULL -4 3 4 5 NULL -5 4 5 6 NULL -6 5 6 7 NULL -7 6 7 8 NULL -8 7 8 9 NULL -9 8 9 NULL NULL +0 0 1 \N \N +1 0 1 2 \N +2 1 2 3 \N +3 2 3 4 \N +4 3 4 5 \N +5 4 5 6 \N +6 5 6 7 \N +7 6 7 8 \N +8 7 8 9 \N +9 8 9 \N \N -- In this case, we had a problem with PartialSortingTransform returning zero-row -- chunks for input chunks w/o columns. select count() over () from numbers(4) where number < 2; From df9ec5655c98f796c6e46ba00241e5445e09e5d5 Mon Sep 17 00:00:00 2001 From: ryzuo Date: Thu, 15 Jul 2021 13:41:31 +0800 Subject: [PATCH 3/6] Add missing test back in 01591_window_function --- src/Processors/Transforms/WindowTransform.cpp | 29 ++++++---- .../01591_window_functions.reference | 54 +++++++++++++++++++ .../0_stateless/01591_window_functions.sql | 23 ++++++++ 3 files changed, 97 insertions(+), 9 deletions(-) diff --git a/src/Processors/Transforms/WindowTransform.cpp b/src/Processors/Transforms/WindowTransform.cpp index 41bf29e1b1c..5f93386034b 100644 --- a/src/Processors/Transforms/WindowTransform.cpp +++ b/src/Processors/Transforms/WindowTransform.cpp @@ -9,7 +9,6 @@ #include #include - namespace DB { @@ -1558,10 +1557,16 @@ struct WindowFunctionLagLeadInFrame final : public WindowFunction else { // Offset is inside the frame. - auto ptr = ColumnNullable::create(transform->blockAt(target_row).input_columns[ - workspace.argument_column_indices[0]], - ColumnUInt8::create()); - to.insertFrom(*ptr, target_row.row); + auto srcColumnPtr = transform->blockAt(target_row).input_columns[workspace.argument_column_indices[0]]; + // If the original column type is Nullable(from DDL) + if(srcColumnPtr->getDataType() == TypeIndex::Nullable) + { + to.insertFrom(*srcColumnPtr, target_row.row); + } + else + { + assert_cast(to).insertFromNotNullable(*srcColumnPtr, target_row.row); + } } } }; @@ -1633,10 +1638,16 @@ struct WindowFunctionNthValue final : public WindowFunction else { // Offset is inside the frame. - auto ptr = ColumnNullable::create(transform->blockAt(target_row).input_columns[ - workspace.argument_column_indices[0]], - ColumnUInt8::create()); - to.insertFrom(*ptr, target_row.row); + auto srcColumnPtr = transform->blockAt(target_row).input_columns[workspace.argument_column_indices[0]]; + // If the original column type is Nullable(from DDL) + if(srcColumnPtr->getDataType() == TypeIndex::Nullable) + { + to.insertFrom(*srcColumnPtr, target_row.row); + } + else + { + assert_cast(to).insertFromNotNullable(*srcColumnPtr, target_row.row); + } } } }; diff --git a/tests/queries/0_stateless/01591_window_functions.reference b/tests/queries/0_stateless/01591_window_functions.reference index a56e16e2b54..a7f6a353d43 100644 --- a/tests/queries/0_stateless/01591_window_functions.reference +++ b/tests/queries/0_stateless/01591_window_functions.reference @@ -1053,11 +1053,33 @@ settings max_block_size = 3; 12 2 10 11 10 10 14 13 2 10 12 10 10 143 14 2 10 13 10 10 154 +<<<<<<< HEAD 15 3 15 0 15 15 15 +======= +15 3 15 \N 15 15 15 +-- careful with auto-application of Null combinator +SELECT number, + lagInFrame(toNullable(number), 1) OVER w AS prevOne, + lagInFrame(toNullable(number), 2) OVER w AS prevTwo +FROM numbers(10) +WINDOW w AS (ORDER BY number ASC) +; +0 \N \N +1 0 \N +2 1 0 +3 2 1 +4 3 2 +5 4 3 +6 5 4 +7 6 5 +8 7 6 +9 8 7 +>>>>>>> Add missing test back in 01591_window_function -- careful with auto-application of Null combinator select lagInFrame(toNullable(1)) over (); \N select lagInFrameOrNull(1) over (); -- { serverError 36 } +<<<<<<< HEAD -- this is the same as `select max(Null::Nullable(Nothing))` select intDiv(1, NULL) x, toTypeName(x), max(x) over (); \N Nullable(Nothing) \N @@ -1076,6 +1098,10 @@ WINDOW w AS (ORDER BY number ASC) 1 0 \N 0 0 2 1 0 1 0 3 2 1 2 1 +======= +select intDiv(1, NULL) x, toTypeName(x), max(x) over (); +\N Nullable(Nothing) \N +>>>>>>> Add missing test back in 01591_window_function -- case-insensitive SQL-standard synonyms for any and anyLast select number, @@ -1178,3 +1204,31 @@ select count() over (rows between 2147483648 preceding and 2147493648 following) select count() over () from (select 1 a) l inner join (select 2 a) r using a; -- This case works as expected, one empty input chunk marked as input end. select count() over () where null; +-- Inheriting another window. +select number, count() over (w1 rows unbounded preceding) from numbers(10) +window + w0 as (partition by intDiv(number, 5) as p), + w1 as (w0 order by mod(number, 3) as o) +order by p, o, number +; +0 1 +3 2 +1 3 +4 4 +2 5 +6 1 +9 2 +7 3 +5 4 +8 5 +-- can't redefine PARTITION BY +select count() over (w partition by number) from numbers(1) window w as (partition by intDiv(number, 5)); -- { serverError 36 } +-- can't redefine existing ORDER BY +select count() over (w order by number) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3)); -- { serverError 36 } +-- parent window can't have frame +select count() over (w range unbounded preceding) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3) rows unbounded preceding); -- { serverError 36 } +-- looks weird but probably should work -- this is a window that inherits and changes nothing +select count() over (w) from numbers(1) window w as (); +1 +-- nonexistent parent window +select count() over (w2 rows unbounded preceding); -- { serverError 36 } diff --git a/tests/queries/0_stateless/01591_window_functions.sql b/tests/queries/0_stateless/01591_window_functions.sql index e6dcc48fece..ffa44583579 100644 --- a/tests/queries/0_stateless/01591_window_functions.sql +++ b/tests/queries/0_stateless/01591_window_functions.sql @@ -463,3 +463,26 @@ select count() over (rows between 2147483648 preceding and 2147493648 following) select count() over () from (select 1 a) l inner join (select 2 a) r using a; -- This case works as expected, one empty input chunk marked as input end. select count() over () where null; + +-- Inheriting another window. +select number, count() over (w1 rows unbounded preceding) from numbers(10) +window + w0 as (partition by intDiv(number, 5) as p), + w1 as (w0 order by mod(number, 3) as o) +order by p, o, number +; + +-- can't redefine PARTITION BY +select count() over (w partition by number) from numbers(1) window w as (partition by intDiv(number, 5)); -- { serverError 36 } + +-- can't redefine existing ORDER BY +select count() over (w order by number) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3)); -- { serverError 36 } + +-- parent window can't have frame +select count() over (w range unbounded preceding) from numbers(1) window w as (partition by intDiv(number, 5) order by mod(number, 3) rows unbounded preceding); -- { serverError 36 } + +-- looks weird but probably should work -- this is a window that inherits and changes nothing +select count() over (w) from numbers(1) window w as (); + +-- nonexistent parent window +select count() over (w2 rows unbounded preceding); -- { serverError 36 } From 94d4fb9dfc63b4e4cb06e5e9586818b9cda636b3 Mon Sep 17 00:00:00 2001 From: ryzuo Date: Fri, 16 Jul 2021 10:36:40 +0800 Subject: [PATCH 4/6] Cleanup the incomplete fix of nullability return value of lagInFrame/leadInFrame --- src/Processors/Transforms/WindowTransform.cpp | 17 ++++-------- .../01591_window_functions.reference | 26 ------------------- 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/src/Processors/Transforms/WindowTransform.cpp b/src/Processors/Transforms/WindowTransform.cpp index 5f93386034b..455475bf1b7 100644 --- a/src/Processors/Transforms/WindowTransform.cpp +++ b/src/Processors/Transforms/WindowTransform.cpp @@ -1557,16 +1557,9 @@ struct WindowFunctionLagLeadInFrame final : public WindowFunction else { // Offset is inside the frame. - auto srcColumnPtr = transform->blockAt(target_row).input_columns[workspace.argument_column_indices[0]]; - // If the original column type is Nullable(from DDL) - if(srcColumnPtr->getDataType() == TypeIndex::Nullable) - { - to.insertFrom(*srcColumnPtr, target_row.row); - } - else - { - assert_cast(to).insertFromNotNullable(*srcColumnPtr, target_row.row); - } + to.insertFrom(*transform->blockAt(target_row).input_columns[ + workspace.argument_column_indices[0]], + target_row.row); } } }; @@ -1589,7 +1582,7 @@ struct WindowFunctionNthValue final : public WindowFunction "Function {} takes at least one argument", name_); } - if(argument_types.size() != 2) + if (argument_types.size() != 2) { throw Exception(ErrorCodes::BAD_ARGUMENTS, "Function '{}' accepts 2 arguments, {} given", @@ -1640,7 +1633,7 @@ struct WindowFunctionNthValue final : public WindowFunction // Offset is inside the frame. auto srcColumnPtr = transform->blockAt(target_row).input_columns[workspace.argument_column_indices[0]]; // If the original column type is Nullable(from DDL) - if(srcColumnPtr->getDataType() == TypeIndex::Nullable) + if (srcColumnPtr->getDataType() == TypeIndex::Nullable) { to.insertFrom(*srcColumnPtr, target_row.row); } diff --git a/tests/queries/0_stateless/01591_window_functions.reference b/tests/queries/0_stateless/01591_window_functions.reference index a7f6a353d43..06ac346f4cf 100644 --- a/tests/queries/0_stateless/01591_window_functions.reference +++ b/tests/queries/0_stateless/01591_window_functions.reference @@ -1053,33 +1053,11 @@ settings max_block_size = 3; 12 2 10 11 10 10 14 13 2 10 12 10 10 143 14 2 10 13 10 10 154 -<<<<<<< HEAD 15 3 15 0 15 15 15 -======= -15 3 15 \N 15 15 15 --- careful with auto-application of Null combinator -SELECT number, - lagInFrame(toNullable(number), 1) OVER w AS prevOne, - lagInFrame(toNullable(number), 2) OVER w AS prevTwo -FROM numbers(10) -WINDOW w AS (ORDER BY number ASC) -; -0 \N \N -1 0 \N -2 1 0 -3 2 1 -4 3 2 -5 4 3 -6 5 4 -7 6 5 -8 7 6 -9 8 7 ->>>>>>> Add missing test back in 01591_window_function -- careful with auto-application of Null combinator select lagInFrame(toNullable(1)) over (); \N select lagInFrameOrNull(1) over (); -- { serverError 36 } -<<<<<<< HEAD -- this is the same as `select max(Null::Nullable(Nothing))` select intDiv(1, NULL) x, toTypeName(x), max(x) over (); \N Nullable(Nothing) \N @@ -1098,10 +1076,6 @@ WINDOW w AS (ORDER BY number ASC) 1 0 \N 0 0 2 1 0 1 0 3 2 1 2 1 -======= -select intDiv(1, NULL) x, toTypeName(x), max(x) over (); -\N Nullable(Nothing) \N ->>>>>>> Add missing test back in 01591_window_function -- case-insensitive SQL-standard synonyms for any and anyLast select number, From 4d36d54c81d506d24c5905d1e8d24baab030f392 Mon Sep 17 00:00:00 2001 From: ryzuo Date: Fri, 23 Jul 2021 00:34:08 +0800 Subject: [PATCH 5/6] Update the implementaion of nth_value Make nth_value for nullable values for out of frame rows, as the same fashion as lagInFrame just does. --- src/Processors/Transforms/WindowTransform.cpp | 13 ++---- .../01591_window_functions.reference | 40 +++++++++++++------ .../0_stateless/01591_window_functions.sql | 10 +++++ 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/Processors/Transforms/WindowTransform.cpp b/src/Processors/Transforms/WindowTransform.cpp index 455475bf1b7..22911c9b774 100644 --- a/src/Processors/Transforms/WindowTransform.cpp +++ b/src/Processors/Transforms/WindowTransform.cpp @@ -1631,16 +1631,9 @@ struct WindowFunctionNthValue final : public WindowFunction else { // Offset is inside the frame. - auto srcColumnPtr = transform->blockAt(target_row).input_columns[workspace.argument_column_indices[0]]; - // If the original column type is Nullable(from DDL) - if (srcColumnPtr->getDataType() == TypeIndex::Nullable) - { - to.insertFrom(*srcColumnPtr, target_row.row); - } - else - { - assert_cast(to).insertFromNotNullable(*srcColumnPtr, target_row.row); - } + to.insertFrom(*transform->blockAt(target_row).input_columns[ + workspace.argument_column_indices[0]], + target_row.row); } } }; diff --git a/tests/queries/0_stateless/01591_window_functions.reference b/tests/queries/0_stateless/01591_window_functions.reference index 06ac346f4cf..20006a79e90 100644 --- a/tests/queries/0_stateless/01591_window_functions.reference +++ b/tests/queries/0_stateless/01591_window_functions.reference @@ -1106,9 +1106,9 @@ from numbers(10) window w as (order by number) order by number ; -0 0 \N \N \N -1 0 1 \N \N -2 0 1 2 \N +0 0 0 0 0 +1 0 1 0 0 +2 0 1 2 0 3 0 1 2 3 4 0 1 2 3 5 0 1 2 3 @@ -1127,16 +1127,30 @@ from numbers(10) window w as (order by number range between 1 preceding and 1 following) order by number ; -0 0 1 \N \N -1 0 1 2 \N -2 1 2 3 \N -3 2 3 4 \N -4 3 4 5 \N -5 4 5 6 \N -6 5 6 7 \N -7 6 7 8 \N -8 7 8 9 \N -9 8 9 \N \N +0 0 1 0 0 +1 0 1 2 0 +2 1 2 3 0 +3 2 3 4 0 +4 3 4 5 0 +5 4 5 6 0 +6 5 6 7 0 +7 6 7 8 0 +8 7 8 9 0 +9 8 9 0 0 +-- to make nth_value return null for out-of-frame rows, cast the argument to +-- Nullable; otherwise, it returns default values. +SELECT + number, + nth_value(toNullable(number), 1) OVER w as firstValue, + nth_value(toNullable(number), 3) OVER w as thridValue +FROM numbers(5) +WINDOW w AS (ORDER BY number ASC) +; +0 0 \N +1 0 \N +2 0 2 +3 0 2 +4 0 2 -- In this case, we had a problem with PartialSortingTransform returning zero-row -- chunks for input chunks w/o columns. select count() over () from numbers(4) where number < 2; diff --git a/tests/queries/0_stateless/01591_window_functions.sql b/tests/queries/0_stateless/01591_window_functions.sql index ffa44583579..59b1340bb2e 100644 --- a/tests/queries/0_stateless/01591_window_functions.sql +++ b/tests/queries/0_stateless/01591_window_functions.sql @@ -427,6 +427,16 @@ window w as (order by number range between 1 preceding and 1 following) order by number ; +-- to make nth_value return null for out-of-frame rows, cast the argument to +-- Nullable; otherwise, it returns default values. +SELECT + number, + nth_value(toNullable(number), 1) OVER w as firstValue, + nth_value(toNullable(number), 3) OVER w as thridValue +FROM numbers(5) +WINDOW w AS (ORDER BY number ASC) +; + -- In this case, we had a problem with PartialSortingTransform returning zero-row -- chunks for input chunks w/o columns. select count() over () from numbers(4) where number < 2; From 0e742487f1db001885ef0bcb9e1e0f4f8853811f Mon Sep 17 00:00:00 2001 From: ryzuo Date: Mon, 26 Jul 2021 13:31:45 +0800 Subject: [PATCH 6/6] Remove unnecessary emtpy argument check for nth_value as code review suggested --- src/Processors/Transforms/WindowTransform.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Processors/Transforms/WindowTransform.cpp b/src/Processors/Transforms/WindowTransform.cpp index 22911c9b774..566a931e76f 100644 --- a/src/Processors/Transforms/WindowTransform.cpp +++ b/src/Processors/Transforms/WindowTransform.cpp @@ -9,6 +9,7 @@ #include #include + namespace DB { @@ -1576,12 +1577,6 @@ struct WindowFunctionNthValue final : public WindowFunction "Function {} cannot be parameterized", name_); } - if (argument_types.empty()) - { - throw Exception(ErrorCodes::BAD_ARGUMENTS, - "Function {} takes at least one argument", name_); - } - if (argument_types.size() != 2) { throw Exception(ErrorCodes::BAD_ARGUMENTS,