fixes

docker/test/fuzzer/run-fuzzer.sh

@@ -119,7 +119,12 @@ continue
     # SC2012: Use find instead of ls to better handle non-alphanumeric filenames. They are all alphanumeric.
     # SC2046: Quote this to prevent word splitting. Actually I need word splitting.
     # shellcheck disable=SC2012,SC2046
-    clickhouse-client --query-fuzzer-runs=1000 --queries-file $(ls -1 ch/tests/queries/0_stateless/*.sql | sort -R) $NEW_TESTS_OPT \
+    clickhouse-client \
+        --receive_timeout=10 \
+        --receive_data_timeout_ms=10000 \
+        --query-fuzzer-runs=1000 \
+        --queries-file $(ls -1 ch/tests/queries/0_stateless/*.sql | sort -R) \
+        $NEW_TESTS_OPT \
         > >(tail -n 100000 > fuzzer.log) \
         2>&1 \
         || fuzzer_exit_code=$?

programs/client/QueryFuzzer.cpp

@@ -325,14 +325,14 @@ void QueryFuzzer::fuzzColumnLikeExpressionList(IAST * ast)
     // the generic recursion into IAST.children.
 }
 
-void QueryFuzzer::fuzzWindowFrame(WindowFrame & frame)
+void QueryFuzzer::fuzzWindowFrame(ASTWindowDefinition & def)
 {
     switch (fuzz_rand() % 40)
     {
         case 0:
         {
             const auto r = fuzz_rand() % 3;
-            frame.type = r == 0 ? WindowFrame::FrameType::Rows
+            def.frame_type = r == 0 ? WindowFrame::FrameType::Rows
                 : r == 1 ? WindowFrame::FrameType::Range
                     : WindowFrame::FrameType::Groups;
             break;
@@ -340,44 +340,80 @@ void QueryFuzzer::fuzzWindowFrame(WindowFrame & frame)
         case 1:
         {
             const auto r = fuzz_rand() % 3;
-            frame.begin_type = r == 0 ? WindowFrame::BoundaryType::Unbounded
+            def.frame_begin_type = r == 0 ? WindowFrame::BoundaryType::Unbounded
                 : r == 1 ? WindowFrame::BoundaryType::Current
                     : WindowFrame::BoundaryType::Offset;
+
+            if (def.frame_begin_type == WindowFrame::BoundaryType::Offset)
+            {
+                // The offsets are fuzzed normally through 'children'.
+                def.frame_begin_offset
+                    = std::make_shared<ASTLiteral>(getRandomField(0));
+                def.children.push_back(def.frame_begin_offset);
+            }
+            else
+            {
+                // Don't keep the offset if it is not used, because it will
+                // acquire random mutations that will surely make it invalid.
+                const auto old_size = def.children.size();
+                def.children.erase(
+                    std::remove(def.children.begin(), def.children.end(),
+                        def.frame_begin_offset),
+                    def.children.end());
+                assert(def.children.size() == old_size - 1
+                    || def.frame_begin_offset == nullptr);
+                def.frame_begin_offset = nullptr;
+            }
             break;
         }
         case 2:
         {
             const auto r = fuzz_rand() % 3;
-            frame.end_type = r == 0 ? WindowFrame::BoundaryType::Unbounded
+            def.frame_end_type = r == 0 ? WindowFrame::BoundaryType::Unbounded
                 : r == 1 ? WindowFrame::BoundaryType::Current
                     : WindowFrame::BoundaryType::Offset;
-            break;
+
-        }
+            if (def.frame_end_type == WindowFrame::BoundaryType::Offset)
-        case 3:
             {
-            frame.begin_offset = getRandomField(0).get<Int64>();
+                def.frame_end_offset
-            break;
+                    = std::make_shared<ASTLiteral>(getRandomField(0));
+                def.children.push_back(def.frame_end_offset);
             }
-        case 4:
+            else
             {
-            frame.end_offset = getRandomField(0).get<Int64>();
+                def.children.erase(
+                    std::remove(def.children.begin(), def.children.end(),
+                        def.frame_end_offset),
+                    def.children.end());
+                def.frame_end_offset = nullptr;
+            }
             break;
         }
         case 5:
         {
-            frame.begin_preceding = fuzz_rand() % 2;
+            def.frame_begin_preceding = fuzz_rand() % 2;
             break;
         }
         case 6:
         {
-            frame.end_preceding = fuzz_rand() % 2;
+            def.frame_end_preceding = fuzz_rand() % 2;
             break;
         }
         default:
             break;
     }
 
-    frame.is_default = (frame == WindowFrame{});
+    if (def.frame_type == WindowFrame::FrameType::Range
+        && def.frame_begin_type == WindowFrame::BoundaryType::Unbounded
+        && def.frame_begin_preceding == true
+        && def.frame_end_type == WindowFrame::BoundaryType::Current)
+    {
+        def.frame_is_default = true;
+    }
+    else
+    {
+        def.frame_is_default = false;
+    }
 }
 
 void QueryFuzzer::fuzz(ASTs & asts)
@@ -464,7 +500,7 @@ void QueryFuzzer::fuzz(ASTPtr & ast)
             auto & def = fn->window_definition->as<ASTWindowDefinition &>();
             fuzzColumnLikeExpressionList(def.partition_by.get());
             fuzzOrderByList(def.order_by.get());
-            fuzzWindowFrame(def.frame);
+            fuzzWindowFrame(def);
         }
 
         fuzz(fn->children);

programs/client/QueryFuzzer.h

@@ -17,7 +17,7 @@ namespace DB
 
 class ASTExpressionList;
 class ASTOrderByElement;
-struct WindowFrame;
+struct ASTWindowDefinition;
 
 /*
  * This is an AST-based query fuzzer that makes random modifications to query
@@ -69,7 +69,7 @@ struct QueryFuzzer
     void fuzzOrderByElement(ASTOrderByElement * elem);
     void fuzzOrderByList(IAST * ast);
     void fuzzColumnLikeExpressionList(IAST * ast);
-    void fuzzWindowFrame(WindowFrame & frame);
+    void fuzzWindowFrame(ASTWindowDefinition & def);
     void fuzz(ASTs & asts);
     void fuzz(ASTPtr & ast);
     void collectFuzzInfoMain(const ASTPtr ast);

src/Interpreters/ExpressionAnalyzer.cpp

@@ -568,22 +568,23 @@ void makeWindowDescriptionFromAST(const Context & context,
     desc.full_sort_description.insert(desc.full_sort_description.end(),
         desc.order_by.begin(), desc.order_by.end());
 
-    if (definition.frame.type != WindowFrame::FrameType::Rows
+    if (definition.frame_type != WindowFrame::FrameType::Rows
-        && definition.frame.type != WindowFrame::FrameType::Range)
+        && definition.frame_type != WindowFrame::FrameType::Range)
     {
-        std::string name = definition.frame.type == WindowFrame::FrameType::Rows
-            ? "ROWS"
-            : definition.frame.type == WindowFrame::FrameType::Groups
-                ? "GROUPS" : "RANGE";
-
         throw Exception(ErrorCodes::NOT_IMPLEMENTED,
             "Window frame '{}' is not implemented (while processing '{}')",
-            name, ast->formatForErrorMessage());
+            WindowFrame::toString(definition.frame_type),
+            ast->formatForErrorMessage());
     }
 
-    desc.frame = definition.frame;
+    desc.frame.is_default = definition.frame_is_default;
+    desc.frame.type = definition.frame_type;
+    desc.frame.begin_type = definition.frame_begin_type;
+    desc.frame.begin_preceding = definition.frame_begin_preceding;
+    desc.frame.end_type = definition.frame_end_type;
+    desc.frame.end_preceding = definition.frame_end_preceding;
 
-    if (definition.frame.end_type == WindowFrame::BoundaryType::Offset)
+    if (definition.frame_end_type == WindowFrame::BoundaryType::Offset)
     {
         auto [value, _] = evaluateConstantExpression(
             definition.frame_end_offset,
@@ -591,7 +592,7 @@ void makeWindowDescriptionFromAST(const Context & context,
         desc.frame.end_offset = value;
     }
 
-    if (definition.frame.begin_type == WindowFrame::BoundaryType::Offset)
+    if (definition.frame_begin_type == WindowFrame::BoundaryType::Offset)
     {
         auto [value, _] = evaluateConstantExpression(
             definition.frame_begin_offset,

src/Parsers/ASTWindowDefinition.cpp

@@ -26,7 +26,12 @@ ASTPtr ASTWindowDefinition::clone() const
         result->children.push_back(result->order_by);
     }
 
-    result->frame = frame;
+    result->frame_is_default = frame_is_default;
+    result->frame_type = frame_type;
+    result->frame_begin_type = frame_begin_type;
+    result->frame_begin_preceding = frame_begin_preceding;
+    result->frame_end_type = frame_end_type;
+    result->frame_end_preceding = frame_end_preceding;
 
     if (frame_begin_offset)
     {
@@ -87,19 +92,19 @@ void ASTWindowDefinition::formatImpl(const FormatSettings & settings,
         need_space = true;
     }
 
-    if (!frame.is_default)
+    if (!frame_is_default)
     {
         if (need_space)
         {
             settings.ostr << " ";
         }
 
-        settings.ostr << WindowFrame::toString(frame.type) << " BETWEEN ";
+        settings.ostr << WindowFrame::toString(frame_type) << " BETWEEN ";
-        if (frame.begin_type == WindowFrame::BoundaryType::Current)
+        if (frame_begin_type == WindowFrame::BoundaryType::Current)
         {
             settings.ostr << "CURRENT ROW";
         }
-        else if (frame.begin_type == WindowFrame::BoundaryType::Unbounded)
+        else if (frame_begin_type == WindowFrame::BoundaryType::Unbounded)
         {
             settings.ostr << "UNBOUNDED PRECEDING";
         }
@@ -107,14 +112,14 @@ void ASTWindowDefinition::formatImpl(const FormatSettings & settings,
         {
             frame_begin_offset->formatImpl(settings, state, format_frame);
             settings.ostr << " "
-                << (!frame.begin_preceding ? "FOLLOWING" : "PRECEDING");
+                << (!frame_begin_preceding ? "FOLLOWING" : "PRECEDING");
         }
         settings.ostr << " AND ";
-        if (frame.end_type == WindowFrame::BoundaryType::Current)
+        if (frame_end_type == WindowFrame::BoundaryType::Current)
         {
             settings.ostr << "CURRENT ROW";
         }
-        else if (frame.end_type == WindowFrame::BoundaryType::Unbounded)
+        else if (frame_end_type == WindowFrame::BoundaryType::Unbounded)
         {
             settings.ostr << "UNBOUNDED FOLLOWING";
         }
@@ -122,7 +127,7 @@ void ASTWindowDefinition::formatImpl(const FormatSettings & settings,
         {
             frame_end_offset->formatImpl(settings, state, format_frame);
             settings.ostr << " "
-                << (!frame.end_preceding ? "FOLLOWING" : "PRECEDING");
+                << (!frame_end_preceding ? "FOLLOWING" : "PRECEDING");
         }
     }
 }

src/Parsers/ASTWindowDefinition.h

@@ -16,12 +16,14 @@ struct ASTWindowDefinition : public IAST
 
     ASTPtr order_by;
 
-    // Be careful: offsets can contain constant expressions such as INTERVAL 1 DAY,
+    bool frame_is_default = true;
-    // that are evaluated later by ExpressionAnalyzer. The WindowFrame struct
+    WindowFrame::FrameType frame_type = WindowFrame::FrameType::Range;
-    // can be incomplete after parsing.
+    WindowFrame::BoundaryType frame_begin_type = WindowFrame::BoundaryType::Unbounded;
-    WindowFrame frame;
     ASTPtr frame_begin_offset;
+    bool frame_begin_preceding = true;
+    WindowFrame::BoundaryType frame_end_type = WindowFrame::BoundaryType::Current;
     ASTPtr frame_end_offset;
+    bool frame_end_preceding = false;
 
     ASTPtr clone() const override;
 

src/Parsers/ExpressionElementParsers.cpp

@@ -530,23 +530,23 @@ static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & p
     ParserKeyword keyword_groups("GROUPS");
     ParserKeyword keyword_range("RANGE");
 
-    node->frame.is_default = false;
+    node->frame_is_default = false;
     if (keyword_rows.ignore(pos, expected))
     {
-        node->frame.type = WindowFrame::FrameType::Rows;
+        node->frame_type = WindowFrame::FrameType::Rows;
     }
     else if (keyword_groups.ignore(pos, expected))
     {
-        node->frame.type = WindowFrame::FrameType::Groups;
+        node->frame_type = WindowFrame::FrameType::Groups;
     }
     else if (keyword_range.ignore(pos, expected))
     {
-        node->frame.type = WindowFrame::FrameType::Range;
+        node->frame_type = WindowFrame::FrameType::Range;
     }
     else
     {
         /* No frame clause. */
-        node->frame.is_default = true;
+        node->frame_is_default = true;
         return true;
     }
 
@@ -565,19 +565,19 @@ static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & p
 
     if (keyword_current_row.ignore(pos, expected))
     {
-        node->frame.begin_type = WindowFrame::BoundaryType::Current;
+        node->frame_begin_type = WindowFrame::BoundaryType::Current;
     }
     else
     {
         ParserExpression parser_expression;
         if (keyword_unbounded.ignore(pos, expected))
         {
-            node->frame.begin_type = WindowFrame::BoundaryType::Unbounded;
+            node->frame_begin_type = WindowFrame::BoundaryType::Unbounded;
         }
         else if (parser_expression.parse(pos, node->frame_begin_offset, expected))
         {
             // We will evaluate the expression for offset expression later.
-            node->frame.begin_type = WindowFrame::BoundaryType::Offset;
+            node->frame_begin_type = WindowFrame::BoundaryType::Offset;
         }
         else
         {
@@ -586,12 +586,12 @@ static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & p
 
         if (keyword_preceding.ignore(pos, expected))
         {
-            node->frame.begin_preceding = true;
+            node->frame_begin_preceding = true;
         }
         else if (keyword_following.ignore(pos, expected))
         {
-            node->frame.begin_preceding = false;
+            node->frame_begin_preceding = false;
-            if (node->frame.begin_type == WindowFrame::BoundaryType::Unbounded)
+            if (node->frame_begin_type == WindowFrame::BoundaryType::Unbounded)
             {
                 throw Exception(ErrorCodes::BAD_ARGUMENTS,
                     "Frame start cannot be UNBOUNDED FOLLOWING");
@@ -612,19 +612,19 @@ static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & p
 
         if (keyword_current_row.ignore(pos, expected))
         {
-            node->frame.end_type = WindowFrame::BoundaryType::Current;
+            node->frame_end_type = WindowFrame::BoundaryType::Current;
         }
         else
         {
             ParserExpression parser_expression;
             if (keyword_unbounded.ignore(pos, expected))
             {
-                node->frame.end_type = WindowFrame::BoundaryType::Unbounded;
+                node->frame_end_type = WindowFrame::BoundaryType::Unbounded;
             }
             else if (parser_expression.parse(pos, node->frame_end_offset, expected))
             {
                 // We will evaluate the expression for offset expression later.
-                node->frame.end_type = WindowFrame::BoundaryType::Offset;
+                node->frame_end_type = WindowFrame::BoundaryType::Offset;
             }
             else
             {
@@ -633,8 +633,8 @@ static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & p
 
             if (keyword_preceding.ignore(pos, expected))
             {
-                node->frame.end_preceding = true;
+                node->frame_end_preceding = true;
-                if (node->frame.end_type == WindowFrame::BoundaryType::Unbounded)
+                if (node->frame_end_type == WindowFrame::BoundaryType::Unbounded)
                 {
                     throw Exception(ErrorCodes::BAD_ARGUMENTS,
                         "Frame end cannot be UNBOUNDED PRECEDING");
@@ -643,7 +643,7 @@ static bool tryParseFrameDefinition(ASTWindowDefinition * node, IParser::Pos & p
             else if (keyword_following.ignore(pos, expected))
             {
                 // Positive offset or UNBOUNDED FOLLOWING.
-                node->frame.end_preceding = false;
+                node->frame_end_preceding = false;
             }
             else
             {