Add enable_secure_identifiers setting to disallow insecure identifiers

src/Core/Settings.h

@@ -901,6 +901,7 @@ class IColumn;
     M(Bool, restore_replace_external_engines_to_null, false, "Replace all the external table engines to Null on restore. Useful for testing purposes", 0) \
     M(Bool, restore_replace_external_table_functions_to_null, false, "Replace all table functions to Null on restore. Useful for testing purposes", 0) \
     M(Bool, create_if_not_exists, false, "Enable IF NOT EXISTS for CREATE statements by default", 0) \
+    M(Bool, enable_secure_identifiers, false, "If enabled, only secure identifiers are allowed", 0) \
     \
     \
     /* ###################################### */ \

src/Core/SettingsChangesHistory.cpp

@@ -79,6 +79,7 @@ static std::initializer_list<std::pair<ClickHouseVersion, SettingsChangesHistory
             {"output_format_identifier_quoting_style", "Backticks", "Backticks", "New setting."},
             {"database_replicated_allow_replicated_engine_arguments", 1, 0, "Don't allow explicit arguments by default"},
             {"database_replicated_allow_explicit_uuid", 0, 0, "Added a new setting to disallow explicitly specifying table UUID"},
+            {"enable_secure_identifiers", false, false, "New setting."},
         }
     },
     {"24.8",

src/Interpreters/executeQuery.cpp

@@ -929,6 +929,14 @@ static std::tuple<ASTPtr, BlockIO> executeQueryImpl(
         InterpreterSetQuery::applySettingsFromQuery(ast, context);
         validateAnalyzerSettings(ast, context->getSettingsRef().allow_experimental_analyzer);
 
+        if (settings.enable_secure_identifiers)
+        {
+            WriteBufferFromOwnString buf;
+            IAST::FormatSettings enable_secure_identifiers_settings(buf, true);
+            enable_secure_identifiers_settings.enable_secure_identifiers = true;
+            ast->format(enable_secure_identifiers_settings);
+        }
+
         if (auto * insert_query = ast->as<ASTInsertQuery>())
             insert_query->tail = istr;
 

src/Parsers/IAST.cpp

@@ -219,6 +219,7 @@ String IAST::getColumnNameWithoutAlias() const
 
 void IAST::FormatSettings::writeIdentifier(const String & name) const
 {
+    checkIdentifier(name);
     switch (identifier_quoting_style)
     {
         case IdentifierQuotingStyle::None:
@@ -260,6 +261,7 @@ void IAST::FormatSettings::writeIdentifier(const String & name) const
 
 void IAST::FormatSettings::quoteIdentifier(const String & name) const
 {
+    checkIdentifier(name);
     switch (identifier_quoting_style)
     {
         case IdentifierQuotingStyle::None:
@@ -285,6 +287,20 @@ void IAST::FormatSettings::quoteIdentifier(const String & name) const
     }
 }
 
+void IAST::FormatSettings::checkIdentifier(const String & name) const
+{
+    if (enable_secure_identifiers)
+    {
+        for (char c : name)
+        {
+            if (!std::isalnum(c) && c != '_')
+            {
+                throw Exception(ErrorCodes::BAD_ARGUMENTS, "Not a secure identifier: `{}`", name);
+            }
+        }
+    }
+}
+
 void IAST::dumpTree(WriteBuffer & ostr, size_t indent) const
 {
     String indent_str(indent, '-');

src/Parsers/IAST.h

@@ -202,6 +202,7 @@ public:
         char nl_or_ws; /// Newline or whitespace.
         LiteralEscapingStyle literal_escaping_style;
         bool print_pretty_type_names;
+        bool enable_secure_identifiers;
 
         explicit FormatSettings(
             WriteBuffer & ostr_,
@@ -211,7 +212,8 @@ public:
             IdentifierQuotingStyle identifier_quoting_style_ = IdentifierQuotingStyle::Backticks,
             bool show_secrets_ = true,
             LiteralEscapingStyle literal_escaping_style_ = LiteralEscapingStyle::Regular,
-            bool print_pretty_type_names_ = false)
+            bool print_pretty_type_names_ = false,
+            bool enable_secure_identifiers_ = false)
             : ostr(ostr_)
             , one_line(one_line_)
             , hilite(hilite_)
@@ -221,6 +223,7 @@ public:
             , nl_or_ws(one_line ? ' ' : '\n')
             , literal_escaping_style(literal_escaping_style_)
             , print_pretty_type_names(print_pretty_type_names_)
+            , enable_secure_identifiers(enable_secure_identifiers_)
         {
         }
 
@@ -234,6 +237,7 @@ public:
             , nl_or_ws(other.nl_or_ws)
             , literal_escaping_style(other.literal_escaping_style)
             , print_pretty_type_names(other.print_pretty_type_names)
+            , enable_secure_identifiers(other.enable_secure_identifiers)
         {
         }
 
@@ -241,6 +245,7 @@ public:
         // Quote identifier `name` even when `always_quote_identifiers` is false.
         // If `identifier_quoting_style` is `IdentifierQuotingStyle::None`, quote it with `IdentifierQuotingStyle::Backticks`
         void quoteIdentifier(const String & name) const;
+        void checkIdentifier(const String & name) const;
     };
 
     /// State. For example, a set of nodes can be remembered, which we already walk through.

tests/queries/0_stateless/03234_enable_secure_identifiers.reference

@@ -0,0 +1,3 @@
+CREATE TABLE default.test_foo\n(\n    `secure_123` Int8,\n    `date` Date,\n    `town` LowCardinality(String)\n)\nENGINE = MergeTree\nPARTITION BY toYear(date)\nPRIMARY KEY (town, date)\nORDER BY (town, date)\nSETTINGS index_granularity = 8192\nCOMMENT \'test\'
+CREATE TABLE default.test_foo\n(\n    `123_secure` Int8,\n    `date` Date,\n    `town` LowCardinality(String)\n)\nENGINE = MergeTree\nPARTITION BY toYear(date)\nPRIMARY KEY (town, date)\nORDER BY (town, date)\nSETTINGS index_granularity = 8192\nCOMMENT \'test\'
+CREATE TABLE default.test_foo\n(\n    `insecure_$` Int8,\n    `date` Date,\n    `town` LowCardinality(String)\n)\nENGINE = MergeTree\nPARTITION BY toYear(date)\nPRIMARY KEY (town, date)\nORDER BY (town, date)\nSETTINGS index_granularity = 8192

tests/queries/0_stateless/03234_enable_secure_identifiers.sql

@@ -0,0 +1,111 @@
+DROP TABLE IF EXISTS `test_foo_#`;
+CREATE TABLE `test_foo_#` (
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date)
+COMMENT 'test' -- to end ENGINE definition, so SETTINGS will be in the query level
+SETTINGS
+    enable_secure_identifiers=true; -- { serverError BAD_ARGUMENTS }
+DROP TABLE IF EXISTS `test_foo_#`;
+
+
+DROP TABLE IF EXISTS test_foo;
+CREATE TABLE test_foo (
+    `insecure_#` Int8,
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date)
+COMMENT 'test' -- to end ENGINE definition, so SETTINGS will be in the query level
+SETTINGS
+    enable_secure_identifiers=true; -- { serverError BAD_ARGUMENTS }
+
+DROP TABLE IF EXISTS test_foo;
+CREATE TABLE test_foo (
+    `insecure_'` Int8,
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date)
+COMMENT 'test' -- to end ENGINE definition, so SETTINGS will be in the query level
+SETTINGS
+    enable_secure_identifiers=true; -- { serverError BAD_ARGUMENTS }
+
+DROP TABLE IF EXISTS test_foo;
+CREATE TABLE test_foo (
+    `insecure_"` Int8,
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date)
+COMMENT 'test' -- to end ENGINE definition, so SETTINGS will be in the query level
+SETTINGS
+    enable_secure_identifiers=true; -- { serverError BAD_ARGUMENTS }
+
+DROP TABLE IF EXISTS test_foo;
+CREATE TABLE test_foo (
+    `secure_123` Int8,
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date)
+COMMENT 'test' -- to end ENGINE definition, so SETTINGS will be in the query level
+SETTINGS
+    enable_secure_identifiers=true;
+
+SHOW CREATE TABLE test_foo 
+SETTINGS
+    enable_secure_identifiers=true;
+
+DROP TABLE IF EXISTS test_foo;
+CREATE TABLE test_foo (
+    `123_secure` Int8,
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date)
+COMMENT 'test' -- to end ENGINE definition, so SETTINGS will be in the query level
+SETTINGS
+    enable_secure_identifiers=true;
+
+SHOW CREATE TABLE test_foo 
+SETTINGS
+    enable_secure_identifiers=true;
+
+-- CREATE TABLE without `enable_secure_identifiers`
+DROP TABLE IF EXISTS test_foo;
+CREATE TABLE `test_foo` (
+    `insecure_$` Int8,
+    `date` Date,
+    `town` LowCardinality(String),
+)
+ENGINE = MergeTree
+PRIMARY KEY (town, date)
+PARTITION BY toYear(date);
+-- Then SHOW CREATE .. with `enable_secure_identifiers`
+-- While the result contains insecure identifiers (`insecure_$`), the `SHOW CREATE TABLE ...` query does not have any. So the query is expected to succeed.
+SHOW CREATE TABLE test_foo 
+SETTINGS
+    enable_secure_identifiers=true;
+
+DROP TABLE IF EXISTS test_foo;
+
+-- SHOW CREATE .. query contains an insecure identifier (`test_foo$`) with `enable_secure_identifiers`
+SHOW CREATE TABLE `test_foo$`
+SETTINGS
+    enable_secure_identifiers=true; -- { serverError BAD_ARGUMENTS }
+
+DROP TABLE IF EXISTS test_foo;