From a695bb80c77bd82763ba36f89d718d49247da498 Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Wed, 20 Nov 2024 15:07:40 +0000 Subject: [PATCH] Backport #71761 to 24.8: Forbid Dynamic/Variant types in min/max functions to avoid confusion --- .../AggregateFunctionsArgMinArgMax.cpp | 8 ++++++++ .../AggregateFunctionsMinMax.cpp | 8 ++++++++ ...ggregateFunctionCombinatorsArgMinArgMax.cpp | 8 ++++++++ .../MergeTree/MergeTreeIndexMinMax.cpp | 8 ++++++++ .../03271_dynamic_variant_in_min_max.reference | 0 .../03271_dynamic_variant_in_min_max.sql | 18 ++++++++++++++++++ 6 files changed, 50 insertions(+) create mode 100644 tests/queries/0_stateless/03271_dynamic_variant_in_min_max.reference create mode 100644 tests/queries/0_stateless/03271_dynamic_variant_in_min_max.sql diff --git a/src/AggregateFunctions/AggregateFunctionsArgMinArgMax.cpp b/src/AggregateFunctions/AggregateFunctionsArgMinArgMax.cpp index 9608ca26f37..cc72a26af16 100644 --- a/src/AggregateFunctions/AggregateFunctionsArgMinArgMax.cpp +++ b/src/AggregateFunctions/AggregateFunctionsArgMinArgMax.cpp @@ -79,6 +79,14 @@ public: "Illegal type {} of second argument of aggregate function {} because the values of that data type are not comparable", type_val->getName(), getName()); + + if (isDynamic(this->type_val) || isVariant(this->type_val)) + throw Exception( + ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, + "Illegal type {} of argument of aggregate function {} because the column of that type can contain values with different " + "data types. Consider using typed subcolumns or cast column to a specific data type", + this->type_val->getName(), + getName()); } void create(AggregateDataPtr __restrict place) const override /// NOLINT diff --git a/src/AggregateFunctions/AggregateFunctionsMinMax.cpp b/src/AggregateFunctions/AggregateFunctionsMinMax.cpp index 5fa9a4ff5d1..0c21be574c4 100644 --- a/src/AggregateFunctions/AggregateFunctionsMinMax.cpp +++ b/src/AggregateFunctions/AggregateFunctionsMinMax.cpp @@ -35,6 +35,14 @@ public: "Illegal type {} of argument of aggregate function {} because the values of that data type are not comparable", this->result_type->getName(), getName()); + + if (isDynamic(this->result_type) || isVariant(this->result_type)) + throw Exception( + ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, + "Illegal type {} of argument of aggregate function {} because the column of that type can contain values with different " + "data types. Consider using typed subcolumns or cast column to a specific data type", + this->result_type->getName(), + getName()); } String getName() const override diff --git a/src/AggregateFunctions/Combinators/AggregateFunctionCombinatorsArgMinArgMax.cpp b/src/AggregateFunctions/Combinators/AggregateFunctionCombinatorsArgMinArgMax.cpp index a1716f18725..fc8b5b6d4cd 100644 --- a/src/AggregateFunctions/Combinators/AggregateFunctionCombinatorsArgMinArgMax.cpp +++ b/src/AggregateFunctions/Combinators/AggregateFunctionCombinatorsArgMinArgMax.cpp @@ -63,6 +63,14 @@ public: "Illegal type {} for combinator {} because the values of that data type are not comparable", arguments[key_col]->getName(), getName()); + + if (isDynamic(arguments[key_col]) || isVariant(arguments[key_col])) + throw Exception( + ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, + "Illegal type {} of argument of aggregate function {} because the column of that type can contain values with different " + "data types. Consider using typed subcolumns or cast column to a specific data type", + arguments[key_col]->getName(), + getName()); } String getName() const override diff --git a/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp b/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp index c60d63a59ba..3af551d8165 100644 --- a/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp @@ -231,6 +231,14 @@ void minmaxIndexValidator(const IndexDescription & index, bool attach) "Data type of argument for minmax index must be comparable, got {} type for column {} instead", column.type->getName(), column.name); } + + if (isDynamic(column.type) || isVariant(column.type)) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "{} data type of column {} is not allowed in minmax index because the column of that type can contain values with different data " + "types. Consider using typed subcolumns or cast column to a specific data type", + column.type->getName(), column.name); + } } } diff --git a/tests/queries/0_stateless/03271_dynamic_variant_in_min_max.reference b/tests/queries/0_stateless/03271_dynamic_variant_in_min_max.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/03271_dynamic_variant_in_min_max.sql b/tests/queries/0_stateless/03271_dynamic_variant_in_min_max.sql new file mode 100644 index 00000000000..a7fea941548 --- /dev/null +++ b/tests/queries/0_stateless/03271_dynamic_variant_in_min_max.sql @@ -0,0 +1,18 @@ +set allow_experimental_dynamic_type=1; +select max(number::Dynamic) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select min(number::Dynamic) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select argMax(number, number::Dynamic) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select argMin(number, number::Dynamic) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select anyArgMax(number, number::Dynamic) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select anyArgMin(number, number::Dynamic) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +create table test (d Dynamic, index idx d type minmax); -- {serverError BAD_ARGUMENTS} + +set allow_experimental_variant_type=1; +select max(number::Variant(UInt64)) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select min(number::Variant(UInt64)) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select argMax(number, number::Variant(UInt64)) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select argMin(number, number::Variant(UInt64)) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select anyArgMax(number, number::Variant(UInt64)) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +select anyArgMin(number, number::Variant(UInt64)) from numbers(10); -- {serverError ILLEGAL_TYPE_OF_ARGUMENT} +create table test (d Variant(UInt64), index idx d type minmax); -- {serverError BAD_ARGUMENTS} +