From 9e82ed1d048f509a961a553aad6df1108d20a6f8 Mon Sep 17 00:00:00 2001 From: Larry Luo Date: Wed, 18 Jan 2023 22:08:20 -0800 Subject: [PATCH] Fixed clang-tidy and Arch tests --- .../KustoFunctions/IParserKQLFunction.cpp | 56 +++++++++---------- .../KustoFunctions/KQLCastingFunctions.cpp | 4 +- .../KustoFunctions/KQLDataTypeFunctions.cpp | 4 +- .../KustoFunctions/KQLStringFunctions.cpp | 26 ++++----- .../Kusto/ParserKQLDateTypeTimespan.cpp | 28 +++++----- src/Parsers/Kusto/ParserKQLMVExpand.cpp | 6 +- src/Parsers/Kusto/ParserKQLMakeSeries.cpp | 2 +- src/Parsers/Kusto/ParserKQLMakeSeries.h | 6 +- src/Parsers/Kusto/ParserKQLQuery.cpp | 2 +- src/Parsers/Kusto/ParserKQLSummarize.cpp | 4 +- .../0_stateless/02366_kql_summarize.sql | 16 +++--- 11 files changed, 77 insertions(+), 77 deletions(-) diff --git a/src/Parsers/Kusto/KustoFunctions/IParserKQLFunction.cpp b/src/Parsers/Kusto/KustoFunctions/IParserKQLFunction.cpp index 5acd794612e..0bb35c05105 100644 --- a/src/Parsers/Kusto/KustoFunctions/IParserKQLFunction.cpp +++ b/src/Parsers/Kusto/KustoFunctions/IParserKQLFunction.cpp @@ -128,8 +128,8 @@ String IParserKQLFunction::generateUniqueIdentifier() String IParserKQLFunction::getArgument(const String & function_name, DB::IParser::Pos & pos, const ArgumentState argument_state) { - if (auto optionalArgument = getOptionalArgument(function_name, pos, argument_state)) - return std::move(*optionalArgument); + if (auto optional_argument = getOptionalArgument(function_name, pos, argument_state)) + return std::move(*optional_argument); throw Exception(std::format("Required argument was not provided in {}", function_name), ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); } @@ -187,7 +187,7 @@ String IParserKQLFunction::getConvertedArgument(const String & fn_name, IParser: if (pos->type == TokenType::Comma || pos->type == TokenType::ClosingRoundBracket || pos->type == TokenType::ClosingSquareBracket) break; } - for (auto token : tokens) + for (auto const & token : tokens) converted_arg = converted_arg.empty() ? token : converted_arg + " " + token; return converted_arg; @@ -342,31 +342,31 @@ String IParserKQLFunction::getExpression(IParser::Pos & pos) int IParserKQLFunction::getNullCounts(String arg) { size_t index = 0; - int nullCounts = 0; - for (size_t i = 0; i < arg.size(); i++) + int null_counts = 0; + for (char & i : arg) { - if (arg[i] == 'n') - arg[i] = 'N'; - if (arg[i] == 'u') - arg[i] = 'U'; - if (arg[i] == 'l') - arg[i] = 'L'; + if (i == 'n') + i = 'N'; + if (i == 'u') + i = 'U'; + if (i == 'l') + i = 'L'; } while ((index = arg.find("NULL", index)) != std::string::npos) { index += 4; - nullCounts += 1; + null_counts += 1; } - return nullCounts; + return null_counts; } int IParserKQLFunction::IParserKQLFunction::getArrayLength(String arg) { int array_length = 0; bool comma_found = false; - for (size_t i = 0; i < arg.size(); i++) + for (char i : arg) { - if (arg[i] == ',') + if (i == ',') { comma_found = true; array_length += 1; @@ -389,7 +389,7 @@ String IParserKQLFunction::ArraySortHelper(String & out, IParser::Pos & pos, boo reverse = "Reverse"; ++pos; String first_arg = getConvertedArgument(fn_name, pos); - int nullCount = getNullCounts(first_arg); + int null_count = getNullCounts(first_arg); if (pos->type == TokenType::Comma) ++pos; out = "array("; @@ -423,14 +423,14 @@ String IParserKQLFunction::ArraySortHelper(String & out, IParser::Pos & pos, boo out += "array" + reverse + "Sort(" + first_arg + ")"; } - if (argument_list.size() > 0) + if (!argument_list.empty()) { String temp_first_arg = first_arg; int first_arg_length = getArrayLength(temp_first_arg); - if (nullCount > 0 && expr.empty()) + if (null_count > 0 && expr.empty()) expr = "true"; - if (nullCount > 0) + if (null_count > 0) first_arg = "if (" + expr + ", array" + reverse + "Sort(" + first_arg + "), concat(arraySlice(array" + reverse + "Sort(" + first_arg + ") as as1, indexOf(as1, NULL) as len1 ), arraySlice( as1, 1, len1-1) ) )"; else @@ -438,19 +438,19 @@ String IParserKQLFunction::ArraySortHelper(String & out, IParser::Pos & pos, boo out += first_arg; - for (size_t i = 0; i < argument_list.size(); i++) + for (auto & i : argument_list) { out += " , "; - if (first_arg_length != getArrayLength(argument_list[i])) + if (first_arg_length != getArrayLength(i)) out += "array(NULL)"; - else if (nullCount > 0) - out += "If (" + expr + "," + "array" + reverse + "Sort((x, y) -> y, " + argument_list[i] + "," + temp_first_arg - + "), arrayConcat(arraySlice(" + "array" + reverse + "Sort((x, y) -> y, " + argument_list[i] + "," + temp_first_arg - + ") , length(" + temp_first_arg + ") - " + std::to_string(nullCount) + " + 1) , arraySlice(" + "array" + reverse - + "Sort((x, y) -> y, " + argument_list[i] + "," + temp_first_arg + ") , 1, length(" + temp_first_arg + ") - " - + std::to_string(nullCount) + ") ) )"; + else if (null_count > 0) + out += "If (" + expr + "," + "array" + reverse + "Sort((x, y) -> y, " + i + "," + temp_first_arg + + "), arrayConcat(arraySlice(" + "array" + reverse + "Sort((x, y) -> y, " + i + "," + temp_first_arg + + ") , length(" + temp_first_arg + ") - " + std::to_string(null_count) + " + 1) , arraySlice(" + "array" + reverse + + "Sort((x, y) -> y, " + i + "," + temp_first_arg + ") , 1, length(" + temp_first_arg + ") - " + + std::to_string(null_count) + ") ) )"; else - out += "array" + reverse + "Sort((x, y) -> y, " + argument_list[i] + "," + temp_first_arg + ")"; + out += "array" + reverse + "Sort((x, y) -> y, " + i + "," + temp_first_arg + ")"; } } out += " )"; diff --git a/src/Parsers/Kusto/KustoFunctions/KQLCastingFunctions.cpp b/src/Parsers/Kusto/KustoFunctions/KQLCastingFunctions.cpp index d591ca39463..a6a7b2ee320 100644 --- a/src/Parsers/Kusto/KustoFunctions/KQLCastingFunctions.cpp +++ b/src/Parsers/Kusto/KustoFunctions/KQLCastingFunctions.cpp @@ -142,7 +142,7 @@ bool ToDecimal::convertImpl(String & out, IParser::Pos & pos) out = "NULL"; else if (std::regex_match(res, expr)) { - auto exponential_pos = res.find("e"); + auto exponential_pos = res.find('e'); if (res[exponential_pos + 1] == '+' || res[exponential_pos + 1] == '-') scale = std::stoi(res.substr(exponential_pos + 2, res.length())); else @@ -152,7 +152,7 @@ bool ToDecimal::convertImpl(String & out, IParser::Pos & pos) } else { - if (const auto dot_pos = res.find("."); dot_pos != String::npos) + if (const auto dot_pos = res.find('.'); dot_pos != String::npos) { const auto tmp = res.substr(0, dot_pos - 1); const auto tmp_length = static_cast(std::ssize(tmp)); diff --git a/src/Parsers/Kusto/KustoFunctions/KQLDataTypeFunctions.cpp b/src/Parsers/Kusto/KustoFunctions/KQLDataTypeFunctions.cpp index 8bf31a5bcf4..d745aeb2c0c 100644 --- a/src/Parsers/Kusto/KustoFunctions/KQLDataTypeFunctions.cpp +++ b/src/Parsers/Kusto/KustoFunctions/KQLDataTypeFunctions.cpp @@ -232,7 +232,7 @@ bool DatatypeDecimal::convertImpl(String & out, IParser::Pos & pos) if (std::regex_match(arg, expr)) { - auto exponential_pos = arg.find("e"); + auto exponential_pos = arg.find('e'); if (arg[exponential_pos + 1] == '+' || arg[exponential_pos + 1] == '-') scale = std::stoi(arg.substr(exponential_pos + 2, arg.length())); else @@ -242,7 +242,7 @@ bool DatatypeDecimal::convertImpl(String & out, IParser::Pos & pos) return true; } - if (const auto dot_pos = arg.find("."); dot_pos != String::npos) + if (const auto dot_pos = arg.find('.'); dot_pos != String::npos) { const auto length = static_cast(std::ssize(arg.substr(0, dot_pos - 1))); scale = std::max(precision - length, 0); diff --git a/src/Parsers/Kusto/KustoFunctions/KQLStringFunctions.cpp b/src/Parsers/Kusto/KustoFunctions/KQLStringFunctions.cpp index 0615f5228b3..f14440e5255 100644 --- a/src/Parsers/Kusto/KustoFunctions/KQLStringFunctions.cpp +++ b/src/Parsers/Kusto/KustoFunctions/KQLStringFunctions.cpp @@ -462,19 +462,19 @@ bool ParseURL::convertImpl(String & out, IParser::Pos & pos) ++pos; const String url = getConvertedArgument(fn_name, pos); - const String scheme = std::format("concat('\"Scheme\":\"', protocol({0}),'\"')", url); - const String host = std::format("concat('\"Host\":\"', domain({0}),'\"')", url); - const String port = std::format("concat('\"Port\":\"', toString(port({0})),'\"')", url); - const String path = std::format("concat('\"Path\":\"', path({0}),'\"')", url); + const String scheme = std::format(R"(concat('"Scheme":"', protocol({0}),'"'))", url); + const String host = std::format(R"(concat('"Host":"', domain({0}),'"'))", url); + const String port = std::format(R"(concat('"Port":"', toString(port({0})),'"'))", url); + const String path = std::format(R"(concat('"Path":"', path({0}),'"'))", url); const String username_pwd = std::format("netloc({0})", url); const String query_string = std::format("queryString({0})", url); - const String fragment = std::format("concat('\"Fragment\":\"',fragment({0}),'\"')", url); + const String fragment = std::format(R"(concat('"Fragment":"',fragment({0}),'"'))", url); const String username = std::format( - "concat('\"Username\":\"', arrayElement(splitByChar(':',arrayElement(splitByChar('@',{0}) ,1)),1),'\"')", username_pwd); + R"(concat('"Username":"', arrayElement(splitByChar(':',arrayElement(splitByChar('@',{0}) ,1)),1),'"'))", username_pwd); const String password = std::format( - "concat('\"Password\":\"', arrayElement(splitByChar(':',arrayElement(splitByChar('@',{0}) ,1)),2),'\"')", username_pwd); + R"(concat('"Password":"', arrayElement(splitByChar(':',arrayElement(splitByChar('@',{0}) ,1)),2),'"'))", username_pwd); const String query_parameters = std::format( - "concat('\"Query Parameters\":', concat('{{\"', replace(replace({}, '=', '\":\"'),'&','\",\"') ,'\"}}'))", query_string); + R"(concat('"Query Parameters":', concat('{{"', replace(replace({}, '=', '":"'),'&','","') ,'"}}')))", query_string); out = std::format( "concat('{{',{},',',{},',',{},',',{},',',{},',',{},',',{},',',{},'}}')", @@ -499,7 +499,7 @@ bool ParseURLQuery::convertImpl(String & out, IParser::Pos & pos) const String query_string = std::format("if (position({},'?') > 0, queryString({}), {})", query, query, query); const String query_parameters = std::format( - "concat('\"Query Parameters\":', concat('{{\"', replace(replace({}, '=', '\":\"'),'&','\",\"') ,'\"}}'))", query_string); + R"(concat('"Query Parameters":', concat('{{"', replace(replace({}, '=', '":"'),'&','","') ,'"}}')))", query_string); out = std::format("concat('{{',{},'}}')", query_parameters); return true; } @@ -666,21 +666,21 @@ bool SubString::convertImpl(String & out, IParser::Pos & pos) String source = getConvertedArgument(fn_name, pos); ++pos; - String startingIndex = getConvertedArgument(fn_name, pos); + String starting_index = getConvertedArgument(fn_name, pos); if (pos->type == TokenType::Comma) { ++pos; auto length = getConvertedArgument(fn_name, pos); - if (startingIndex.empty()) + if (starting_index.empty()) throw Exception("number of arguments do not match in function: " + fn_name, ErrorCodes::SYNTAX_ERROR); else - out = "if(toInt64(length(" + source + ")) <= 0, '', substr(" + source + ", " + "((" + startingIndex + "% toInt64(length(" + out = "if(toInt64(length(" + source + ")) <= 0, '', substr(" + source + ", " + "((" + starting_index + "% toInt64(length(" + source + ")) + toInt64(length(" + source + "))) % toInt64(length(" + source + "))) + 1, " + length + ") )"; } else - out = "if(toInt64(length(" + source + ")) <= 0, '', substr(" + source + "," + "((" + startingIndex + "% toInt64(length(" + source + out = "if(toInt64(length(" + source + ")) <= 0, '', substr(" + source + "," + "((" + starting_index + "% toInt64(length(" + source + ")) + toInt64(length(" + source + "))) % toInt64(length(" + source + "))) + 1))"; return true; diff --git a/src/Parsers/Kusto/ParserKQLDateTypeTimespan.cpp b/src/Parsers/Kusto/ParserKQLDateTypeTimespan.cpp index d3892bfc246..4295e42f9ac 100644 --- a/src/Parsers/Kusto/ParserKQLDateTypeTimespan.cpp +++ b/src/Parsers/Kusto/ParserKQLDateTypeTimespan.cpp @@ -1,7 +1,7 @@ #include #include #include -#include +#include #include #include #include @@ -52,7 +52,7 @@ double ParserKQLDateTypeTimespan ::toSeconds() bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) { - std::unordered_map TimespanSuffixes + std::unordered_map timespan_suffixes = {{"d", KQLTimespanUint::day}, {"day", KQLTimespanUint::day}, {"days", KQLTimespanUint::day}, @@ -94,9 +94,9 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) const char * ptr = text.c_str(); bool sign = false; - auto scanDigit = [&](const char * start) -> int + auto scan_digit = [&](const char * start) -> int { - auto index = start; + const auto * index = start; while (isdigit(*index)) ++index; return index > start ? static_cast(index - start) : -1; @@ -106,7 +106,7 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) sign = true; ++ptr; } - auto number_len = scanDigit(ptr); + auto number_len = scan_digit(ptr); if (number_len <= 0) return false; @@ -114,11 +114,11 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) if (*(ptr + number_len) == '.') { - auto fractionLen = scanDigit(ptr + number_len + 1); - if (fractionLen >= 0) + auto fraction_len = scan_digit(ptr + number_len + 1); + if (fraction_len >= 0) { - hours = std::stoi(String(ptr + number_len + 1, ptr + number_len + 1 + fractionLen)); - number_len += fractionLen + 1; + hours = std::stoi(String(ptr + number_len + 1, ptr + number_len + 1 + fraction_len)); + number_len += fraction_len + 1; } } else if (*(ptr + number_len) == '\0') @@ -142,11 +142,11 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) String timespan_suffix(ptr + number_len, ptr + text.size()); trim(timespan_suffix); - if (TimespanSuffixes.find(timespan_suffix) == TimespanSuffixes.end()) + if (timespan_suffixes.find(timespan_suffix) == timespan_suffixes.end()) return false; time_span = std::stod(String(ptr, ptr + number_len)); - time_span_unit = TimespanSuffixes[timespan_suffix]; + time_span_unit = timespan_suffixes[timespan_suffix]; return true; } @@ -154,7 +154,7 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) if (hours > 23) return false; - auto min_len = scanDigit(ptr + number_len + 1); + auto min_len = scan_digit(ptr + number_len + 1); if (min_len < 0) return false; @@ -165,7 +165,7 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) number_len += min_len + 1; if (*(ptr + number_len) == ':') { - auto sec_len = scanDigit(ptr + number_len + 1); + auto sec_len = scan_digit(ptr + number_len + 1); if (sec_len > 0) { seconds = std::stoi(String(ptr + number_len + 1, ptr + number_len + 1 + sec_len)); @@ -175,7 +175,7 @@ bool ParserKQLDateTypeTimespan ::parseConstKQLTimespan(const String & text) number_len += sec_len + 1; if (*(ptr + number_len) == '.') { - sec_scale_len = scanDigit(ptr + number_len + 1); + sec_scale_len = scan_digit(ptr + number_len + 1); if (sec_scale_len > 0) { nanoseconds = std::stoi(String(ptr + number_len + 1, ptr + number_len + 1 + sec_scale_len)); diff --git a/src/Parsers/Kusto/ParserKQLMVExpand.cpp b/src/Parsers/Kusto/ParserKQLMVExpand.cpp index 6adda0675ee..2cf7d99809b 100644 --- a/src/Parsers/Kusto/ParserKQLMVExpand.cpp +++ b/src/Parsers/Kusto/ParserKQLMVExpand.cpp @@ -66,7 +66,7 @@ bool ParserKQLMVExpand::parseColumnArrayExprs(ColumnArrayExprs & column_array_ex expr_begin_pos = pos; } - auto addColumns = [&] + auto add_columns = [&] { column_array_expr = getExprFromToken(String(expr_begin_pos->begin, expr_end_pos->end), pos.max_depth); @@ -109,7 +109,7 @@ bool ParserKQLMVExpand::parseColumnArrayExprs(ColumnArrayExprs & column_array_ex expr_end_pos = pos; --expr_end_pos; } - addColumns(); + add_columns(); expr_begin_pos = pos; expr_end_pos = pos; ++expr_begin_pos; @@ -133,7 +133,7 @@ bool ParserKQLMVExpand::parseColumnArrayExprs(ColumnArrayExprs & column_array_ex expr_end_pos = pos; --expr_end_pos; } - addColumns(); + add_columns(); break; } } diff --git a/src/Parsers/Kusto/ParserKQLMakeSeries.cpp b/src/Parsers/Kusto/ParserKQLMakeSeries.cpp index 6719a7bd40b..a3727653049 100644 --- a/src/Parsers/Kusto/ParserKQLMakeSeries.cpp +++ b/src/Parsers/Kusto/ParserKQLMakeSeries.cpp @@ -215,7 +215,7 @@ bool ParserKQLMakeSeries ::makeSeries(KQLMakeSeries & kql_make_series, ASTPtr & ++pos; } String res; - for (auto token : group_expression_tokens) + for (auto const & token : group_expression_tokens) res = res + token + " "; return res; }; diff --git a/src/Parsers/Kusto/ParserKQLMakeSeries.h b/src/Parsers/Kusto/ParserKQLMakeSeries.h index 4beb7f9e302..ef7cc4976f6 100644 --- a/src/Parsers/Kusto/ParserKQLMakeSeries.h +++ b/src/Parsers/Kusto/ParserKQLMakeSeries.h @@ -42,9 +42,9 @@ protected: String main_query; }; - bool makeSeries(KQLMakeSeries & kql_make_series, ASTPtr & select_node, const uint32_t & max_depth); - bool parseAggregationColumns(AggregationColumns & aggregation_columns, Pos & pos); - bool parseFromToStepClause(FromToStepClause & from_to_step, Pos & pos); + static bool makeSeries(KQLMakeSeries & kql_make_series, ASTPtr & select_node, const uint32_t & max_depth); + static bool parseAggregationColumns(AggregationColumns & aggregation_columns, Pos & pos); + static bool parseFromToStepClause(FromToStepClause & from_to_step, Pos & pos); const char * getName() const override { return "KQL make-series"; } bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; diff --git a/src/Parsers/Kusto/ParserKQLQuery.cpp b/src/Parsers/Kusto/ParserKQLQuery.cpp index 9d31fc0ae1e..ed8c74cd8af 100644 --- a/src/Parsers/Kusto/ParserKQLQuery.cpp +++ b/src/Parsers/Kusto/ParserKQLQuery.cpp @@ -197,7 +197,7 @@ String ParserKQLBase::getExprFromToken(Pos & pos) tokens.push_back(alias); } - for (auto token : tokens) + for (auto const & token : tokens) res = res.empty() ? token : res + " " + token; return res; } diff --git a/src/Parsers/Kusto/ParserKQLSummarize.cpp b/src/Parsers/Kusto/ParserKQLSummarize.cpp index 68aaa730896..11518fcc684 100644 --- a/src/Parsers/Kusto/ParserKQLSummarize.cpp +++ b/src/Parsers/Kusto/ParserKQLSummarize.cpp @@ -174,10 +174,10 @@ bool ParserKQLSummarize::parseImpl(Pos & pos, ASTPtr & node, Expected & expected --pos; apply_aliais(column_begin_pos, pos, groupby); - for (auto expr : expr_aggregations) + for (auto const & expr : expr_aggregations) expr_aggregation = expr_aggregation.empty() ? expr : expr_aggregation + "," + expr; - for (auto expr : expr_groupbys) + for (auto const & expr : expr_groupbys) expr_groupby = expr_groupby.empty() ? expr : expr_groupby + "," + expr; if (!expr_groupby.empty()) diff --git a/tests/queries/0_stateless/02366_kql_summarize.sql b/tests/queries/0_stateless/02366_kql_summarize.sql index 6b5e1c3af11..e76aed9217e 100644 --- a/tests/queries/0_stateless/02366_kql_summarize.sql +++ b/tests/queries/0_stateless/02366_kql_summarize.sql @@ -57,17 +57,17 @@ Customers | summarize job_count = count() by Occupation | where job_count > 0; Customers | summarize 'Edu Count'=count() by Education | sort by 'Edu Count' desc; -- { clientError 62 } print '-- make_list() --'; -Customers | summarize f_list = make_list(Education) by Occupation; -Customers | summarize f_list = make_list(Education, 2) by Occupation; +Customers | summarize f_list = make_list(Education) by Occupation | sort by Occupation; +Customers | summarize f_list = make_list(Education, 2) by Occupation | sort by Occupation; print '-- make_list_if() --'; -Customers | summarize f_list = make_list_if(FirstName, Age>30) by Occupation; -Customers | summarize f_list = make_list_if(FirstName, Age>30, 1) by Occupation; +Customers | summarize f_list = make_list_if(FirstName, Age>30) by Occupation | sort by Occupation; +Customers | summarize f_list = make_list_if(FirstName, Age>30, 1) by Occupation | sort by Occupation; print '-- make_set() --'; -Customers | summarize f_list = make_set(Education) by Occupation; -Customers | summarize f_list = make_set(Education, 2) by Occupation; +Customers | summarize f_list = make_set(Education) by Occupation | sort by Occupation; +Customers | summarize f_list = make_set(Education, 2) by Occupation | sort by Occupation; print '-- make_set_if() --'; -Customers | summarize f_list = make_set_if(Education, Age>30) by Occupation; -Customers | summarize f_list = make_set_if(Education, Age>30, 1) by Occupation; +Customers | summarize f_list = make_set_if(Education, Age>30) by Occupation | sort by Occupation; +Customers | summarize f_list = make_set_if(Education, Age>30, 1) by Occupation | sort by Occupation; print '-- stdev() --'; Customers | project Age | summarize stdev(Age); print '-- stdevif() --';