From 61d6fae14bb73fc3530fc391fc9f6ac7b21618ae Mon Sep 17 00:00:00 2001 From: Duc Canh Le Date: Mon, 19 Sep 2022 15:00:04 +0800 Subject: [PATCH 1/2] add settings allow_suspicious_fixed_string_types --- src/Core/Settings.h | 1 + src/Interpreters/InterpreterCreateQuery.cpp | 19 +++++++++++++++++++ ...6_create_suspicious_fixed_string.reference | 0 .../02426_create_suspicious_fixed_string.sql | 4 ++++ 4 files changed, 24 insertions(+) create mode 100644 tests/queries/0_stateless/02426_create_suspicious_fixed_string.reference create mode 100644 tests/queries/0_stateless/02426_create_suspicious_fixed_string.sql diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 870647b3254..2a27d2fd257 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -125,6 +125,7 @@ static constexpr UInt64 operator""_GiB(unsigned long long value) M(Float, totals_auto_threshold, 0.5, "The threshold for totals_mode = 'auto'.", 0) \ \ M(Bool, allow_suspicious_low_cardinality_types, false, "In CREATE TABLE statement allows specifying LowCardinality modifier for types of small fixed size (8 or less). Enabling this may increase merge times and memory consumption.", 0) \ + M(Bool, allow_suspicious_fixed_string_types, false, "In CREATE TABLE statement allows creating columns of type FixedString(n) with n > 256. FixedString with length >= 256 is suspicious and most likely indicates misusage", 0) \ M(Bool, compile_expressions, true, "Compile some scalar functions and operators to native code.", 0) \ M(UInt64, min_count_to_compile_expression, 3, "The number of identical expressions before they are JIT-compiled", 0) \ M(Bool, compile_aggregate_expressions, true, "Compile aggregate functions to native code.", 0) \ diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index e66fe543ab0..b5c222ac503 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -74,8 +74,11 @@ #include #include +#include +#define MAX_FIXEDSTRING_SIZE_WITHOUT_SUSPICIOUS 256 + namespace DB { @@ -806,6 +809,22 @@ void InterpreterCreateQuery::validateTableStructure(const ASTCreateQuery & creat } } } + if (!create.attach && !settings.allow_suspicious_fixed_string_types) + { + for (const auto & [name, type] : properties.columns.getAllPhysical()) + { + auto basic_type = removeLowCardinality(removeNullable(type)); + if (const auto * fixed_string = typeid_cast(basic_type.get())) + { + if (fixed_string->getN() > MAX_FIXEDSTRING_SIZE_WITHOUT_SUSPICIOUS) + throw Exception(ErrorCodes::ILLEGAL_COLUMN, + "Cannot create table with column '{}' which type is '{}' " + "because fixed string with size > {} is suspicious. " + "Set setting allow_suspicious_fixed_string_types = 1 in order to allow it", + name, type->getName(), MAX_FIXEDSTRING_SIZE_WITHOUT_SUSPICIOUS); + } + } + } } String InterpreterCreateQuery::getTableEngineName(DefaultTableEngine default_table_engine) diff --git a/tests/queries/0_stateless/02426_create_suspicious_fixed_string.reference b/tests/queries/0_stateless/02426_create_suspicious_fixed_string.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02426_create_suspicious_fixed_string.sql b/tests/queries/0_stateless/02426_create_suspicious_fixed_string.sql new file mode 100644 index 00000000000..c681c3c54d6 --- /dev/null +++ b/tests/queries/0_stateless/02426_create_suspicious_fixed_string.sql @@ -0,0 +1,4 @@ +CREATE TABLE fixed_string (id UInt64, s FixedString(256)) ENGINE = MergeTree() ORDER BY id; +CREATE TABLE suspicious_fixed_string (id UInt64, s FixedString(257)) ENGINE = MergeTree() ORDER BY id; -- { serverError 44 } +SET allow_suspicious_fixed_string_types = 1; +CREATE TABLE suspicious_fixed_string (id UInt64, s FixedString(257)) ENGINE = MergeTree() ORDER BY id; From bb9739308c14149397d6c8551117f553d3477a67 Mon Sep 17 00:00:00 2001 From: Duc Canh Le Date: Tue, 20 Sep 2022 20:48:50 +0800 Subject: [PATCH 2/2] fix a test --- .../02426_low_cardinality_fixed_string_insert_field.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/02426_low_cardinality_fixed_string_insert_field.sh b/tests/queries/0_stateless/02426_low_cardinality_fixed_string_insert_field.sh index dc9f1ec8ed2..7c367329b90 100755 --- a/tests/queries/0_stateless/02426_low_cardinality_fixed_string_insert_field.sh +++ b/tests/queries/0_stateless/02426_low_cardinality_fixed_string_insert_field.sh @@ -5,4 +5,4 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # shellcheck source=../shell_config.sh . "$CURDIR"/../shell_config.sh -${CLICKHOUSE_LOCAL} --structure 'x LowCardinality(FixedString(2454139))' --input-format Values --output-format TSV --query "SELECT * FROM table" <<< '(1)' | wc -c +${CLICKHOUSE_LOCAL} --allow_suspicious_fixed_string_types 1 --structure 'x LowCardinality(FixedString(2454139))' --input-format Values --output-format TSV --query "SELECT * FROM table" <<< '(1)' | wc -c