From 7dc0811ffd2b1f0f9f2128a186959e792d08071a Mon Sep 17 00:00:00 2001 From: avogar Date: Tue, 10 Dec 2024 19:15:57 +0000 Subject: [PATCH 1/3] Improve formatting of identifiers with JSON subcolumns --- src/Parsers/ASTIdentifier.cpp | 11 ++++++++++- src/Parsers/ExpressionElementParsers.cpp | 13 +++++++++++++ src/Parsers/ExpressionElementParsers.h | 14 ++++++++------ .../03285_json_subcolumns_formatting.reference | 12 ++++++++++++ .../03285_json_subcolumns_formatting.sh | 14 ++++++++++++++ 5 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 tests/queries/0_stateless/03285_json_subcolumns_formatting.reference create mode 100755 tests/queries/0_stateless/03285_json_subcolumns_formatting.sh diff --git a/src/Parsers/ASTIdentifier.cpp b/src/Parsers/ASTIdentifier.cpp index 54b5f45f20b..159ea5e8b5a 100644 --- a/src/Parsers/ASTIdentifier.cpp +++ b/src/Parsers/ASTIdentifier.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -109,7 +110,15 @@ void ASTIdentifier::formatImplWithoutAlias(WriteBuffer & ostr, const FormatSetti auto format_element = [&](const String & elem_name) { ostr << (settings.hilite ? hilite_identifier : ""); - settings.writeIdentifier(ostr, elem_name, /*ambiguous=*/false); + if (auto special_delimiter_and_identifier = ParserCompoundIdentifier::splitSpecialDelimiterIfAny(elem_name)) + { + ostr << special_delimiter_and_identifier->first; + settings.writeIdentifier(ostr, special_delimiter_and_identifier->second, /*ambiguous=*/false); + } + else + { + settings.writeIdentifier(ostr, elem_name, /*ambiguous=*/false); + } ostr << (settings.hilite ? hilite_none : ""); }; diff --git a/src/Parsers/ExpressionElementParsers.cpp b/src/Parsers/ExpressionElementParsers.cpp index 28760cdaef9..75177f80ca1 100644 --- a/src/Parsers/ExpressionElementParsers.cpp +++ b/src/Parsers/ExpressionElementParsers.cpp @@ -415,6 +415,19 @@ bool ParserCompoundIdentifier::parseImpl(Pos & pos, ASTPtr & node, Expected & ex return true; } +std::optional> ParserCompoundIdentifier::splitSpecialDelimiterIfAny(const String & name) +{ + if (name.size() < 3 + || (name[0] != char(SpecialDelimiter::JSON_PATH_DYNAMIC_TYPE) && name[0] != char(SpecialDelimiter::JSON_PATH_PREFIX)) + || name[1] != '`' || name.back() != '`') + return std::nullopt; + + String identifier; + ReadBufferFromMemory buf(name.data() + 1, name.size() - 1); + readBackQuotedString(identifier, buf); + return std::make_pair(name[0], identifier); +} + ASTPtr createFunctionCast(const ASTPtr & expr_ast, const ASTPtr & type_ast) { diff --git a/src/Parsers/ExpressionElementParsers.h b/src/Parsers/ExpressionElementParsers.h index 903111f32db..a0cee442a17 100644 --- a/src/Parsers/ExpressionElementParsers.h +++ b/src/Parsers/ExpressionElementParsers.h @@ -61,6 +61,14 @@ protected: class ParserCompoundIdentifier : public IParserBase { public: + explicit ParserCompoundIdentifier(bool table_name_with_optional_uuid_ = false, bool allow_query_parameter_ = false, Highlight highlight_type_ = Highlight::identifier) + : table_name_with_optional_uuid(table_name_with_optional_uuid_), allow_query_parameter(allow_query_parameter_), highlight_type(highlight_type_) + { + } + + static std::optional> splitSpecialDelimiterIfAny(const String & name); + +protected: enum class SpecialDelimiter : char { NONE = '\0', @@ -68,12 +76,6 @@ public: JSON_PATH_PREFIX = '^', }; - explicit ParserCompoundIdentifier(bool table_name_with_optional_uuid_ = false, bool allow_query_parameter_ = false, Highlight highlight_type_ = Highlight::identifier) - : table_name_with_optional_uuid(table_name_with_optional_uuid_), allow_query_parameter(allow_query_parameter_), highlight_type(highlight_type_) - { - } - -protected: const char * getName() const override { return "compound identifier"; } bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; bool table_name_with_optional_uuid; diff --git a/tests/queries/0_stateless/03285_json_subcolumns_formatting.reference b/tests/queries/0_stateless/03285_json_subcolumns_formatting.reference new file mode 100644 index 00000000000..1ef2e6b6afb --- /dev/null +++ b/tests/queries/0_stateless/03285_json_subcolumns_formatting.reference @@ -0,0 +1,12 @@ +SELECT json.^sub.object.path +FROM test +SELECT json.^`s^b`.object.path +FROM test +SELECT json.`^sub`.object.path +FROM test +SELECT json.path.:UInt64 +FROM test +SELECT json.path.:`Array(JSON)` +FROM test +SELECT json.path.`:UInt64` +FROM test diff --git a/tests/queries/0_stateless/03285_json_subcolumns_formatting.sh b/tests/queries/0_stateless/03285_json_subcolumns_formatting.sh new file mode 100755 index 00000000000..c3b1d6ae31f --- /dev/null +++ b/tests/queries/0_stateless/03285_json_subcolumns_formatting.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +$CLICKHOUSE_FORMAT --query "select json.^sub.object.path from test"; +$CLICKHOUSE_FORMAT --query "select json.^\`s^b\`.object.path from test"; +$CLICKHOUSE_FORMAT --query "select json.\`^sub\`.object.path from test"; + +$CLICKHOUSE_FORMAT --query "select json.path.:UInt64 from test"; +$CLICKHOUSE_FORMAT --query "select json.path.:\`Array(JSON)\` from test"; +$CLICKHOUSE_FORMAT --query "select json.path.\`:UInt64\` from test"; + From 227346e7aeeb65cd72d7e2073432d6911691aaf0 Mon Sep 17 00:00:00 2001 From: avogar Date: Tue, 10 Dec 2024 19:16:30 +0000 Subject: [PATCH 2/3] Rename method --- src/Parsers/ASTIdentifier.cpp | 2 +- src/Parsers/ExpressionElementParsers.cpp | 2 +- src/Parsers/ExpressionElementParsers.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Parsers/ASTIdentifier.cpp b/src/Parsers/ASTIdentifier.cpp index 159ea5e8b5a..9d6ad1df08d 100644 --- a/src/Parsers/ASTIdentifier.cpp +++ b/src/Parsers/ASTIdentifier.cpp @@ -110,7 +110,7 @@ void ASTIdentifier::formatImplWithoutAlias(WriteBuffer & ostr, const FormatSetti auto format_element = [&](const String & elem_name) { ostr << (settings.hilite ? hilite_identifier : ""); - if (auto special_delimiter_and_identifier = ParserCompoundIdentifier::splitSpecialDelimiterIfAny(elem_name)) + if (auto special_delimiter_and_identifier = ParserCompoundIdentifier::splitSpecialDelimiterAndIdentifierIfAny(elem_name)) { ostr << special_delimiter_and_identifier->first; settings.writeIdentifier(ostr, special_delimiter_and_identifier->second, /*ambiguous=*/false); diff --git a/src/Parsers/ExpressionElementParsers.cpp b/src/Parsers/ExpressionElementParsers.cpp index 75177f80ca1..d200ec5706f 100644 --- a/src/Parsers/ExpressionElementParsers.cpp +++ b/src/Parsers/ExpressionElementParsers.cpp @@ -415,7 +415,7 @@ bool ParserCompoundIdentifier::parseImpl(Pos & pos, ASTPtr & node, Expected & ex return true; } -std::optional> ParserCompoundIdentifier::splitSpecialDelimiterIfAny(const String & name) +std::optional> ParserCompoundIdentifier::splitSpecialDelimiterAndIdentifierIfAny(const String & name) { if (name.size() < 3 || (name[0] != char(SpecialDelimiter::JSON_PATH_DYNAMIC_TYPE) && name[0] != char(SpecialDelimiter::JSON_PATH_PREFIX)) diff --git a/src/Parsers/ExpressionElementParsers.h b/src/Parsers/ExpressionElementParsers.h index a0cee442a17..1b77ff0c85d 100644 --- a/src/Parsers/ExpressionElementParsers.h +++ b/src/Parsers/ExpressionElementParsers.h @@ -66,7 +66,7 @@ public: { } - static std::optional> splitSpecialDelimiterIfAny(const String & name); + static std::optional> splitSpecialDelimiterAndIdentifierIfAny(const String & name); protected: enum class SpecialDelimiter : char From 1024bb9df5638a4bda6ba7fdfced1ab8b27e2413 Mon Sep 17 00:00:00 2001 From: avogar Date: Tue, 10 Dec 2024 19:19:09 +0000 Subject: [PATCH 3/3] Add comments --- src/Parsers/ExpressionElementParsers.cpp | 1 + src/Parsers/ExpressionElementParsers.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/Parsers/ExpressionElementParsers.cpp b/src/Parsers/ExpressionElementParsers.cpp index d200ec5706f..e64a931900b 100644 --- a/src/Parsers/ExpressionElementParsers.cpp +++ b/src/Parsers/ExpressionElementParsers.cpp @@ -417,6 +417,7 @@ bool ParserCompoundIdentifier::parseImpl(Pos & pos, ASTPtr & node, Expected & ex std::optional> ParserCompoundIdentifier::splitSpecialDelimiterAndIdentifierIfAny(const String & name) { + /// Identifier with special delimiter looks like this: ``. if (name.size() < 3 || (name[0] != char(SpecialDelimiter::JSON_PATH_DYNAMIC_TYPE) && name[0] != char(SpecialDelimiter::JSON_PATH_PREFIX)) || name[1] != '`' || name.back() != '`') diff --git a/src/Parsers/ExpressionElementParsers.h b/src/Parsers/ExpressionElementParsers.h index 1b77ff0c85d..2151ec0216e 100644 --- a/src/Parsers/ExpressionElementParsers.h +++ b/src/Parsers/ExpressionElementParsers.h @@ -66,6 +66,8 @@ public: { } + /// Checks if the identirier is actually a pair of a special delimiter and the identifier in back quotes. + /// For example: :`UInt64` or ^`path` from special JSON subcolumns. static std::optional> splitSpecialDelimiterAndIdentifierIfAny(const String & name); protected: