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

Merge pull request #26072 from vdimir/fix-offset-check-in-window-frame

Browse Source 
Fix logical error with signed and unsigned offset in WindowFrame::checkValid
This commit is contained in:
alexey-milovidov 2021-07-09 03:06:15 +03:00 committed by GitHub
commit c98e131a81
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 23 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/Common/FieldVisitorsAccurateComparison.h
View File

@ -117,4 +117,16 @@ public:
}
};
class FieldVisitorAccurateLessOrEqual : public StaticVisitor<bool>
{
public:
template <typename T, typename U>
bool operator()(const T & l, const U & r) const
{
auto less_cmp = FieldVisitorAccurateLess();
return !less_cmp(r, l);
}
};
}

10
src/Interpreters/WindowDescription.cpp
View File

@ -1,6 +1,7 @@
#include <Interpreters/WindowDescription.h>
#include <Core/Field.h>
#include <Common/FieldVisitorsAccurateComparison.h>
#include <Common/FieldVisitorToString.h>
#include <IO/Operators.h>
#include <Parsers/ASTFunction.h>
@ -99,7 +100,7 @@ void WindowFrame::checkValid() const
&& begin_offset.get<Int64>() < INT_MAX))
{
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Frame start offset for '{}' frame must be a nonnegative 32-bit integer, '{}' of type '{}' given.",
"Frame start offset for '{}' frame must be a nonnegative 32-bit integer, '{}' of type '{}' given",
toString(type),
applyVisitor(FieldVisitorToString(), begin_offset),
Field::Types::toString(begin_offset.getType()));
@ -112,7 +113,7 @@ void WindowFrame::checkValid() const
&& end_offset.get<Int64>() < INT_MAX))
{
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Frame end offset for '{}' frame must be a nonnegative 32-bit integer, '{}' of type '{}' given.",
"Frame end offset for '{}' frame must be a nonnegative 32-bit integer, '{}' of type '{}' given",
toString(type),
applyVisitor(FieldVisitorToString(), end_offset),
Field::Types::toString(end_offset.getType()));
@ -160,7 +161,8 @@ void WindowFrame::checkValid() const
bool begin_less_equal_end;
if (begin_preceding && end_preceding)
{
begin_less_equal_end = begin_offset >= end_offset;
/// we can't compare Fields using operator<= if fields have different types
begin_less_equal_end = applyVisitor(FieldVisitorAccurateLessOrEqual(), end_offset, begin_offset);
}
else if (begin_preceding && !end_preceding)
{
@ -172,7 +174,7 @@ void WindowFrame::checkValid() const
}
else /* if (!begin_preceding && !end_preceding) */
{
begin_less_equal_end = begin_offset <= end_offset;
begin_less_equal_end = applyVisitor(FieldVisitorAccurateLessOrEqual(), begin_offset, end_offset);
}
if (!begin_less_equal_end)

2
tests/queries/0_stateless/01571_window_functions.reference
View File

@ -13,3 +13,5 @@ select count() over (rows between 1 + 1 preceding and 1 + 1 following) from numb
5
4
3
-- signed and unsigned in offset do not cause logical error
select count() over (rows between 2 following and 1 + -1 following) FROM numbers(10); -- { serverError 36 }

3
tests/queries/0_stateless/01571_window_functions.sql
View File

@ -4,3 +4,6 @@ set allow_experimental_window_functions = 1;
-- expressions in window frame
select count() over (rows between 1 + 1 preceding and 1 + 1 following) from numbers(10);
-- signed and unsigned in offset do not cause logical error
select count() over (rows between 2 following and 1 + -1 following) FROM numbers(10); -- { serverError 36 }