From 8fbef6c69dda57b9eee814364f1eda88a9f3e5d1 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Thu, 2 Sep 2021 18:30:55 +0300 Subject: [PATCH 1/2] Function dictGet default implementation for nulls --- src/Functions/FunctionsExternalDictionaries.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Functions/FunctionsExternalDictionaries.h b/src/Functions/FunctionsExternalDictionaries.h index 5f94a1e1f4b..14a6430723c 100644 --- a/src/Functions/FunctionsExternalDictionaries.h +++ b/src/Functions/FunctionsExternalDictionaries.h @@ -293,7 +293,6 @@ public: size_t getNumberOfArguments() const override { return 0; } bool useDefaultImplementationForConstants() const final { return true; } - bool useDefaultImplementationForNulls() const final { return false; } ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0, 1}; } bool isDeterministic() const override { return false; } From c329f04b2298838e1077c1331109e0922e55a1a7 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Fri, 3 Sep 2021 23:08:55 +0300 Subject: [PATCH 2/2] Fixed tests --- src/Functions/FunctionsExternalDictionaries.h | 66 ++++++++++++++++--- .../2014_dict_get_nullable_key.reference | 13 ++++ .../2014_dict_get_nullable_key.sql | 29 ++++++++ 3 files changed, 99 insertions(+), 9 deletions(-) create mode 100644 tests/queries/0_stateless/2014_dict_get_nullable_key.reference create mode 100644 tests/queries/0_stateless/2014_dict_get_nullable_key.sql diff --git a/src/Functions/FunctionsExternalDictionaries.h b/src/Functions/FunctionsExternalDictionaries.h index 14a6430723c..4f79b06b44a 100644 --- a/src/Functions/FunctionsExternalDictionaries.h +++ b/src/Functions/FunctionsExternalDictionaries.h @@ -293,6 +293,7 @@ public: size_t getNumberOfArguments() const override { return 0; } bool useDefaultImplementationForConstants() const final { return true; } + bool useDefaultImplementationForNulls() const final { return false; } ColumnNumbers getArgumentsThatAreAlwaysConstant() const final { return {0, 1}; } bool isDeterministic() const override { return false; } @@ -320,21 +321,32 @@ public: Strings attribute_names = getAttributeNamesFromColumn(arguments[1].column, arguments[1].type); - DataTypes types; - auto dictionary_structure = helper.getDictionaryStructure(dictionary_name); + DataTypes attribute_types; + attribute_types.reserve(attribute_names.size()); for (auto & attribute_name : attribute_names) { /// We're extracting the return type from the dictionary's config, without loading the dictionary. - auto attribute = dictionary_structure.getAttribute(attribute_name); - types.emplace_back(attribute.type); + const auto & attribute = dictionary_structure.getAttribute(attribute_name); + attribute_types.emplace_back(attribute.type); } - if (types.size() > 1) - return std::make_shared(types, attribute_names); + bool key_is_nullable = arguments[2].type->isNullable(); + if (attribute_types.size() > 1) + { + if (key_is_nullable) + throw Exception(ErrorCodes::UNSUPPORTED_METHOD, "Function {} support nullable key only for single dictionary attribute", getName()); + + return std::make_shared(attribute_types, attribute_names); + } else - return types.front(); + { + if (key_is_nullable) + return makeNullable(attribute_types.front()); + else + return attribute_types.front(); + } } ColumnPtr executeImpl(const ColumnsWithTypeAndName & arguments, const DataTypePtr & result_type, size_t input_rows_count) const override @@ -417,7 +429,9 @@ public: default_cols = tuple_column->getColumnsCopy(); } else + { default_cols.emplace_back(result); + } } else { @@ -425,7 +439,16 @@ public: default_cols.emplace_back(nullptr); } - const auto & key_col_with_type = arguments[2]; + auto key_col_with_type = arguments[2]; + + bool key_is_only_null = key_col_with_type.type->onlyNull(); + if (key_is_only_null) + return result_type->createColumnConstWithDefaultValue(input_rows_count); + + bool key_is_nullable = key_col_with_type.type->isNullable(); + if (key_is_nullable) + key_col_with_type = columnGetNested(key_col_with_type); + auto key_column = key_col_with_type.column; Columns key_columns; @@ -481,7 +504,26 @@ public: key_types.emplace_back(range_col_type); } - return executeDictionaryRequest(dictionary, attribute_names, key_columns, key_types, result_type, default_cols); + DataTypePtr attribute_type = result_type; + if (key_is_nullable) + { + DataTypes attribute_types; + attribute_types.reserve(attribute_names.size()); + for (auto & attribute_name : attribute_names) + { + const auto & attribute = dictionary->getStructure().getAttribute(attribute_name); + attribute_types.emplace_back(attribute.type); + } + + attribute_type = attribute_types.front(); + } + + auto result_column = executeDictionaryRequest(dictionary, attribute_names, key_columns, key_types, attribute_type, default_cols); + + if (key_is_nullable) + result_column = wrapInNullable(result_column, {arguments[2]}, result_type, input_rows_count); + + return result_column; } private: @@ -510,12 +552,14 @@ private: result = ColumnTuple::create(std::move(result_columns)); } else + { result = dictionary->getColumn( attribute_names[0], result_type, key_columns, key_types, default_cols.front()); + } return result; } @@ -525,7 +569,9 @@ private: Strings attribute_names; if (const auto * name_col = checkAndGetColumnConst(column.get())) + { attribute_names.emplace_back(name_col->getValue()); + } else if (const auto * tuple_col_const = checkAndGetColumnConst(column.get())) { const ColumnTuple & tuple_col = assert_cast(tuple_col_const->getDataColumn()); @@ -550,10 +596,12 @@ private: } } else + { throw Exception(ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, "Illegal type {} of second argument of function {}, expected a const string or const tuple of const strings.", type->getName(), getName()); + } return attribute_names; } diff --git a/tests/queries/0_stateless/2014_dict_get_nullable_key.reference b/tests/queries/0_stateless/2014_dict_get_nullable_key.reference new file mode 100644 index 00000000000..08127d35829 --- /dev/null +++ b/tests/queries/0_stateless/2014_dict_get_nullable_key.reference @@ -0,0 +1,13 @@ +Non nullable value only null key +\N +Non nullable value nullable key +Test +\N + +Nullable value only null key +\N +Nullable value nullable key +Test +\N +\N +\N diff --git a/tests/queries/0_stateless/2014_dict_get_nullable_key.sql b/tests/queries/0_stateless/2014_dict_get_nullable_key.sql new file mode 100644 index 00000000000..d6c058b285f --- /dev/null +++ b/tests/queries/0_stateless/2014_dict_get_nullable_key.sql @@ -0,0 +1,29 @@ +DROP TABLE IF EXISTS dictionary_non_nullable_source_table; +CREATE TABLE dictionary_non_nullable_source_table (id UInt64, value String) ENGINE=TinyLog; +INSERT INTO dictionary_non_nullable_source_table VALUES (0, 'Test'); + +DROP DICTIONARY IF EXISTS test_dictionary_non_nullable; +CREATE DICTIONARY test_dictionary_non_nullable (id UInt64, value String) PRIMARY KEY id LAYOUT(DIRECT()) SOURCE(CLICKHOUSE(TABLE 'dictionary_non_nullable_source_table')); + +SELECT 'Non nullable value only null key '; +SELECT dictGet('test_dictionary_non_nullable', 'value', NULL); +SELECT 'Non nullable value nullable key'; +SELECT dictGet('test_dictionary_non_nullable', 'value', arrayJoin([toUInt64(0), NULL, 1])); + +DROP DICTIONARY test_dictionary_non_nullable; +DROP TABLE dictionary_non_nullable_source_table; + +DROP TABLE IF EXISTS dictionary_nullable_source_table; +CREATE TABLE dictionary_nullable_source_table (id UInt64, value Nullable(String)) ENGINE=TinyLog; +INSERT INTO dictionary_nullable_source_table VALUES (0, 'Test'), (1, NULL); + +DROP DICTIONARY IF EXISTS test_dictionary_nullable; +CREATE DICTIONARY test_dictionary_nullable (id UInt64, value Nullable(String)) PRIMARY KEY id LAYOUT(DIRECT()) SOURCE(CLICKHOUSE(TABLE 'dictionary_nullable_source_table')); + +SELECT 'Nullable value only null key '; +SELECT dictGet('test_dictionary_nullable', 'value', NULL); +SELECT 'Nullable value nullable key'; +SELECT dictGet('test_dictionary_nullable', 'value', arrayJoin([toUInt64(0), NULL, 1, 2])); + +DROP DICTIONARY test_dictionary_nullable; +DROP TABLE dictionary_nullable_source_table;