From e89a3b5d098f63327a2fb521370d27294844ed6a Mon Sep 17 00:00:00 2001 From: Mike Date: Fri, 16 Oct 2020 12:08:44 +0300 Subject: [PATCH] Fixing arrayIndex functions when right operand is LC but left is not (#16038) --- src/Functions/array/arrayIndex.h | 30 ++++++++++++------- ...1441_low_cardinality_array_index.reference | 2 ++ .../01441_low_cardinality_array_index.sql | 6 ++++ 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/Functions/array/arrayIndex.h b/src/Functions/array/arrayIndex.h index d0cce3d0d58..738899b4b8c 100644 --- a/src/Functions/array/arrayIndex.h +++ b/src/Functions/array/arrayIndex.h @@ -384,7 +384,7 @@ public: /** * If one or both arguments passed to this function are nullable, - * we create a new columns that contains non-nullable arguments: + * we create a new column that contains non-nullable arguments: * * - if the 1st argument is a non-constant array of nullable values, * it is turned into a non-constant array of ordinary values + a null @@ -497,7 +497,7 @@ private: NullMaps maps; ResultColumnPtr result { ResultColumnType::create() }; - inline void move_result() { columns[result_pos].column = std::move(result); } + inline void moveResult() { columns[result_pos].column = std::move(result); } }; static inline bool allowNested(const DataTypePtr & left, const DataTypePtr & right) @@ -524,9 +524,11 @@ private: const DataTypePtr array_nullable_nested = checkAndGetDataType(array_inner_type.get())->getNestedType(); + // We also allow Nullable(T) and LC(U) if the Nullable(T) and U are allowed, + // the LC(U) will be converted to U. return allowNested( array_nullable_nested, - arg_or_arg_nullable_nested); + recursiveRemoveLowCardinality(arg_or_arg_nullable_nested)); } else if (arg_is_nullable) // cannot compare Array(T) elem (namely, T) and Nullable(T) return false; @@ -574,6 +576,9 @@ private: return allowNested(array_lc_nested_or_lc_nullable_nested, arg); } + if (arg_is_lc) // Allow T and LC(U) if U and T are allowed (the low cardinality column will be converted). + return allowNested(array_inner_type, arg_lc_inner_type); + return false; } @@ -644,7 +649,8 @@ private: if (!left) return false; - const IColumn& right = *columns[arguments[1]].column.get(); + const ColumnPtr right_converted_ptr = columns[arguments[1]].column->convertToFullColumnIfLowCardinality(); + const IColumn& right = *right_converted_ptr.get(); ExecutionData data = { left->getData(), @@ -710,7 +716,7 @@ private: else return false; - data.move_result(); + data.moveResult(); return true; } @@ -863,7 +869,7 @@ private: data.result->getData(), data.maps.first, data.maps.second); - data.move_result(); + data.moveResult(); return true; } @@ -881,7 +887,8 @@ private: if (!left) return false; - const IColumn & right = *columns[arguments[1]].column.get(); + const ColumnPtr right_ptr = columns[arguments[1]].column->convertToFullColumnIfLowCardinality(); + const IColumn & right = *right_ptr.get(); ExecutionData data = { *left, right, array->getOffsets(), @@ -950,7 +957,7 @@ private: else return false; - data.move_result(); + data.moveResult(); return true; } @@ -964,7 +971,8 @@ private: Array arr = col_array->getValue(); - const IColumn * item_arg = columns[arguments[1]].column.get(); + const ColumnPtr right_ptr = columns[arguments[1]].column->convertToFullColumnIfLowCardinality(); + const IColumn * item_arg = right_ptr.get(); if (isColumnConst(*item_arg)) { @@ -1039,7 +1047,9 @@ private: return false; const IColumn & col_nested = col->getData(); - const IColumn & item_arg = *columns[arguments[1]].column; + + const ColumnPtr right_ptr = columns[arguments[1]].column->convertToFullColumnIfLowCardinality(); + const IColumn & item_arg = *right_ptr.get(); auto col_res = ResultColumnType::create(); diff --git a/tests/queries/0_stateless/01441_low_cardinality_array_index.reference b/tests/queries/0_stateless/01441_low_cardinality_array_index.reference index 4e68594ce22..4e01b47292e 100644 --- a/tests/queries/0_stateless/01441_low_cardinality_array_index.reference +++ b/tests/queries/0_stateless/01441_low_cardinality_array_index.reference @@ -13,3 +13,5 @@ 1000000 1000000 1000000 +1 +1 diff --git a/tests/queries/0_stateless/01441_low_cardinality_array_index.sql b/tests/queries/0_stateless/01441_low_cardinality_array_index.sql index 45d69b5aa64..8febe8f2e44 100644 --- a/tests/queries/0_stateless/01441_low_cardinality_array_index.sql +++ b/tests/queries/0_stateless/01441_low_cardinality_array_index.sql @@ -38,4 +38,10 @@ SELECT count() FROM t_01411_num WHERE has(arr, num); SELECT count() FROM t_01411_num WHERE indexOf(arr, num) > 0; SELECT count() FROM t_01411_num WHERE indexOf(arr, num % 337) > 0; +-- Checking Arr(String) and LC(String) +SELECT indexOf(['a', 'b', 'c'], toLowCardinality('a')); + +-- Checking Arr(Nullable(String)) and LC(String) +SELECT indexOf(['a', 'b', NULL], toLowCardinality('a')); + DROP TABLE IF EXISTS t_01411_num;