From 892ab4ce8afadc04a0c18e1accfd4aba0cc5e840 Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Wed, 31 Oct 2018 12:17:47 +0300
Subject: [PATCH] Better code for check on null values inside array IN
 expression

---
 dbms/src/Formats/ValuesRowInputStream.cpp     | 28 ++++---------------
 dbms/src/Interpreters/convertFieldToType.cpp  |  5 ++++
 .../00748_insert_array_with_null.reference    |  1 +
 .../00748_insert_array_with_null.sql          | 11 ++++++++
 4 files changed, 23 insertions(+), 22 deletions(-)
 create mode 100644 dbms/tests/queries/0_stateless/00748_insert_array_with_null.reference
 create mode 100644 dbms/tests/queries/0_stateless/00748_insert_array_with_null.sql

diff --git a/dbms/src/Formats/ValuesRowInputStream.cpp b/dbms/src/Formats/ValuesRowInputStream.cpp
index 13d013a8ac9..2dd27ce8df7 100644
--- a/dbms/src/Formats/ValuesRowInputStream.cpp
+++ b/dbms/src/Formats/ValuesRowInputStream.cpp
@@ -2,7 +2,6 @@
 #include <Interpreters/evaluateConstantExpression.h>
 #include <Interpreters/Context.h>
 #include <Interpreters/convertFieldToType.h>
-#include <DataTypes/DataTypeArray.h>
 #include <Parsers/TokenIterator.h>
 #include <Parsers/ExpressionListParsers.h>
 #include <Formats/ValuesRowInputStream.h>
@@ -30,20 +29,6 @@ namespace ErrorCodes
 }
 
 
-bool is_array_type_compatible(const DataTypeArray & type, const Field & value)
-{
-    if (type.getNestedType()->isNullable())
-        return true;
-
-    const Array & array = DB::get<const Array &>(value);
-    size_t size = array.size();
-    for (size_t i = 0; i < size; ++i)
-        if (array[i].isNull())
-            return false;
-
-    return true;
-}
-
 ValuesRowInputStream::ValuesRowInputStream(ReadBuffer & istr_, const Block & header_, const Context & context_, const FormatSettings & format_settings)
     : istr(istr_), header(header_), context(std::make_unique<Context>(context_)), format_settings(format_settings)
 {
@@ -131,15 +116,14 @@ bool ValuesRowInputStream::read(MutableColumns & columns)
                 std::pair<Field, DataTypePtr> value_raw = evaluateConstantExpression(ast, *context);
                 Field value = convertFieldToType(value_raw.first, type, value_raw.second.get());
 
-                const auto * array_type = typeid_cast<const DataTypeArray *>(&type);
-
                 /// Check that we are indeed allowed to insert a NULL.
-                if ((value.isNull() && !type.isNullable()) || (array_type && !is_array_type_compatible(*array_type, value)))
+                if (value.isNull())
                 {
-                    throw Exception{"Expression returns value " + applyVisitor(FieldVisitorToString(), value)
-                        + ", that is out of range of type " + type.getName()
-                        + ", at: " + String(prev_istr_position, std::min(SHOW_CHARS_ON_SYNTAX_ERROR, istr.buffer().end() - prev_istr_position)),
-                        ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE};
+                    if (!type.isNullable())
+                        throw Exception{"Expression returns value " + applyVisitor(FieldVisitorToString(), value)
+                            + ", that is out of range of type " + type.getName()
+                            + ", at: " + String(prev_istr_position, std::min(SHOW_CHARS_ON_SYNTAX_ERROR, istr.buffer().end() - prev_istr_position)),
+                            ErrorCodes::VALUE_IS_OUT_OF_RANGE_OF_DATA_TYPE};
                 }
 
                 columns[i]->insert(value);
diff --git a/dbms/src/Interpreters/convertFieldToType.cpp b/dbms/src/Interpreters/convertFieldToType.cpp
index 58b0c164c35..a9d94ddd4f3 100644
--- a/dbms/src/Interpreters/convertFieldToType.cpp
+++ b/dbms/src/Interpreters/convertFieldToType.cpp
@@ -218,7 +218,12 @@ Field convertFieldToTypeImpl(const Field & src, const IDataType & type, const ID
 
             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);
+            }
 
             return res;
         }
diff --git a/dbms/tests/queries/0_stateless/00748_insert_array_with_null.reference b/dbms/tests/queries/0_stateless/00748_insert_array_with_null.reference
new file mode 100644
index 00000000000..1ffffa4aa6b
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/00748_insert_array_with_null.reference
@@ -0,0 +1 @@
+['aaaaa','bbbbb','ccccc']
diff --git a/dbms/tests/queries/0_stateless/00748_insert_array_with_null.sql b/dbms/tests/queries/0_stateless/00748_insert_array_with_null.sql
new file mode 100644
index 00000000000..027f9b6b472
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/00748_insert_array_with_null.sql
@@ -0,0 +1,11 @@
+DROP TABLE IF EXISTS test.arraytest;
+
+CREATE TABLE test.arraytest ( created_date Date DEFAULT toDate(created_at), created_at DateTime DEFAULT now(), strings Array(String) DEFAULT emptyArrayString()) ENGINE = MergeTree(created_date, cityHash64(created_at), (created_date, cityHash64(created_at)), 8192);
+
+INSERT INTO test.arraytest (created_at, strings) VALUES (now(), ['aaaaa', 'bbbbb', 'ccccc']);
+INSERT INTO test.arraytest (created_at, strings) VALUES (now(), ['aaaaa', 'bbbbb', null]); -- { clientError 53 }
+
+SELECT strings from test.arraytest;
+
+DROP TABLE IF EXISTS test.arraytest;
+