From f1e9f6714cf29d9192f844e754901e0995f7413e Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Sun, 29 Nov 2020 15:53:29 +0300 Subject: [PATCH] Backport #17145 to 20.12: Fix unmatched type comparison in KeyCondition --- src/DataTypes/getLeastSupertype.cpp | 25 ++++++- src/Storages/MergeTree/KeyCondition.cpp | 68 ++++++++++++++----- ...9_date_datetime_index_comparison.reference | 5 ++ .../01579_date_datetime_index_comparison.sql | 16 +++++ 4 files changed, 95 insertions(+), 19 deletions(-) create mode 100644 tests/queries/0_stateless/01579_date_datetime_index_comparison.reference create mode 100644 tests/queries/0_stateless/01579_date_datetime_index_comparison.sql diff --git a/src/DataTypes/getLeastSupertype.cpp b/src/DataTypes/getLeastSupertype.cpp index 4343408180..0d2d3e6620 100644 --- a/src/DataTypes/getLeastSupertype.cpp +++ b/src/DataTypes/getLeastSupertype.cpp @@ -358,6 +358,8 @@ DataTypePtr getLeastSupertype(const DataTypes & types) maximize(max_bits_of_unsigned_integer, 32); else if (typeid_cast(type.get())) maximize(max_bits_of_unsigned_integer, 64); + else if (typeid_cast(type.get())) + maximize(max_bits_of_unsigned_integer, 256); else if (typeid_cast(type.get())) maximize(max_bits_of_signed_integer, 8); else if (typeid_cast(type.get())) @@ -366,6 +368,10 @@ DataTypePtr getLeastSupertype(const DataTypes & types) maximize(max_bits_of_signed_integer, 32); else if (typeid_cast(type.get())) maximize(max_bits_of_signed_integer, 64); + else if (typeid_cast(type.get())) + maximize(max_bits_of_signed_integer, 128); + else if (typeid_cast(type.get())) + maximize(max_bits_of_signed_integer, 256); else if (typeid_cast(type.get())) maximize(max_mantissa_bits_of_floating, 24); else if (typeid_cast(type.get())) @@ -386,7 +392,18 @@ DataTypePtr getLeastSupertype(const DataTypes & types) /// If unsigned is not covered by signed. if (max_bits_of_signed_integer && max_bits_of_unsigned_integer >= max_bits_of_signed_integer) - ++min_bit_width_of_integer; + { + // Because 128 and 256 bit integers are significantly slower, we should not promote to them. + // But if we already have wide numbers, promotion is necessary. + if (min_bit_width_of_integer != 64) + ++min_bit_width_of_integer; + else + throw Exception( + getExceptionMessagePrefix(types) + + " because some of them are signed integers and some are unsigned integers," + " but there is no signed integer type, that can exactly represent all required unsigned integer values", + ErrorCodes::NO_COMMON_TYPE); + } /// If the result must be floating. if (max_mantissa_bits_of_floating) @@ -413,6 +430,10 @@ DataTypePtr getLeastSupertype(const DataTypes & types) return std::make_shared(); else if (min_bit_width_of_integer <= 64) return std::make_shared(); + else if (min_bit_width_of_integer <= 128) + return std::make_shared(); + else if (min_bit_width_of_integer <= 256) + return std::make_shared(); else throw Exception(getExceptionMessagePrefix(types) + " because some of them are signed integers and some are unsigned integers," @@ -429,6 +450,8 @@ DataTypePtr getLeastSupertype(const DataTypes & types) return std::make_shared(); else if (min_bit_width_of_integer <= 64) return std::make_shared(); + else if (min_bit_width_of_integer <= 256) + return std::make_shared(); else throw Exception("Logical error: " + getExceptionMessagePrefix(types) + " but as all data types are unsigned integers, we must have found maximum unsigned integer type", ErrorCodes::NO_COMMON_TYPE); diff --git a/src/Storages/MergeTree/KeyCondition.cpp b/src/Storages/MergeTree/KeyCondition.cpp index 7b2044ef76..7aff1da1fe 100644 --- a/src/Storages/MergeTree/KeyCondition.cpp +++ b/src/Storages/MergeTree/KeyCondition.cpp @@ -2,12 +2,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -23,7 +25,6 @@ #include #include - namespace DB { @@ -959,9 +960,6 @@ bool KeyCondition::isKeyPossiblyWrappedByMonotonicFunctionsImpl( static void castValueToType(const DataTypePtr & desired_type, Field & src_value, const DataTypePtr & src_type, const ASTPtr & node) { - if (desired_type->equals(*src_type)) - return; - try { src_value = convertFieldToType(src_value, *desired_type, src_type.get()); @@ -1068,15 +1066,6 @@ bool KeyCondition::tryParseAtomFromAST(const ASTPtr & node, const Context & cont if (key_column_num == static_cast(-1)) throw Exception("`key_column_num` wasn't initialized. It is a bug.", ErrorCodes::LOGICAL_ERROR); - /// Transformed constant must weaken the condition, for example "x > 5" must weaken to "round(x) >= 5" - if (is_constant_transformed) - { - if (func_name == "less") - func_name = "lessOrEquals"; - else if (func_name == "greater") - func_name = "greaterOrEquals"; - } - /// Replace on to <-sign> if (key_arg_pos == 1) { @@ -1098,12 +1087,55 @@ bool KeyCondition::tryParseAtomFromAST(const ASTPtr & node, const Context & cont } } - bool cast_not_needed = - is_set_const /// Set args are already casted inside Set::createFromAST - || (isNativeNumber(key_expr_type) && isNativeNumber(const_type)); /// Numbers are accurately compared without cast. + bool cast_not_needed = is_set_const /// Set args are already casted inside Set::createFromAST + || ((isNativeNumber(key_expr_type) || isDateTime(key_expr_type)) + && (isNativeNumber(const_type) || isDateTime(const_type))); /// Numbers and DateTime are accurately compared without cast. + + if (!cast_not_needed && !key_expr_type->equals(*const_type)) + { + if (const_value.getType() == Field::Types::String) + { + const_value = convertFieldToType(const_value, *key_expr_type); + if (const_value.isNull()) + return false; + // No need to set is_constant_transformed because we're doing exact conversion + } + else + { + DataTypePtr common_type = getLeastSupertype({key_expr_type, const_type}); + if (!const_type->equals(*common_type)) + { + castValueToType(common_type, const_value, const_type, node); + + // Need to set is_constant_transformed unless we're doing exact conversion + if (!key_expr_type->equals(*common_type)) + is_constant_transformed = true; + } + if (!key_expr_type->equals(*common_type)) + { + ColumnsWithTypeAndName arguments{ + {nullptr, key_expr_type, ""}, {DataTypeString().createColumnConst(1, common_type->getName()), common_type, ""}}; + FunctionOverloadResolverPtr func_builder_cast + = std::make_shared(CastOverloadResolver::createImpl(false)); + auto func_cast = func_builder_cast->build(arguments); + + /// If we know the given range only contains one value, then we treat all functions as positive monotonic. + if (!func_cast || (!single_point && !func_cast->hasInformationAboutMonotonicity())) + return false; + chain.push_back(func_cast); + } + } + } + + /// Transformed constant must weaken the condition, for example "x > 5" must weaken to "round(x) >= 5" + if (is_constant_transformed) + { + if (func_name == "less") + func_name = "lessOrEquals"; + else if (func_name == "greater") + func_name = "greaterOrEquals"; + } - if (!cast_not_needed) - castValueToType(key_expr_type, const_value, const_type, node); } else return false; diff --git a/tests/queries/0_stateless/01579_date_datetime_index_comparison.reference b/tests/queries/0_stateless/01579_date_datetime_index_comparison.reference new file mode 100644 index 0000000000..e31666d96b --- /dev/null +++ b/tests/queries/0_stateless/01579_date_datetime_index_comparison.reference @@ -0,0 +1,5 @@ +1 +Array(Int64) +Array(Int128) +Array(Int128) +Array(UInt256) diff --git a/tests/queries/0_stateless/01579_date_datetime_index_comparison.sql b/tests/queries/0_stateless/01579_date_datetime_index_comparison.sql new file mode 100644 index 0000000000..60de837f8f --- /dev/null +++ b/tests/queries/0_stateless/01579_date_datetime_index_comparison.sql @@ -0,0 +1,16 @@ +drop table if exists test_index; + +create table test_index(date Date) engine MergeTree partition by toYYYYMM(date) order by date; + +insert into test_index values('2020-10-30'); + +select 1 from test_index where date < toDateTime('2020-10-30 06:00:00'); + +drop table if exists test_index; + +select toTypeName([-1, toUInt32(1)]); +-- We don't promote to wide integers +select toTypeName([-1, toUInt64(1)]); -- { serverError 386 } +select toTypeName([-1, toInt128(1)]); +select toTypeName([toInt64(-1), toInt128(1)]); +select toTypeName([toUInt64(1), toUInt256(1)]); -- GitLab