From a249840421de5fd432e648e96ebe55c94123583d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Thu, 15 Feb 2024 21:42:47 +0100 Subject: [PATCH 1/2] Fix formatting of NOT with single literals --- src/Parsers/ASTFunction.cpp | 21 +++++++++++-------- .../02990_format_not_precedence.reference | 13 ++++++++++++ .../02990_format_not_precedence.sql | 7 +++++++ 3 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 tests/queries/0_stateless/02990_format_not_precedence.reference create mode 100644 tests/queries/0_stateless/02990_format_not_precedence.sql diff --git a/src/Parsers/ASTFunction.cpp b/src/Parsers/ASTFunction.cpp index e7f7b48091ac..dc3b011b0966 100644 --- a/src/Parsers/ASTFunction.cpp +++ b/src/Parsers/ASTFunction.cpp @@ -835,34 +835,37 @@ void ASTFunction::formatImplWithoutAlias(const FormatSettings & settings, Format const auto * literal = arguments->children[0]->as(); const auto * function = arguments->children[0]->as(); - bool negate = name == "negate"; bool is_tuple = literal && literal->value.getType() == Field::Types::Tuple; // do not add parentheses for tuple literal, otherwise extra parens will be added `-((3, 7, 3), 1)` -> `-(((3, 7, 3), 1))` bool literal_need_parens = literal && !is_tuple; + // negate always requires parentheses, otherwise -(-1) will be printed as --1 - bool negate_need_parens = negate && (literal_need_parens || (function && function->name == "negate")); - // We don't need parentheses around a single literal. - bool need_parens = !literal && frame.need_parens && !negate_need_parens; + bool inside_parens = name == "negate" && (literal_need_parens || (function && function->name == "negate")); + + /// We DO need parentheses around a single literal + /// For example, SELECT (NOT 0) + (NOT 0) cannot be transformed into SELECT NOT 0 + NOT 0, since + /// this is equal to SELECT NOT (0 + NOT 0) + bool outside_parens = frame.need_parens && !inside_parens; // do not add extra parentheses for functions inside negate, i.e. -(-toUInt64(-(1))) - if (negate_need_parens) + if (inside_parens) nested_need_parens.need_parens = false; - if (need_parens) + if (outside_parens) settings.ostr << '('; settings.ostr << (settings.hilite ? hilite_operator : "") << func[1] << (settings.hilite ? hilite_none : ""); - if (negate_need_parens) + if (inside_parens) settings.ostr << '('; arguments->formatImpl(settings, state, nested_need_parens); written = true; - if (negate_need_parens) + if (inside_parens) settings.ostr << ')'; - if (need_parens) + if (outside_parens) settings.ostr << ')'; break; diff --git a/tests/queries/0_stateless/02990_format_not_precedence.reference b/tests/queries/0_stateless/02990_format_not_precedence.reference new file mode 100644 index 000000000000..f44cf2fdb52a --- /dev/null +++ b/tests/queries/0_stateless/02990_format_not_precedence.reference @@ -0,0 +1,13 @@ +-- { echoOn } +SELECT NOT 0 + NOT 0; +0 +SELECT NOT (0 + (NOT 0)); +0 +SELECT (NOT 0) + (NOT 0); +2 +SELECT formatQuery('SELECT NOT 0 + NOT 0'); +SELECT NOT (0 + (NOT 0)) +SELECT formatQuery('SELECT NOT (0 + (NOT 0))'); +SELECT NOT (0 + (NOT 0)) +SELECT formatQuery('SELECT (NOT 0) + (NOT 0)'); +SELECT (NOT 0) + (NOT 0) diff --git a/tests/queries/0_stateless/02990_format_not_precedence.sql b/tests/queries/0_stateless/02990_format_not_precedence.sql new file mode 100644 index 000000000000..98ef2c9e7812 --- /dev/null +++ b/tests/queries/0_stateless/02990_format_not_precedence.sql @@ -0,0 +1,7 @@ +-- { echoOn } +SELECT NOT 0 + NOT 0; +SELECT NOT (0 + (NOT 0)); +SELECT (NOT 0) + (NOT 0); +SELECT formatQuery('SELECT NOT 0 + NOT 0'); +SELECT formatQuery('SELECT NOT (0 + (NOT 0))'); +SELECT formatQuery('SELECT (NOT 0) + (NOT 0)'); From 867eb8b9b934502df56f8412014809d9d850be0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Thu, 15 Feb 2024 22:19:09 +0100 Subject: [PATCH 2/2] Adapt tests --- tests/queries/0_stateless/01920_not_chain_format.reference | 4 ++-- tests/queries/0_stateless/01921_not_chain.reference | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/queries/0_stateless/01920_not_chain_format.reference b/tests/queries/0_stateless/01920_not_chain_format.reference index 22abfd17dc7e..bb58a0ff146b 100644 --- a/tests/queries/0_stateless/01920_not_chain_format.reference +++ b/tests/queries/0_stateless/01920_not_chain_format.reference @@ -1,5 +1,5 @@ -- { echo } EXPLAIN SYNTAX SELECT NOT NOT (NOT (NOT (NULL))); -SELECT NOT (NOT (NOT NOT NULL)) +SELECT NOT (NOT (NOT (NOT NULL))) EXPLAIN SYNTAX SELECT NOT (NOT (NOT NOT NULL)); -SELECT NOT (NOT (NOT NOT NULL)) +SELECT NOT (NOT (NOT (NOT NULL))) diff --git a/tests/queries/0_stateless/01921_not_chain.reference b/tests/queries/0_stateless/01921_not_chain.reference index c29c66f1274d..ebd18f4b342f 100644 --- a/tests/queries/0_stateless/01921_not_chain.reference +++ b/tests/queries/0_stateless/01921_not_chain.reference @@ -4,6 +4,6 @@ SELECT 1 != (NOT 1); SELECT 1 != NOT 1; 1 EXPLAIN SYNTAX SELECT 1 != (NOT 1); -SELECT 1 != NOT 1 +SELECT 1 != (NOT 1) EXPLAIN SYNTAX SELECT 1 != NOT 1; -SELECT 1 != NOT 1 +SELECT 1 != (NOT 1)