Merge pull request ClickHouse#11934 from ClickHouse/fix_some_logical_errors

tavplubix · web-flow · commit 0260358253c5 · 2020-06-25T14:20:58.000+03:00
Fix some logical errors
diff --git a/src/Functions/hasColumnInTable.cpp b/src/Functions/hasColumnInTable.cpp
@@ -17,6 +17,7 @@ namespace ErrorCodes
 {
     extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
     extern const int ILLEGAL_TYPE_OF_ARGUMENT;
+    extern const int UNKNOWN_TABLE;
 }
 
 
@@ -110,6 +111,9 @@ void FunctionHasColumnInTable::executeImpl(Block & block, const ColumnNumbers &
     String table_name = get_string_from_block(arguments[arg++]);
     String column_name = get_string_from_block(arguments[arg++]);
 
+    if (table_name.empty())
+        throw Exception("Table name is empty", ErrorCodes::UNKNOWN_TABLE);
+
     bool has_column;
     if (host_name.empty())
     {
diff --git a/src/Interpreters/convertFieldToType.cpp b/src/Interpreters/convertFieldToType.cpp
@@ -329,5 +329,16 @@ Field convertFieldToType(const Field & from_value, const IDataType & to_type, co
         return convertFieldToTypeImpl(from_value, to_type, from_type_hint);
 }
 
+Field convertFieldToTypeOrThrow(const Field & from_value, const IDataType & to_type, const IDataType * from_type_hint)
+{
+    bool is_null = from_value.isNull();
+    if (is_null && !to_type.isNullable())
+        throw Exception(ErrorCodes::TYPE_MISMATCH, "Cannot convert NULL to {}", to_type.getName());
+    Field converted = convertFieldToType(from_value, to_type, from_type_hint);
+    if (!is_null && converted.isNull())
+        throw Exception(ErrorCodes::ARGUMENT_OUT_OF_BOUND, "Cannot convert value{}: it cannot be represented as {}",
+                        from_type_hint ? " from " + from_type_hint->getName() : "", to_type.getName());
+    return converted;
+}
 
 }
diff --git a/src/Interpreters/convertFieldToType.h b/src/Interpreters/convertFieldToType.h
@@ -17,4 +17,7 @@ class IDataType;
   */
 Field convertFieldToType(const Field & from_value, const IDataType & to_type, const IDataType * from_type_hint = nullptr);
 
+/// Does the same, but throws ARGUMENT_OUT_OF_BOUND if value does not fall into the range.
+Field convertFieldToTypeOrThrow(const Field & from_value, const IDataType & to_type, const IDataType * from_type_hint = nullptr);
+
 }
diff --git a/src/TableFunctions/TableFunctionValues.cpp b/src/TableFunctions/TableFunctionValues.cpp
@@ -38,7 +38,7 @@ static void parseAndInsertValues(MutableColumns & res_columns, const ASTs & args
         {
             const auto & [value_field, value_type_ptr] = evaluateConstantExpression(args[i], context);
 
-            Field value = convertFieldToType(value_field, *sample_block.getByPosition(0).type, value_type_ptr.get());
+            Field value = convertFieldToTypeOrThrow(value_field, *sample_block.getByPosition(0).type, value_type_ptr.get());
             res_columns[0]->insert(value);
         }
     }
@@ -51,11 +51,11 @@ static void parseAndInsertValues(MutableColumns & res_columns, const ASTs & args
             const Tuple & value_tuple = value_field.safeGet<Tuple>();
 
             if (value_tuple.size() != sample_block.columns())
-                throw Exception("Values size should match with number of columns", ErrorCodes::LOGICAL_ERROR);
+                throw Exception("Values size should match with number of columns", ErrorCodes::BAD_ARGUMENTS);
 
             for (size_t j = 0; j < value_tuple.size(); ++j)
             {
-                Field value = convertFieldToType(value_tuple[j], *sample_block.getByPosition(j).type, value_types_tuple[j].get());
+                Field value = convertFieldToTypeOrThrow(value_tuple[j], *sample_block.getByPosition(j).type, value_types_tuple[j].get());
                 res_columns[j]->insert(value);
             }
         }
diff --git a/tests/queries/0_stateless/00386_has_column_in_table.reference b/tests/queries/0_stateless/00386_has_column_in_table.reference
@@ -12,3 +12,4 @@
 0
 0
 0
+0
diff --git a/tests/queries/0_stateless/00386_has_column_in_table.sql b/tests/queries/0_stateless/00386_has_column_in_table.sql
@@ -18,5 +18,14 @@ SELECT hasColumnInTable(currentDatabase(), 'has_column_in_table', 'nest.not_exis
 SELECT hasColumnInTable('localhost', currentDatabase(), 'has_column_in_table', 'nest.not_existing');
 SELECT hasColumnInTable(currentDatabase(), 'has_column_in_table', 'not_existing');
 SELECT hasColumnInTable('localhost', currentDatabase(), 'has_column_in_table', 'not_existing');
+SELECT hasColumnInTable('system', 'one', '');
+
+/* bad queries */
+SELECT hasColumnInTable('', '', '');  -- { serverError 60; }
+SELECT hasColumnInTable('', 't', 'c');  -- { serverError 81; }
+SELECT hasColumnInTable(currentDatabase(), '', 'c'); -- { serverError 60; }
+SELECT hasColumnInTable('d', 't', 's');  -- { serverError 81; }
+SELECT hasColumnInTable(currentDatabase(), 't', 's');  -- { serverError 60; }
+
 
 DROP TABLE has_column_in_table;
diff --git a/tests/queries/0_stateless/00975_values_list.reference b/tests/queries/0_stateless/00975_values_list.reference
@@ -11,3 +11,4 @@ abracadabra
 23	23	23
 24	24	24
 1.6660	a	b
+\N
diff --git a/tests/queries/0_stateless/00975_values_list.sql b/tests/queries/0_stateless/00975_values_list.sql
@@ -11,4 +11,9 @@ SELECT * FROM VALUES('s String', ('abra'), ('cadabra'), ('abracadabra'));
 SELECT * FROM VALUES('n UInt64, s String, ss String', (1 + 22, '23', toString(23)), (toUInt64('24'), '24', concat('2', '4')));
 
 SELECT * FROM VALUES('a Decimal(4, 4), b String, c String', (divide(toDecimal32(5, 3), 3), 'a', 'b'));
+
+SELECT * FROM VALUES('x Float64', toUInt64(-1)); -- { serverError 69; }
+SELECT * FROM VALUES('x Float64', NULL); -- { serverError 53; }
+SELECT * FROM VALUES('x Nullable(Float64)', NULL);
+
 DROP TABLE values_list;

Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ namespace ErrorCodes`
`17`	`17`	`{`
`18`	`18`	`extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;`
`19`	`19`	`extern const int ILLEGAL_TYPE_OF_ARGUMENT;`
	`20`	`+ extern const int UNKNOWN_TABLE;`
`20`	`21`	`}`
`21`	`22`
`22`	`23`
`@@ -110,6 +111,9 @@ void FunctionHasColumnInTable::executeImpl(Block & block, const ColumnNumbers &`
`110`	`111`	`String table_name = get_string_from_block(arguments[arg++]);`
`111`	`112`	`String column_name = get_string_from_block(arguments[arg++]);`
`112`	`113`
	`114`	`+ if (table_name.empty())`
	`115`	`+ throw Exception("Table name is empty", ErrorCodes::UNKNOWN_TABLE);`
	`116`	`+`
`113`	`117`	`bool has_column;`
`114`	`118`	`if (host_name.empty())`
`115`	`119`	`{`
Original file line number	Diff line number	Diff line change
`@@ -17,4 +17,7 @@ class IDataType;`
`17`	`17`	`*/`
`18`	`18`	`Field convertFieldToType(const Field & from_value, const IDataType & to_type, const IDataType * from_type_hint = nullptr);`
`19`	`19`
	`20`	`+/// Does the same, but throws ARGUMENT_OUT_OF_BOUND if value does not fall into the range.`
	`21`	`+Field convertFieldToTypeOrThrow(const Field & from_value, const IDataType & to_type, const IDataType * from_type_hint = nullptr);`
	`22`	`+`
`20`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ static void parseAndInsertValues(MutableColumns & res_columns, const ASTs & args`
`38`	`38`	`{`
`39`	`39`	`const auto & [value_field, value_type_ptr] = evaluateConstantExpression(args[i], context);`
`40`	`40`
`41`		`- Field value = convertFieldToType(value_field, *sample_block.getByPosition(0).type, value_type_ptr.get());`
	`41`	`+ Field value = convertFieldToTypeOrThrow(value_field, *sample_block.getByPosition(0).type, value_type_ptr.get());`
`42`	`42`	`res_columns[0]->insert(value);`
`43`	`43`	`}`
`44`	`44`	`}`
`@@ -51,11 +51,11 @@ static void parseAndInsertValues(MutableColumns & res_columns, const ASTs & args`
`51`	`51`	`const Tuple & value_tuple = value_field.safeGet<Tuple>();`
`52`	`52`
`53`	`53`	`if (value_tuple.size() != sample_block.columns())`
`54`		`- throw Exception("Values size should match with number of columns", ErrorCodes::LOGICAL_ERROR);`
	`54`	`+ throw Exception("Values size should match with number of columns", ErrorCodes::BAD_ARGUMENTS);`
`55`	`55`
`56`	`56`	`for (size_t j = 0; j < value_tuple.size(); ++j)`
`57`	`57`	`{`
`58`		`- Field value = convertFieldToType(value_tuple[j], *sample_block.getByPosition(j).type, value_types_tuple[j].get());`
	`58`	`+ Field value = convertFieldToTypeOrThrow(value_tuple[j], *sample_block.getByPosition(j).type, value_types_tuple[j].get());`
`59`	`59`	`res_columns[j]->insert(value);`
`60`	`60`	`}`
`61`	`61`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,3 +12,4 @@`
`12`	`12`	`0`
`13`	`13`	`0`
`14`	`14`	`0`
	`15`	`+0`
-Original file line number
+Diff line change
 	23	23
 	24	24
 .6660	a	b
 +\N