From df1a9eee7c6398f250f69f9400d541ccbba5c3e9 Mon Sep 17 00:00:00 2001 From: vdimir Date: Wed, 28 Aug 2024 17:12:58 +0000 Subject: [PATCH] fix --- src/Parsers/ExpressionElementParsers.cpp | 23 ++++---- src/Parsers/tests/gtest_Parser.cpp | 68 +++++++++++++++++------- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/Parsers/ExpressionElementParsers.cpp b/src/Parsers/ExpressionElementParsers.cpp index 9b9a24f387e..6208c3fcd74 100644 --- a/src/Parsers/ExpressionElementParsers.cpp +++ b/src/Parsers/ExpressionElementParsers.cpp @@ -1289,16 +1289,17 @@ bool ParserStringLiteral::parseImpl(Pos & pos, ASTPtr & node, Expected & expecte template struct CollectionOfLiteralsLayer { - explicit CollectionOfLiteralsLayer(IParser::Pos & pos, TokenType closing_bracket_) + explicit CollectionOfLiteralsLayer(IParser::Pos & pos, bool is_functional_style_ = false) : literal_begin(pos) - , closing_bracket(closing_bracket_) + , is_functional_style(is_functional_style_) { pos.increaseDepth(); ++pos; } IParser::Pos literal_begin; - TokenType closing_bracket; + /// Functional-style literal definition, e.g. `tuple(1, 2, 3)`, or `array(1, 2, 3)`. + bool is_functional_style = false; Collection arr; }; @@ -1309,7 +1310,7 @@ bool ParserCollectionOfLiterals::parseImpl(Pos & pos, ASTPtr & node, return false; std::vector> layers; - layers.emplace_back(pos, closing_bracket); + layers.emplace_back(pos); ParserLiteral literal_p; @@ -1317,17 +1318,21 @@ bool ParserCollectionOfLiterals::parseImpl(Pos & pos, ASTPtr & node, { if (!layers.back().arr.empty()) { - if (pos->type == layers.back().closing_bracket) + auto expected_closing = layers.back().is_functional_style ? TokenType::ClosingRoundBracket : closing_bracket; + if (pos->type == expected_closing) { std::shared_ptr literal; - // /// Parse one-element tuples (e.g. (1)) later as single values for backward compatibility. - if (std::is_same_v && layers.back().arr.size() == 1) + if (std::is_same_v && layers.back().arr.size() == 1 && !layers.back().is_functional_style) { + /// Single element inside round brackets is not a tuple, but a scalar. + /// We need to merge it with the previous layer. + /// Example: (1, (2)) is parsed as tuple(1, 2) instead of tuple(1, tuple(2)). if (layers.size() > 1) { layers[layers.size() - 2].arr.push_back(layers.back().arr[0]); layers.pop_back(); + ++pos; pos.decreaseDepth(); continue; } @@ -1372,14 +1377,14 @@ bool ParserCollectionOfLiterals::parseImpl(Pos & pos, ASTPtr & node, } else if (pos->type == opening_bracket) { - layers.emplace_back(pos, closing_bracket); + layers.emplace_back(pos); } else if (pos->type == TokenType::BareWord && std::string_view(pos->begin, pos->end) == FunctionName::value) { ++pos; if (pos.isValid() && pos->type == TokenType::OpeningRoundBracket) { - layers.emplace_back(pos, TokenType::ClosingRoundBracket); + layers.emplace_back(pos, true); } else return false; diff --git a/src/Parsers/tests/gtest_Parser.cpp b/src/Parsers/tests/gtest_Parser.cpp index c072e9288dc..d2890052a8b 100644 --- a/src/Parsers/tests/gtest_Parser.cpp +++ b/src/Parsers/tests/gtest_Parser.cpp @@ -665,20 +665,23 @@ INSTANTIATE_TEST_SUITE_P( namespace DB { std::ostream & operator<<(std::ostream & stream, const Field & field) { return stream << field.dump(); } } -TEST(ParserTest, parseTupleLiterals) -try -{ - ParserTupleOfLiterals parser; - auto parse_tuple = [&](std::string_view text) -> std::optional +template +void testCollectionOfLiteralsParser( + const std::vector & test_cases, + const std::vector & negative_test_cases, + size_t top_level_size) +{ + ParserType parser; + auto parse_literal = [&](std::string_view text) -> std::optional { try { auto ast = parseQuery(parser, text.begin(), text.end(), 0, 0, 0); const auto * literal = typeid_cast(ast.get()); - if (!literal || literal->value.getType() != Field::Types::Tuple) + if (!literal || literal->value.getType() != Field::TypeToEnum::value) return {}; - return literal->value.safeGet(); + return literal->value.template safeGet(); } catch (const DB::Exception & e) { @@ -687,21 +690,50 @@ try } }; - const auto & tuple = parse_tuple("((1, 2), (3, 4))"); - ASSERT_TRUE(tuple && tuple->size() == 2); + const auto & reference_literal = parse_literal(test_cases[0]); + ASSERT_TRUE(reference_literal && reference_literal->size() == top_level_size); - std::vector test_cases = { - // "((1, 2,), (3, 4,),)", - // " ( (\t\t 1 \r\n , 2 ,\n ) \t\t, ( 3 , 4 \t, ) , ) ", - "(((1, 2)), (3, 4))", - "(((1), 2), (3, 4))", - "(tuple(1, 2), (3, 4))", - }; - for (size_t i = 0; i < test_cases.size(); ++i) + for (size_t i = 1; i < test_cases.size(); ++i) { SCOPED_TRACE(fmt::format("Test case #{}: {}", i + 1, test_cases[i])); - EXPECT_EQ(tuple, parse_tuple(test_cases[i])); + EXPECT_EQ(reference_literal, parse_literal(test_cases[i])); } + + for (size_t i = 0; i < negative_test_cases.size(); ++i) + { + SCOPED_TRACE(fmt::format("Negative test case #{}: {}", i + 1, negative_test_cases[i])); + auto negative_example = parse_literal(negative_test_cases[i]); + ASSERT_TRUE(negative_example); + EXPECT_NE(reference_literal, negative_example); + } +} + +TEST(ParserTest, parseTupleLiterals) +try +{ + testCollectionOfLiteralsParser({ + "((1, 2), (3, 4))", + "(((1), 2), (3, 4))", + "(((1, 2)), (3, 4))", + "(((1), 2), (((((((3))))), 4)))", + "(tuple(1, 2), (3, 4))", + "(tuple((1), (2)), (tuple(3, 4)))", + }, { + "((tuple(1), 2), (3, 4))", + }, 2); + + // testCollectionOfLiteralsParser({ + // "[[1, 2, 3]]", + // "[[(1), (2), 3]]", + // "[[1, (((2))), (((((3)))))]]", + // "[(([1, (((2))), (((((3)))))]))]", + // "[([1, 2, 3])]", + // }, { + // "[[[1], 2, 3]]", + // "[1, 2, 3]", + // "[[1, (2, 3)]]", + // "[[[1, 2, 3]]]", + // }, 1); } catch (...) {