From e4c84cf4672bebf9809e06d011889e84be429863 Mon Sep 17 00:00:00 2001
From: Michael Kolupaev <michael.kolupaev@clickhouse.com>
Date: Tue, 25 Jun 2024 08:31:24 +0000
Subject: [PATCH] Fix inconsistent AST formatting when a keyword is used as
 type name

---
 src/Parsers/ParserCreateQuery.h               |  1 +
 src/Parsers/ParserDataType.cpp                | 19 ++++++++++++++++++-
 ...3168_inconsistent_ast_formatting.reference |  0
 .../03168_inconsistent_ast_formatting.sql     |  4 ++++
 4 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 tests/queries/0_stateless/03168_inconsistent_ast_formatting.reference
 create mode 100644 tests/queries/0_stateless/03168_inconsistent_ast_formatting.sql

diff --git a/src/Parsers/ParserCreateQuery.h b/src/Parsers/ParserCreateQuery.h
index 5f6df33176f..bb37491a366 100644
--- a/src/Parsers/ParserCreateQuery.h
+++ b/src/Parsers/ParserCreateQuery.h
@@ -213,6 +213,7 @@ bool IParserColumnDeclaration<NameParser>::parseImpl(Pos & pos, ASTPtr & node, E
         return res;
     };
 
+    /// Keep this list of keywords in sync with ParserDataType::parseImpl().
     if (!null_check_without_moving()
         && !s_default.checkWithoutMoving(pos, expected)
         && !s_materialized.checkWithoutMoving(pos, expected)
diff --git a/src/Parsers/ParserDataType.cpp b/src/Parsers/ParserDataType.cpp
index b5bc9f89990..ad33c7e4558 100644
--- a/src/Parsers/ParserDataType.cpp
+++ b/src/Parsers/ParserDataType.cpp
@@ -1,5 +1,6 @@
 #include <Parsers/ParserDataType.h>
 
+#include <boost/algorithm/string/case_conv.hpp>
 #include <Parsers/ASTFunction.h>
 #include <Parsers/ASTIdentifier.h>
 #include <Parsers/ASTIdentifier_fwd.h>
@@ -103,12 +104,28 @@ bool ParserDataType::parseImpl(Pos & pos, ASTPtr & node, Expected & expected)
         return false;
     tryGetIdentifierNameInto(identifier, type_name);
 
-    /// Don't accept things like Array(`x.y`).
+    /// When parsing we accept quoted type names (e.g. `UInt64`), but when formatting we print them
+    /// unquoted (e.g. UInt64). This introduces problems when the string in the quotes is garbage:
+    ///  * Array(`x.y`) -> Array(x.y) -> fails to parse
+    ///  * `Null` -> Null -> parses as keyword instead of type name
+    /// Here we check for these cases and reject.
     if (!std::all_of(type_name.begin(), type_name.end(), [](char c) { return isWordCharASCII(c) || c == '$'; }))
     {
         expected.add(pos, "type name");
         return false;
     }
+    /// Keywords that IParserColumnDeclaration recognizes before the type name.
+    /// E.g. reject CREATE TABLE a (x `Null`) because in "x Null" the Null would be parsed as
+    /// column attribute rather than type name.
+    {
+        String n = type_name;
+        boost::to_upper(n);
+        if (n == "NOT" || n == "NULL" || n == "DEFAULT" || n == "MATERIALIZED" || n == "EPHEMERAL" || n == "ALIAS" || n == "AUTO" || n == "PRIMARY" || n == "COMMENT" || n == "CODEC")
+        {
+            expected.add(pos, "type name");
+            return false;
+        }
+    }
 
     String type_name_upper = Poco::toUpper(type_name);
     String type_name_suffix;
diff --git a/tests/queries/0_stateless/03168_inconsistent_ast_formatting.reference b/tests/queries/0_stateless/03168_inconsistent_ast_formatting.reference
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tests/queries/0_stateless/03168_inconsistent_ast_formatting.sql b/tests/queries/0_stateless/03168_inconsistent_ast_formatting.sql
new file mode 100644
index 00000000000..d43d46d5b14
--- /dev/null
+++ b/tests/queries/0_stateless/03168_inconsistent_ast_formatting.sql
@@ -0,0 +1,4 @@
+create table a (x `Null`); -- { clientError SYNTAX_ERROR }
+create table a (x f(`Null`)); -- { clientError SYNTAX_ERROR }
+create table a (x Enum8(f(`Null`, 'World', 2))); -- { clientError SYNTAX_ERROR }
+create table a (`value2` Enum8('Hello' = 1, equals(`Null`, 'World', 2), '!' = 3)); -- { clientError SYNTAX_ERROR }
\ No newline at end of file