From 160d8a64161ae63884b542985559809d6e2193ed Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 11 Oct 2019 20:52:32 +0300 Subject: [PATCH] Ignore non-convertible values at any depth on the right side of IN operator. The values that are not convertible to the left argument type can't match anyway, so it is safe to discard them. --- dbms/src/Interpreters/convertFieldToType.cpp | 43 +++++++++++++++---- .../Formats/Impl/ValuesBlockInputFormat.cpp | 9 ++-- .../00810_in_operators_segfault.reference | 3 ++ .../00810_in_operators_segfault.sql | 6 +-- ...17_in_unconvertible_complex_type.reference | 6 +++ .../01017_in_unconvertible_complex_type.sql | 13 ++++++ 6 files changed, 63 insertions(+), 17 deletions(-) create mode 100644 dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.reference create mode 100644 dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.sql diff --git a/dbms/src/Interpreters/convertFieldToType.cpp b/dbms/src/Interpreters/convertFieldToType.cpp index 8bc6ee5b9d5..b9d965b017f 100644 --- a/dbms/src/Interpreters/convertFieldToType.cpp +++ b/dbms/src/Interpreters/convertFieldToType.cpp @@ -225,21 +225,23 @@ Field convertFieldToTypeImpl(const Field & src, const IDataType & type, const ID { if (src.getType() == Field::Types::Array) { - const DataTypePtr nested_type = removeNullable(type_array->getNestedType()); - const Array & src_arr = src.get(); size_t src_arr_size = src_arr.size(); + auto & element_type = *(type_array->getNestedType()); + bool have_unconvertible_element = false; Array res(src_arr_size); for (size_t i = 0; i < src_arr_size; ++i) { - res[i] = convertFieldToType(src_arr[i], *nested_type); - if (res[i].isNull() && !type_array->getNestedType()->isNullable()) - throw Exception("Type mismatch of array elements in IN or VALUES section. Expected: " + type_array->getNestedType()->getName() - + ". Got NULL in position " + toString(i + 1), ErrorCodes::TYPE_MISMATCH); + res[i] = convertFieldToType(src_arr[i], element_type); + if (res[i].isNull() && !element_type.isNullable()) + { + // See the comment for Tuples below. + have_unconvertible_element = true; + } } - return res; + return have_unconvertible_element ? Field(Null()) : Field(res); } } else if (const DataTypeTuple * type_tuple = typeid_cast(&type)) @@ -255,10 +257,33 @@ Field convertFieldToTypeImpl(const Field & src, const IDataType & type, const ID + toString(dst_tuple_size) + ", actual size: " + toString(src_tuple_size), ErrorCodes::TYPE_MISMATCH); TupleBackend res(dst_tuple_size); + bool have_unconvertible_element = false; for (size_t i = 0; i < dst_tuple_size; ++i) - res[i] = convertFieldToType(src_tuple[i], *type_tuple->getElements()[i]); + { + auto & element_type = *(type_tuple->getElements()[i]); + res[i] = convertFieldToType(src_tuple[i], element_type); + if (!res[i].isNull() || element_type.isNullable()) + continue; - return res; + /* + * Either the source element was Null, or the conversion did not + * succeed, because the source and the requested types of the + * element are compatible, but the value is not convertible + * (e.g. trying to convert -1 from Int8 to UInt8). In these + * cases, consider the whole tuple also compatible but not + * convertible. According to the specification of this function, + * we must return Null in this case. + * + * The following elements might be not even compatible, so it + * makes sense to check them to detect user errors. Remember + * that there is an unconvertible element, and try to process + * the remaining ones. The convertFieldToType for each element + * will throw if it detects incompatibility. + */ + have_unconvertible_element = true; + } + + return have_unconvertible_element ? Field(Null()) : Field(res); } } else if (const DataTypeAggregateFunction * agg_func_type = typeid_cast(&type)) diff --git a/dbms/src/Processors/Formats/Impl/ValuesBlockInputFormat.cpp b/dbms/src/Processors/Formats/Impl/ValuesBlockInputFormat.cpp index 8fda4294e3d..5c5bca9f962 100644 --- a/dbms/src/Processors/Formats/Impl/ValuesBlockInputFormat.cpp +++ b/dbms/src/Processors/Formats/Impl/ValuesBlockInputFormat.cpp @@ -26,7 +26,7 @@ namespace ErrorCodes extern const int CANNOT_READ_ARRAY_FROM_TEXT; extern const int CANNOT_PARSE_DATE; extern const int SYNTAX_ERROR; - extern const int VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE; + extern const int TYPE_MISMATCH; extern const int SUPPORT_IS_DISABLED; } @@ -291,11 +291,10 @@ ValuesBlockInputFormat::parseExpression(IColumn & column, size_t column_idx) if (value.isNull() && !type.isNullable()) { buf.rollbackToCheckpoint(); - throw Exception{"Expression returns value " + applyVisitor(FieldVisitorToString(), value) - + ", that is out of range of type " + type.getName() - + ", at: " + + throw Exception{"Cannot insert NULL value into a column of type '" + type.getName() + "'" + + " at: " + String(buf.position(), std::min(SHOW_CHARS_ON_SYNTAX_ERROR, buf.buffer().end() - buf.position())), - ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE}; + ErrorCodes::TYPE_MISMATCH}; } column.insert(value); diff --git a/dbms/tests/queries/0_stateless/00810_in_operators_segfault.reference b/dbms/tests/queries/0_stateless/00810_in_operators_segfault.reference index e69de29bb2d..16db301bb51 100644 --- a/dbms/tests/queries/0_stateless/00810_in_operators_segfault.reference +++ b/dbms/tests/queries/0_stateless/00810_in_operators_segfault.reference @@ -0,0 +1,3 @@ +1 +0 +1 diff --git a/dbms/tests/queries/0_stateless/00810_in_operators_segfault.sql b/dbms/tests/queries/0_stateless/00810_in_operators_segfault.sql index 1fa525eaccc..8e4a4723608 100644 --- a/dbms/tests/queries/0_stateless/00810_in_operators_segfault.sql +++ b/dbms/tests/queries/0_stateless/00810_in_operators_segfault.sql @@ -1,5 +1,5 @@ SET send_logs_level = 'none'; -SELECT globalNotIn(['"wh'], [NULL]); -- { serverError 53 } -SELECT globalIn([''], [NULL]); -- { serverError 53 } -SELECT notIn([['']], [[NULL]]); -- { serverError 53 } +SELECT globalNotIn(['"wh'], [NULL]); +SELECT globalIn([''], [NULL]); +SELECT notIn([['']], [[NULL]]); diff --git a/dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.reference b/dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.reference new file mode 100644 index 00000000000..f7eb44d66e0 --- /dev/null +++ b/dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.reference @@ -0,0 +1,6 @@ +0 +0 +0 +0 +0 +0 diff --git a/dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.sql b/dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.sql new file mode 100644 index 00000000000..d675c195726 --- /dev/null +++ b/dbms/tests/queries/0_stateless/01017_in_unconvertible_complex_type.sql @@ -0,0 +1,13 @@ +-- When left and right element types are compatible, but the particular value +-- on the right is not in the range of the left type, it should be ignored. +select (toUInt8(1)) in (-1); +select (toUInt8(0)) in (-1); +select (toUInt8(255)) in (-1); + +select [toUInt8(1)] in [-1]; +select [toUInt8(0)] in [-1]; +select [toUInt8(255)] in [-1]; + +-- When left and right element types are not compatible, we should get an error. +select (toUInt8(1)) in ('a'); -- { serverError 53 } +select [toUInt8(1)] in ['a']; -- { serverError 53 }