Merge 52521ec9d2 into b94a7167a8

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

@@ -82,6 +82,7 @@ static std::initializer_list<std::pair<ClickHouseVersion, SettingsChangesHistory
             {"parallel_replicas_mark_segment_size", 128, 0, "Value for this setting now determined automatically"},
             {"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."},
             {"parallel_replicas_local_plan", false, false, "Use local plan for local replica in a query with parallel replicas"},
             {"join_to_sort_minimum_perkey_rows", 0, 40, "The lower limit of per-key average rows in the right table to determine whether to rerange the right table by key in left or inner join. This setting ensures that the optimization is not applied for sparse table keys"},
             {"join_to_sort_maximum_table_rows", 0, 10000, "The maximum number of rows in the right table to determine whether to rerange the right table by key in left or inner join"},

src/Interpreters/executeQuery.cpp

@@ -930,6 +930,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;