diff --git a/docs/en/sql-reference/data-types/variant.md b/docs/en/sql-reference/data-types/variant.md index 52db63f2a3d..7d10d4b0e97 100644 --- a/docs/en/sql-reference/data-types/variant.md +++ b/docs/en/sql-reference/data-types/variant.md @@ -12,6 +12,11 @@ has a value of either type `T1` or `T2` or ... or `TN` or none of them (`NULL` v The order of nested types doesn't matter: Variant(T1, T2) = Variant(T2, T1). Nested types can be arbitrary types except Nullable(...), LowCardinality(Nullable(...)) and Variant(...) types. +:::note +It's not recommended to use similar types as variants (for example different numeric types like `Variant(UInt32, Int64)` or different date types like `Variant(Date, DateTime)`), +because working with values of such types can lead to ambiguity. By default, creating such `Variant` type will lead to an exception, but can be enabled using setting `allow_suspicious_variant_types` +::: + :::note The Variant data type is an experimental feature. To use it, set `allow_experimental_variant_type = 1`. ::: @@ -371,4 +376,22 @@ SELECT v2, v2.`Array(UInt32)`, variantType(v2) FROM test WHERE variantType(v2) = └────┴──────────────────┴─────────────────┘ ``` +**Note:** values of variants with different numeric types are considered as different variants and not compared between each other, their type names are compared instead. +Example: + +```sql +SET allow_suspicious_variant_types = 1; +CREATE TABLE test (v Variant(UInt32, Int64)) ENGINE=Memory; +INSERT INTO test VALUES (1::UInt32), (1::Int64), (100::UInt32), (100::Int64); +SELECT v, variantType(v) FROM test ORDER by v; +``` + +```text +┌─v───┬─variantType(v)─┐ +│ 1 │ Int64 │ +│ 100 │ Int64 │ +│ 1 │ UInt32 │ +│ 100 │ UInt32 │ +└─────┴────────────────┘ +``` diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 433195af9c3..b6ec444dd39 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -160,6 +160,7 @@ class IColumn; 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 misuse", 0) \ M(Bool, allow_suspicious_indices, false, "Reject primary/secondary indexes and sorting keys with identical expressions", 0) \ M(Bool, allow_suspicious_ttl_expressions, false, "Reject TTL expressions that don't depend on any of table's columns. It indicates a user error most of the time.", 0) \ + M(Bool, allow_suspicious_variant_types, false, "In CREATE TABLE statement allows specifying Variant type with similar variant types (for example, with different numeric or date types). Enabling this setting may introduce some ambiguity when working with values with similar types.", 0) \ M(Bool, compile_expressions, false, "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/parseColumnsListForTableFunction.cpp b/src/Interpreters/parseColumnsListForTableFunction.cpp index 1499568cec9..1333112fa51 100644 --- a/src/Interpreters/parseColumnsListForTableFunction.cpp +++ b/src/Interpreters/parseColumnsListForTableFunction.cpp @@ -1,21 +1,23 @@ +#include +#include +#include +#include +#include +#include +#include +#include #include #include #include -#include -#include -#include -#include -#include -#include namespace DB { namespace ErrorCodes { - extern const int LOGICAL_ERROR; - extern const int SUSPICIOUS_TYPE_FOR_LOW_CARDINALITY; - extern const int ILLEGAL_COLUMN; +extern const int LOGICAL_ERROR; +extern const int SUSPICIOUS_TYPE_FOR_LOW_CARDINALITY; +extern const int ILLEGAL_COLUMN; } @@ -73,6 +75,34 @@ void validateDataType(const DataTypePtr & type_to_check, const DataTypeValidatio data_type.getName()); } } + + if (!settings.allow_suspicious_variant_types) + { + if (const auto * variant_type = typeid_cast(&data_type)) + { + const auto & variants = variant_type->getVariants(); + chassert(!variants.empty()); + for (size_t i = 0; i < variants.size() - 1; ++i) + { + for (size_t j = i + 1; j < variants.size(); ++j) + { + if (auto supertype = tryGetLeastSupertype(DataTypes{variants[i], variants[j]})) + { + throw Exception( + ErrorCodes::ILLEGAL_COLUMN, + "Cannot create column with type '{}' because variants '{}' and '{}' have similar types and working with values " + "of these types may lead to ambiguity. " + "Consider using common single variant '{}' instead of these 2 variants or set setting allow_suspicious_variant_types = 1 " + "in order to allow it", + data_type.getName(), + variants[i]->getName(), + variants[j]->getName(), + supertype->getName()); + } + } + } + } + } }; validate_callback(*type_to_check); @@ -104,7 +134,8 @@ bool tryParseColumnsListFromString(const std::string & structure, ColumnsDescrip const char * start = structure.data(); const char * end = structure.data() + structure.size(); - ASTPtr columns_list_raw = tryParseQuery(parser, start, end, error, false, "columns declaration list", false, settings.max_query_size, settings.max_parser_depth); + ASTPtr columns_list_raw = tryParseQuery( + parser, start, end, error, false, "columns declaration list", false, settings.max_query_size, settings.max_parser_depth); if (!columns_list_raw) return false; diff --git a/src/Interpreters/parseColumnsListForTableFunction.h b/src/Interpreters/parseColumnsListForTableFunction.h index 1fbbfa4b12f..99865185cc7 100644 --- a/src/Interpreters/parseColumnsListForTableFunction.h +++ b/src/Interpreters/parseColumnsListForTableFunction.h @@ -19,6 +19,7 @@ struct DataTypeValidationSettings , allow_experimental_object_type(settings.allow_experimental_object_type) , allow_suspicious_fixed_string_types(settings.allow_suspicious_fixed_string_types) , allow_experimental_variant_type(settings.allow_experimental_variant_type) + , allow_suspicious_variant_types(settings.allow_suspicious_variant_types) { } @@ -26,6 +27,7 @@ struct DataTypeValidationSettings bool allow_experimental_object_type = true; bool allow_suspicious_fixed_string_types = true; bool allow_experimental_variant_type = true; + bool allow_suspicious_variant_types = true; }; void validateDataType(const DataTypePtr & type, const DataTypeValidationSettings & settings); diff --git a/tests/queries/0_stateless/02940_variant_text_deserialization.sql b/tests/queries/0_stateless/02940_variant_text_deserialization.sql index 041d02088ef..b909b2b6790 100644 --- a/tests/queries/0_stateless/02940_variant_text_deserialization.sql +++ b/tests/queries/0_stateless/02940_variant_text_deserialization.sql @@ -1,4 +1,5 @@ set allow_experimental_variant_type = 1; +set allow_suspicious_variant_types = 1; set session_timezone = 'UTC'; select 'JSON'; @@ -263,4 +264,4 @@ select v, variantElement(v, 'Array(LowCardinality(Nullable(String)))') from form select 'Nullable'; select v, variantElement(v, 'Array(Nullable(String))') from format(Values, 'v Variant(String, Array(Nullable(String)))', '(NULL), (''string''), ([''hello'', null, ''world''])') format Values; -select ''; \ No newline at end of file +select ''; diff --git a/tests/queries/0_stateless/02941_variant_type_1.sh b/tests/queries/0_stateless/02941_variant_type_1.sh index ed365bbd244..773a8c4a5e4 100755 --- a/tests/queries/0_stateless/02941_variant_type_1.sh +++ b/tests/queries/0_stateless/02941_variant_type_1.sh @@ -7,7 +7,7 @@ CLICKHOUSE_LOG_COMMENT= # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1" +CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --allow_suspicious_variant_types=1" function test1_insert() { diff --git a/tests/queries/0_stateless/02941_variant_type_2.sh b/tests/queries/0_stateless/02941_variant_type_2.sh index 23666a9b4a8..509c537e7fc 100755 --- a/tests/queries/0_stateless/02941_variant_type_2.sh +++ b/tests/queries/0_stateless/02941_variant_type_2.sh @@ -7,7 +7,7 @@ CLICKHOUSE_LOG_COMMENT= # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1" +CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --allow_suspicious_variant_types=1" function test4_insert() { diff --git a/tests/queries/0_stateless/02941_variant_type_3.sh b/tests/queries/0_stateless/02941_variant_type_3.sh index d6309e26414..a0efead280a 100755 --- a/tests/queries/0_stateless/02941_variant_type_3.sh +++ b/tests/queries/0_stateless/02941_variant_type_3.sh @@ -7,7 +7,7 @@ CLICKHOUSE_LOG_COMMENT= # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1" +CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --allow_suspicious_variant_types=1" function test5_insert() { diff --git a/tests/queries/0_stateless/02941_variant_type_4.sh b/tests/queries/0_stateless/02941_variant_type_4.sh index 5ea04db4bb4..336540d1e79 100755 --- a/tests/queries/0_stateless/02941_variant_type_4.sh +++ b/tests/queries/0_stateless/02941_variant_type_4.sh @@ -7,7 +7,7 @@ CLICKHOUSE_LOG_COMMENT= # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1" +CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --allow_suspicious_variant_types=1" function test6_insert() { diff --git a/tests/queries/0_stateless/02943_variant_read_subcolumns.sh b/tests/queries/0_stateless/02943_variant_read_subcolumns.sh index 88be09c2036..b816a20c818 100755 --- a/tests/queries/0_stateless/02943_variant_read_subcolumns.sh +++ b/tests/queries/0_stateless/02943_variant_read_subcolumns.sh @@ -7,7 +7,7 @@ CLICKHOUSE_LOG_COMMENT= # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --use_variant_as_common_type=1 " +CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --use_variant_as_common_type=1 --allow_suspicious_variant_types=1" function test() diff --git a/tests/queries/0_stateless/02943_variant_type_with_different_local_and_global_order.sh b/tests/queries/0_stateless/02943_variant_type_with_different_local_and_global_order.sh index e4c1206263f..3bb37719a3f 100755 --- a/tests/queries/0_stateless/02943_variant_type_with_different_local_and_global_order.sh +++ b/tests/queries/0_stateless/02943_variant_type_with_different_local_and_global_order.sh @@ -7,7 +7,7 @@ CLICKHOUSE_LOG_COMMENT= # shellcheck source=../shell_config.sh . "$CUR_DIR"/../shell_config.sh -CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --use_variant_as_common_type=1 " +CH_CLIENT="$CLICKHOUSE_CLIENT --allow_experimental_variant_type=1 --use_variant_as_common_type=1 --allow_suspicious_variant_types=1" function test1_insert() diff --git a/tests/queries/0_stateless/02999_variant_suspicious_types.reference b/tests/queries/0_stateless/02999_variant_suspicious_types.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/02999_variant_suspicious_types.sql b/tests/queries/0_stateless/02999_variant_suspicious_types.sql new file mode 100644 index 00000000000..8cdbfc13adb --- /dev/null +++ b/tests/queries/0_stateless/02999_variant_suspicious_types.sql @@ -0,0 +1,7 @@ +set allow_suspicious_variant_types=0; +select 42::Variant(UInt32, Int64); -- {serverError ILLEGAL_COLUMN} +select [42]::Variant(Array(UInt32), Array(Int64)); -- {serverError ILLEGAL_COLUMN} +select 'Hello'::Variant(String, LowCardinality(String)); -- {serverError ILLEGAL_COLUMN} +select (1, 'Hello')::Variant(Tuple(UInt32, String), Tuple(Int64, String)); -- {serverError ILLEGAL_COLUMN} +select map(42, 42)::Variant(Map(UInt64, UInt32), Map(UInt64, Int64)); -- {serverError ILLEGAL_COLUMN} +