Merge pull request #12179 from azat/GROUP-BY-injective-elimination-dictGet-fixes

Fix dictGet arguments check during GROUP BY injective functions elimination
This commit is contained in:
alexey-milovidov 2020-07-09 04:29:26 +03:00 committed by GitHub
commit 0c37fe9c75
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 53 additions and 3 deletions

View File

@ -99,6 +99,10 @@ public:
bool isDictGetFunctionInjective(const Block & sample_block)
{
/// Assume non-injective by default
if (!sample_block)
return false;
if (sample_block.columns() != 3 && sample_block.columns() != 4)
throw Exception{"Function dictGet... takes 3 or 4 arguments", ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH};

View File

@ -133,6 +133,10 @@ public:
* But we assume, that it is injective. This could be documented as implementation-specific behaviour.
*
* sample_block should contain data types of arguments and values of constants, if relevant.
* NOTE: to check is function injective with any arguments, you can pass
* empty block as sample_block (since most of the time function will
* ignore it anyway, and creating arguments just for checking is
* function injective or not is overkill).
*/
virtual bool isInjective(const Block & /*sample_block*/) const { return false; }

View File

@ -248,6 +248,7 @@ void executeScalarSubqueries(ASTPtr & query, const Context & context, size_t sub
const std::unordered_set<String> possibly_injective_function_names
{
"dictGet",
"dictGetString",
"dictGetUInt8",
"dictGetUInt16",
@ -327,10 +328,18 @@ void optimizeGroupBy(ASTSelectQuery * select_query, const NameSet & source_colum
continue;
}
const auto & dict_name = function->arguments->children[0]->as<ASTLiteral &>().value.safeGet<String>();
const auto & dict_ptr = context.getExternalDictionariesLoader().getDictionary(dict_name);
const auto & attr_name = function->arguments->children[1]->as<ASTLiteral &>().value.safeGet<String>();
const auto * dict_name_ast = function->arguments->children[0]->as<ASTLiteral>();
const auto * attr_name_ast = function->arguments->children[1]->as<ASTLiteral>();
if (!dict_name_ast || !attr_name_ast)
{
++i;
continue;
}
const auto & dict_name = dict_name_ast->value.safeGet<String>();
const auto & attr_name = attr_name_ast->value.safeGet<String>();
const auto & dict_ptr = context.getExternalDictionariesLoader().getDictionary(dict_name);
if (!dict_ptr->isInjective(attr_name))
{
++i;

View File

@ -0,0 +1 @@
SELECT dictGetString(concat('default', '.countryId'), 'country', toUInt64(number)) AS country FROM numbers(2) GROUP BY country; -- { serverError 36; }

View File

@ -0,0 +1,31 @@
-- https://github.com/ClickHouse/ClickHouse/issues/11469
SELECT dictGet('default.countryId', 'country', toUInt64(number)) AS country FROM numbers(2) GROUP BY country; -- { serverError 36; }
-- with real dictionary
DROP TABLE IF EXISTS dictdb_01376.table_for_dict;
DROP DICTIONARY IF EXISTS dictdb_01376.dict_exists;
DROP DATABASE IF EXISTS dictdb_01376;
CREATE DATABASE dictdb_01376 ENGINE = Ordinary;
CREATE TABLE dictdb_01376.table_for_dict
(
key_column UInt64,
value Float64
)
ENGINE = Memory();
INSERT INTO dictdb_01376.table_for_dict VALUES (1, 1.1);
CREATE DICTIONARY IF NOT EXISTS dictdb_01376.dict_exists
(
key_column UInt64,
value Float64 DEFAULT 77.77
)
PRIMARY KEY key_column
SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'table_for_dict' DB 'dictdb_01376'))
LIFETIME(1)
LAYOUT(FLAT());
SELECT dictGet('dictdb_01376.dict_exists', 'value', toUInt64(1)) as val FROM numbers(2) GROUP BY val;