Merge pull request #24427 from azat/formatting-fixes

Fix formatting of negative values

src/Parsers/ASTFunction.cpp

@@ -231,68 +231,32 @@ void ASTFunction::formatImplWithoutAlias(const FormatSettings & settings, Format
                 }
 
                 const auto * literal = arguments->children[0]->as<ASTLiteral>();
-                /* A particularly stupid case. If we have a unary minus before
+                const auto * function = arguments->children[0]->as<ASTFunction>();
-                 * a literal that is a negative number "-(-1)" or "- -1", this
+                bool negate = name == "negate";
-                 * can not be formatted as `--1`, since this will be
+                // negate always requires parentheses, otherwise -(-1) will be printed as --1
-                 * interpreted as a comment. Instead, negate the literal
+                bool negate_need_parens = negate && (literal || (function && function->name == "negate"));
-                 * in place. Another possible solution is to use parentheses,
-                 * but the old comment said it is impossible, without mentioning
-                 * the reason. We should also negate the nonnegative literals,
-                 * for symmetry. We print the negated value without parentheses,
-                 * because they are not needed around a single literal. Also we
-                 * use formatting from FieldVisitorToString, so that the type is
-                 * preserved (e.g. -0. is printed with trailing period).
-                 */
-                if (literal && name == "negate")
-                {
-                    written = applyVisitor(
-                        [&settings](const auto & value)
-                        // -INT_MAX is negated to -INT_MAX by the negate()
-                        // function, so we can implement this behavior here as
-                        // well. Technically it is an UB to perform such negation
-                        // w/o a cast to unsigned type.
-                        NO_SANITIZE_UNDEFINED
-                        {
-                            using ValueType = std::decay_t<decltype(value)>;
-                            if constexpr (is_decimal_field<ValueType>)
-                            {
-                                // The parser doesn't create decimal literals, but
-                                // they can be produced by constant folding or the
-                                // fuzzer. Decimals are always signed, so no need
-                                // to deduce the result type like we do for ints.
-                                const auto int_value = value.getValue().value;
-                                settings.ostr << FieldVisitorToString{}(ValueType{
-                                    -int_value,
-                                    value.getScale()});
-                            }
-                            else if constexpr (std::is_arithmetic_v<ValueType>)
-                            {
-                                using ResultType = typename NumberTraits::ResultOfNegate<ValueType>::Type;
-                                settings.ostr << FieldVisitorToString{}(
-                                    -static_cast<ResultType>(value));
-                                return true;
-                            }
-
-                            return false;
-                        },
-                        literal->value);
-
-                    if (written)
-                    {
-                        break;
-                    }
-                }
-
                 // We don't need parentheses around a single literal.
-                if (!literal && frame.need_parens)
+                bool need_parens = !literal && frame.need_parens && !negate_need_parens;
+
+                // do not add extra parentheses for functions inside negate, i.e. -(-toUInt64(-(1)))
+                if (negate_need_parens)
+                    nested_need_parens.need_parens = false;
+
+                if (need_parens)
                     settings.ostr << '(';
 
                 settings.ostr << (settings.hilite ? hilite_operator : "") << func[1] << (settings.hilite ? hilite_none : "");
 
+                if (negate_need_parens)
+                    settings.ostr << '(';
+
                 arguments->formatImpl(settings, state, nested_need_parens);
                 written = true;
 
-                if (!literal && frame.need_parens)
+                if (negate_need_parens)
+                    settings.ostr << ')';
+
+                if (need_parens)
                     settings.ostr << ')';
 
                 break;

tests/queries/0_stateless/01566_negate_formatting.reference

@@ -1,34 +1,34 @@
 -- { echo }
 explain syntax select negate(1), negate(-1), - -1, -(-1), (-1) in (-1);
 SELECT
-    -1,
+    -(1),
-    1,
+    -(-1),
-    1,
+    -(-1),
-    1,
+    -(-1),
     -1 IN (-1)
 explain syntax select negate(1.), negate(-1.), - -1., -(-1.), (-1.) in (-1.);
 SELECT
-    -1.,
+    -(1.),
-    1.,
+    -(-1.),
-    1.,
+    -(-1.),
-    1.,
+    -(-1.),
     -1. IN (-1.)
 explain syntax select negate(-9223372036854775808), -(-9223372036854775808), - -9223372036854775808;
 SELECT
-    -9223372036854775808,
+    -(-9223372036854775808),
-    -9223372036854775808,
+    -(-9223372036854775808),
-    -9223372036854775808
+    -(-9223372036854775808)
 explain syntax select negate(0), negate(-0), - -0, -(-0), (-0) in (-0);
 SELECT
-    0,
+    -(0),
-    0,
+    -(0),
-    0,
+    -(0),
-    0,
+    -(0),
     0 IN (0)
 explain syntax select negate(0.), negate(-0.), - -0., -(-0.), (-0.) in (-0.);
 SELECT
-    -0.,
+    -(0.),
-    0.,
+    -(-0.),
-    0.,
+    -(-0.),
-    0.,
+    -(-0.),
     -0. IN (-0.)

tests/queries/0_stateless/01881_negate_formatting.reference

@@ -0,0 +1,13 @@
+-- { echo }
+EXPLAIN SYNTAX SELECT -1;
+SELECT -1
+EXPLAIN SYNTAX SELECT -(1);
+SELECT -(1)
+EXPLAIN SYNTAX SELECT -(-(1));
+SELECT -(-(1))
+EXPLAIN SYNTAX SELECT -(-(-(1)));
+SELECT -(-(-(1)))
+EXPLAIN SYNTAX SELECT -(-(-1));
+SELECT -(-(-1))
+EXPLAIN SYNTAX SELECT -(-toUInt64(-(1)));
+SELECT -(-toUInt64(-(1)))

tests/queries/0_stateless/01881_negate_formatting.sql

@@ -0,0 +1,7 @@
+-- { echo }
+EXPLAIN SYNTAX SELECT -1;
+EXPLAIN SYNTAX SELECT -(1);
+EXPLAIN SYNTAX SELECT -(-(1));
+EXPLAIN SYNTAX SELECT -(-(-(1)));
+EXPLAIN SYNTAX SELECT -(-(-1));
+EXPLAIN SYNTAX SELECT -(-toUInt64(-(1)));