From 6a568ab692ea62a6e25c373feb6f6bedf346423b Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sat, 9 Dec 2017 23:56:53 +0300 Subject: [PATCH] Technically allowed empty arrays of unspecified type [#CLICKHOUSE-2]. --- .../NullableAdapterBlockInputStream.cpp | 41 +++++++------------ dbms/src/DataStreams/isConvertableTypes.cpp | 26 ------------ dbms/src/DataStreams/isConvertableTypes.h | 11 ----- dbms/src/Functions/FunctionsConversion.h | 13 ++++++ dbms/src/Parsers/ASTFunction.cpp | 40 +++++++++--------- .../00530_arrays_of_nothing.reference | 6 +++ .../0_stateless/00530_arrays_of_nothing.sql | 13 ++++++ 7 files changed, 67 insertions(+), 83 deletions(-) delete mode 100644 dbms/src/DataStreams/isConvertableTypes.cpp delete mode 100644 dbms/src/DataStreams/isConvertableTypes.h create mode 100644 dbms/tests/queries/0_stateless/00530_arrays_of_nothing.reference create mode 100644 dbms/tests/queries/0_stateless/00530_arrays_of_nothing.sql diff --git a/dbms/src/DataStreams/NullableAdapterBlockInputStream.cpp b/dbms/src/DataStreams/NullableAdapterBlockInputStream.cpp index 12142120221..f820650769d 100644 --- a/dbms/src/DataStreams/NullableAdapterBlockInputStream.cpp +++ b/dbms/src/DataStreams/NullableAdapterBlockInputStream.cpp @@ -2,7 +2,6 @@ #include #include #include -#include namespace DB @@ -107,33 +106,23 @@ void NullableAdapterBlockInputStream::buildActions( const auto & in_elem = in_sample.getByPosition(i); const auto & out_elem = out_sample.getByPosition(i); - if (isConvertableTypes(in_elem.type, out_elem.type)) - { - bool is_in_nullable = in_elem.type->isNullable(); - bool is_out_nullable = out_elem.type->isNullable(); + bool is_in_nullable = in_elem.type->isNullable(); + bool is_out_nullable = out_elem.type->isNullable(); - if (is_in_nullable && !is_out_nullable) - actions.push_back(TO_ORDINARY); - else if (!is_in_nullable && is_out_nullable) - actions.push_back(TO_NULLABLE); - else - actions.push_back(NONE); - - if (in_elem.name != out_elem.name) - rename.emplace_back(std::make_optional(out_elem.name)); - else - rename.emplace_back(); - - if (actions.back() != NONE || rename.back()) - must_transform = true; - } + if (is_in_nullable && !is_out_nullable) + actions.push_back(TO_ORDINARY); + else if (!is_in_nullable && is_out_nullable) + actions.push_back(TO_NULLABLE); else - { - throw Exception{String("Types must be the same for columns at same position. ") - + "Column " + in_elem.name + " has type " + in_elem.type->getName() - + ", but column " + out_elem.name + " has type " + out_elem.type->getName(), - ErrorCodes::TYPE_MISMATCH}; - } + actions.push_back(NONE); + + if (in_elem.name != out_elem.name) + rename.emplace_back(std::make_optional(out_elem.name)); + else + rename.emplace_back(); + + if (actions.back() != NONE || rename.back()) + must_transform = true; } } diff --git a/dbms/src/DataStreams/isConvertableTypes.cpp b/dbms/src/DataStreams/isConvertableTypes.cpp deleted file mode 100644 index 9f460650177..00000000000 --- a/dbms/src/DataStreams/isConvertableTypes.cpp +++ /dev/null @@ -1,26 +0,0 @@ -#include - -#include -#include -#include -#include - -namespace DB -{ - -bool isConvertableTypes(const DataTypePtr & from, const DataTypePtr & to) -{ - auto from_nn = removeNullable(from); - auto to_nn = removeNullable(to); - - if ( dynamic_cast(to_nn.get()) && - !dynamic_cast(from_nn.get())) - { - if (from_nn->isString() || from_nn->isInteger()) - return true; - } - - return from_nn->equals(*to_nn); -} - -} diff --git a/dbms/src/DataStreams/isConvertableTypes.h b/dbms/src/DataStreams/isConvertableTypes.h deleted file mode 100644 index af48f9abe38..00000000000 --- a/dbms/src/DataStreams/isConvertableTypes.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#include - -namespace DB -{ - -/// Check that type 'from' can be implicitly converted to type 'to'. -bool isConvertableTypes(const DataTypePtr & from, const DataTypePtr & to); - -} diff --git a/dbms/src/Functions/FunctionsConversion.h b/dbms/src/Functions/FunctionsConversion.h index d3371bdbffd..f64234241ed 100644 --- a/dbms/src/Functions/FunctionsConversion.h +++ b/dbms/src/Functions/FunctionsConversion.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -1388,6 +1389,16 @@ private: }; } + WrapperType createNothingWrapper(const IDataType * to_type) + { + DataTypePtr to_type_clone = to_type->clone(); + return [to_type_clone] (Block & block, const ColumnNumbers &, const size_t result) + { + /// Column of Nothing type is trivially convertible to any other column + block.getByPosition(result).column = to_type_clone->createConstColumn(block.rows(), to_type_clone->getDefault())->convertToFullColumnIfConst(); + }; + } + /// Actions to be taken when performing a conversion. struct NullableConversion { @@ -1494,6 +1505,8 @@ private: { if (from_type->equals(*to_type)) return createIdentityWrapper(from_type); + else if (checkDataType(from_type.get())) + return createNothingWrapper(to_type); else if (const auto to_actual_type = checkAndGetDataType(to_type)) return createWrapper(from_type, to_actual_type); else if (const auto to_actual_type = checkAndGetDataType(to_type)) diff --git a/dbms/src/Parsers/ASTFunction.cpp b/dbms/src/Parsers/ASTFunction.cpp index 1a8766c0d99..d752cdd4374 100644 --- a/dbms/src/Parsers/ASTFunction.cpp +++ b/dbms/src/Parsers/ASTFunction.cpp @@ -123,23 +123,23 @@ void ASTFunction::formatImplWithoutAlias(const FormatSettings & settings, Format { const char * operators[] = { - "multiply", " * ", - "divide", " / ", - "modulo", " % ", - "plus", " + ", - "minus", " - ", - "notEquals", " != ", - "lessOrEquals", " <= ", - "greaterOrEquals", " >= ", - "less", " < ", - "greater", " > ", - "equals", " = ", - "like", " LIKE ", - "notLike", " NOT LIKE ", - "in", " IN ", - "notIn", " NOT IN ", - "globalIn", " GLOBAL IN ", - "globalNotIn", " GLOBAL NOT IN ", + "multiply", " * ", + "divide", " / ", + "modulo", " % ", + "plus", " + ", + "minus", " - ", + "notEquals", " != ", + "lessOrEquals", " <= ", + "greaterOrEquals", " >= ", + "less", " < ", + "greater", " > ", + "equals", " = ", + "like", " LIKE ", + "notLike", " NOT LIKE ", + "in", " IN ", + "notIn", " NOT IN ", + "globalIn", " GLOBAL IN ", + "globalNotIn", " GLOBAL NOT IN ", nullptr }; @@ -205,8 +205,8 @@ void ASTFunction::formatImplWithoutAlias(const FormatSettings & settings, Format { const char * operators[] = { - "and", " AND ", - "or", " OR ", + "and", " AND ", + "or", " OR ", nullptr }; @@ -229,7 +229,7 @@ void ASTFunction::formatImplWithoutAlias(const FormatSettings & settings, Format } } - if (!written && arguments->children.size() >= 1 && 0 == strcmp(name.c_str(), "array")) + if (!written && 0 == strcmp(name.c_str(), "array")) { settings.ostr << (settings.hilite ? hilite_operator : "") << '[' << (settings.hilite ? hilite_none : ""); for (size_t i = 0; i < arguments->children.size(); ++i) diff --git a/dbms/tests/queries/0_stateless/00530_arrays_of_nothing.reference b/dbms/tests/queries/0_stateless/00530_arrays_of_nothing.reference new file mode 100644 index 00000000000..8372a06fc62 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00530_arrays_of_nothing.reference @@ -0,0 +1,6 @@ +[[[[],[]]]] +[[1],[]] +[[[[],['']]]] +['Hello'] +[1] [[],[]] +[] \N [[],[NULL],[NULL,'Hello']] diff --git a/dbms/tests/queries/0_stateless/00530_arrays_of_nothing.sql b/dbms/tests/queries/0_stateless/00530_arrays_of_nothing.sql new file mode 100644 index 00000000000..704bc574647 --- /dev/null +++ b/dbms/tests/queries/0_stateless/00530_arrays_of_nothing.sql @@ -0,0 +1,13 @@ +SELECT [[[[],[]]]]; +SELECT [[1], []]; +SELECT [[[[],['']]]]; +SELECT concat([], ['Hello'], []); +SELECT arrayPushBack([], 1), arrayPushFront([[]], []); + +DROP TABLE IF EXISTS test.arr; +CREATE TABLE test.arr (x Array(String), y Nullable(String), z Array(Array(Nullable(String)))) ENGINE = TinyLog; + +INSERT INTO test.arr SELECT [], NULL, [[], [NULL], [NULL, 'Hello']]; +SELECT * FROM test.arr; + +DROP TABLE test.arr;