rewrote everything again

programs/client/TestHint.h

@@ -23,18 +23,26 @@ namespace ErrorCodes
 class TestHint
 {
 public:
-    TestHint(bool enabled_, const String & query_)
-    : enabled(enabled_)
-    , query(query_)
+    TestHint(bool enabled_, const String & query_) :
+        query(query_)
     {
         if (!enabled_)
             return;
 
+        // Don't parse error hints in leading comments, because it feels weird.
+        // Leading 'echo' hint is OK.
+        bool is_leading_hint = true;
+
         Lexer lexer(query.data(), query.data() + query.size());
 
         for (Token token = lexer.nextToken(); !token.isEnd(); token = lexer.nextToken())
         {
-            if (token.type == TokenType::Comment)
+            if (token.type != TokenType::Comment
+                && token.type != TokenType::Whitespace)
+            {
+                is_leading_hint = false;
+            }
+            else if (token.type == TokenType::Comment)
             {
                 String comment(token.begin, token.begin + token.size());
 
@@ -47,7 +55,7 @@ public:
                         if (pos_end != String::npos)
                         {
                             String hint(comment.begin() + pos_start + 1, comment.begin() + pos_end);
-                            parse(hint);
+                            parse(hint, is_leading_hint);
                         }
                     }
                 }
@@ -55,46 +63,17 @@ public:
         }
     }
 
-    /// @returns true if it's possible to continue without reconnect
-    bool checkActual(int & actual_server_error, int & actual_client_error,
-                     bool & got_exception, std::unique_ptr<Exception> & last_exception) const
-    {
-        if (!enabled)
-            return true;
-
-        if (allErrorsExpected(actual_server_error, actual_client_error))
-        {
-            got_exception = false;
-            last_exception.reset();
-            actual_server_error = 0;
-            actual_client_error = 0;
-            return false;
-        }
-
-        if (lostExpectedError(actual_server_error, actual_client_error))
-        {
-            std::cerr << "Success when error expected in query: " << query << "It expects server error "
-                << server_error << ", client error " << client_error << "." << std::endl;
-            got_exception = true;
-            last_exception = std::make_unique<Exception>("Success when error expected", ErrorCodes::UNEXPECTED_ERROR_CODE); /// return error to OS
-            return false;
-        }
-
-        return true;
-    }
-
     int serverError() const { return server_error; }
     int clientError() const { return client_error; }
     bool echoQueries() const { return echo; }
 
 private:
-    bool enabled = false;
     const String & query;
     int server_error = 0;
     int client_error = 0;
     bool echo = false;
 
-    void parse(const String & hint)
+    void parse(const String & hint, bool is_leading_hint)
     {
         std::stringstream ss;       // STYLE_CHECK_ALLOW_STD_STRING_STREAM
         ss << hint;
@@ -106,11 +85,15 @@ private:
             if (ss.eof())
                 break;
 
-            if (item == "serverError")
-                ss >> server_error;
-            else if (item == "clientError")
-                ss >> client_error;
-            else if (item == "echo")
+            if (!is_leading_hint)
+            {
+                if (item == "serverError")
+                    ss >> server_error;
+                else if (item == "clientError")
+                    ss >> client_error;
+            }
+
+            if (item == "echo")
                 echo = true;
         }
     }

src/Parsers/parseQuery.cpp

@@ -258,63 +258,12 @@ ASTPtr tryParseQuery(
     ASTPtr res;
     const bool parse_res = parser.parse(token_iterator, res, expected);
     const auto last_token = token_iterator.max();
+    _out_query_end = last_token.end;
 
     ASTInsertQuery * insert = nullptr;
     if (parse_res)
         insert = res->as<ASTInsertQuery>();
 
-    // We have to do some extra work to determine where the next query begins,
-    // preserving its leading comments.
-    // The query we just parsed may contain a test hint comment in the same line,
-    // e.g. select nonexistent_column; -- { serverError 12345 }.
-    // We must add this comment to the query text, so that it is handled by the
-    // test hint parser. It is important to do this before handling syntax errors,
-    // so that we can expect for syntax client errors in test hints.
-    // The token iterator skips comments and whitespace, so we have to find the
-    // newline in the string manually. If it's earlier than the next significant
-    // token, it means that the text before newline is some trailing whitespace
-    // or comment, and we should add it to our query.
-    const auto * newline = find_first_symbols<'\n'>(last_token.end, all_queries_end);
-
-    Tokens next_query_tokens(last_token.end, all_queries_end, max_query_size);
-    IParser::Pos next_query_iterator(next_query_tokens, max_parser_depth);
-
-    while (next_query_iterator.isValid()
-        && next_query_iterator->type == TokenType::Semicolon)
-    {
-        ++next_query_iterator;
-    }
-    const char * next_query_begin = next_query_iterator->begin;
-
-    // 1) We must always include the entire line in case of parse errors, because
-    // we don't know where the query ends. OTOH, we can't always include it,
-    // because there might be several valid queries on one line.
-    // 2) We must include the entire line for INSERT queries with inline data.
-    // We don't know where the data ends, but there might be a test hint on the
-    // same line in case of values format.
-    // 3) We include the entire line if the next query starts after it. This is
-    // a generic case of trailing in-line comment.
-    if (!parse_res
-        || (insert && insert->data)
-        || newline < next_query_begin)
-    {
-        assert(newline >= last_token.end);
-        _out_query_end = newline;
-    }
-    else
-    {
-        _out_query_end = last_token.end;
-    }
-
-    // Another corner case -- test hint comment that is ended by the end of data.
-    // Reproduced by test cases without newline at end, your editor may be
-    // configured to add one on save.
-    if (!next_query_iterator.isValid())
-    {
-        assert(next_query_iterator->end == all_queries_end);
-        _out_query_end = all_queries_end;
-    }
-
     // If parsed query ends at data for insertion. Data for insertion could be
     // in any format and not necessary be lexical correct, so we can't perform
     // most of the checks.
@@ -331,7 +280,6 @@ ASTPtr tryParseQuery(
         return res;
     }
 
-
     // More granular checks for queries other than INSERT w/inline data.
     /// Lexical error
     if (last_token.isError())

src/Parsers/parseQuery.h

@@ -9,7 +9,7 @@ namespace DB
 /// Parse query or set 'out_error_message'.
 ASTPtr tryParseQuery(
     IParser & parser,
-    const char * & pos,                /// Moved to end of parsed fragment.
+    const char * & _out_query_end, // query start as input parameter, query end as output
     const char * end,
     std::string & out_error_message,
     bool hilite,

tests/queries/0_stateless/01564_test_hint_woes.reference

@@ -21,3 +21,5 @@ insert into values_01564 values (11); -- { serverError 469 }
 select nonexistent column; -- { serverError 47 }
 -- query after values on the same line
 insert into values_01564 values (1); select 1;
+select 1;
+1

tests/queries/0_stateless/01564_test_hint_woes.sql

@@ -33,8 +33,14 @@ select nonexistent column; -- { serverError 47 }
 -- query after values on the same line
 insert into values_01564 values (1); select 1;
 
+-- even this works (not sure why we need it lol)
+-- insert into values_01564 values (11) /*{ serverError 469 }*/; select 1;
+
 -- syntax error, where the last token we can parse is long before the semicolon.
 select this is too many words for an alias; -- { clientError 62 }
---OPTIMIZE TABLE values_01564 DEDUPLICATE BY; -- { clientError 62 }
---OPTIMIZE TABLE values_01564 DEDUPLICATE BY a EXCEPT a; -- { clientError 62 }
---select 'a' || distinct one || 'c' from system.one; -- { clientError 62 }
+OPTIMIZE TABLE values_01564 DEDUPLICATE BY; -- { clientError 62 }
+OPTIMIZE TABLE values_01564 DEDUPLICATE BY a EXCEPT a; -- { clientError 62 }
+select 'a' || distinct one || 'c' from system.one; -- { clientError 62 }
+
+-- the return code must be zero after the final query has failed with expected error
+insert into values_01564 values (11); -- { serverError 469 }

tests/queries/0_stateless/01591_window_functions.reference

 -- { echo }
 
 set allow_experimental_window_functions = 1;
-
-
 -- just something basic
 select number, count() over (partition by intDiv(number, 3) order by number) from numbers(10);
 0	1
@@ -15,8 +13,6 @@ select number, count() over (partition by intDiv(number, 3) order by number) fro
 7	2
 8	3
 9	1
-
-
 -- proper calculation across blocks
 select number, max(number) over (partition by intDiv(number, 3) order by number desc) from numbers(10) settings max_block_size = 2;
 2	2
@@ -29,12 +25,8 @@ select number, max(number) over (partition by intDiv(number, 3) order by number
 7	8
 6	8
 9	9
-
-
 -- not a window function
 select number, abs(number) over (partition by toString(intDiv(number, 3))) from numbers(10); -- { serverError 63 }
-
-
 -- no partition by
 select number, avg(number) over (order by number) from numbers(10);
 0	0
@@ -47,8 +39,6 @@ select number, avg(number) over (order by number) from numbers(10);
 7	3.5
 8	4
 9	4.5
-
-
 -- no order by
 select number, quantileExact(number) over (partition by intDiv(number, 3)) from numbers(10);
 0	0
@@ -61,8 +51,6 @@ select number, quantileExact(number) over (partition by intDiv(number, 3)) from
 7	7
 8	7
 9	9
-
-
 -- can add an alias after window spec
 select number, quantileExact(number) over (partition by intDiv(number, 3)) q from numbers(10);
 0	0
@@ -75,20 +63,14 @@ select number, quantileExact(number) over (partition by intDiv(number, 3)) q fro
 7	7
 8	7
 9	9
-
-
 -- can't reference it yet -- the window functions are calculated at the
 -- last stage of select, after all other functions.
 select q * 10, quantileExact(number) over (partition by intDiv(number, 3)) q from numbers(10); -- { serverError 47 }
-
-
 -- must work in WHERE if you wrap it in a subquery
 select * from (select count(*) over () c from numbers(3)) where c > 0;
 1
 2
 3
-
-
 -- should work in ORDER BY
 select number, max(number) over (partition by intDiv(number, 3) order by number desc) m from numbers(10) order by m desc, number;
 9	9
@@ -101,15 +83,11 @@ select number, max(number) over (partition by intDiv(number, 3) order by number
 0	2
 1	2
 2	2
-
-
 -- also works in ORDER BY if you wrap it in a subquery
 select * from (select count(*) over () c from numbers(3)) order by c;
 1
 2
 3
-
-
 -- Example with window function only in ORDER BY. Here we make a rank of all
 -- numbers sorted descending, and then sort by this rank descending, and must get
 -- the ascending order.
@@ -119,8 +97,6 @@ select * from (select * from numbers(5) order by rand()) order by count() over (
 2
 3
 4
-
-
 -- Aggregate functions as window function arguments. This query is semantically
 -- the same as the above one, only we replace `number` with
 -- `any(number) group by number` and so on.
@@ -130,18 +106,13 @@ select * from (select * from numbers(5) order by rand()) group by number order b
 2
 3
 4
-
 -- some more simple cases w/aggregate functions
 select sum(any(number)) over () from numbers(1);
 0
-
 select sum(any(number) + 1) over () from numbers(1);
 1
-
 select sum(any(number + 1)) over () from numbers(1);
 1
-
-
 -- different windows
 -- an explain test would also be helpful, but it's too immature now and I don't
 -- want to change reference all the time
@@ -177,8 +148,6 @@ select number, max(number) over (partition by intDiv(number, 3) order by number
 28	29	4
 29	29	5
 30	30	1
-
-
 -- two functions over the same window
 -- an explain test would also be helpful, but it's too immature now and I don't
 -- want to change reference all the time
@@ -190,34 +159,23 @@ select number, max(number) over (partition by intDiv(number, 3) order by number
 4	5	2
 5	5	1
 6	6	1
-
-
 -- check that we can work with constant columns
 select median(x) over (partition by x) from (select 1 x);
 1
-
-
 -- an empty window definition is valid as well
 select groupArray(number) over () from numbers(3);
 [0]
 [0,1]
 [0,1,2]
-
-
 -- This one tests we properly process the window  function arguments.
 -- Seen errors like 'column `1` not found' from count(1).
 select count(1) over (), max(number + 1) over () from numbers(3);
 1	3
-
-
 -- Should work in DISTINCT
 select distinct sum(0) over () from numbers(2);
 0
-
 select distinct any(number) over () from numbers(2);
 0
-
-
 -- Various kinds of aliases are properly substituted into various parts of window
 -- function definition.
 with number + 1 as x select intDiv(number, 3) as y, sum(x + y) over (partition by y order by x) from numbers(7);
@@ -228,13 +186,9 @@ with number + 1 as x select intDiv(number, 3) as y, sum(x + y) over (partition b
 1	11
 1	18
 2	9
-
-
 -- WINDOW clause
 select 1 window w1 as ();
 1
-
-
 select sum(number) over w1, sum(number) over w2
 from numbers(10)
 window
@@ -251,8 +205,6 @@ window
 28	13
 36	21
 45	9
-
-
 select
     sum(number) over w1,
     sum(number) over (partition by intDiv(number, 3))