From ef135c58733554c5f746319f80c00c03f886a5ef Mon Sep 17 00:00:00 2001 From: Alexey Arno Date: Tue, 15 Dec 2015 12:56:14 +0300 Subject: [PATCH 1/3] dbms: Server: Added more explanatory error messages for date-time-related functions. [#METR-19416] --- dbms/include/DB/Functions/FunctionsDateTime.h | 52 ++++++++++--------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/dbms/include/DB/Functions/FunctionsDateTime.h b/dbms/include/DB/Functions/FunctionsDateTime.h index 5faecf01943..0e1719c3cee 100644 --- a/dbms/include/DB/Functions/FunctionsDateTime.h +++ b/dbms/include/DB/Functions/FunctionsDateTime.h @@ -582,35 +582,39 @@ public: /// Получить тип результата по типам аргументов. Если функция неприменима для данных аргументов - кинуть исключение. DataTypePtr getReturnType(const DataTypes & arguments) const override { - if ((arguments.size() < 1) || (arguments.size() > 2)) - throw Exception("Number of arguments for function " + getName() + " doesn't match: passed " - + toString(arguments.size()) + ", should be 1 or 2.", - ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); - - if (typeid_cast(&*arguments[0]) != nullptr) + if (arguments.size() == 1) { - if (arguments.size() > 1) - throw Exception("Number of arguments for function " + getName() + " doesn't match: passed " - + toString(arguments.size()) + ", should be 1.", - ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); + if ((typeid_cast(&*arguments[0]) == nullptr) && + (typeid_cast(&*arguments[0]) == nullptr)) + throw Exception{ + "Illegal type " + arguments[0]->getName() + " of argument of function " + getName() + + ". Should be a date or a date with time", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT + }; } - else if (typeid_cast(&*arguments[0]) != nullptr) + else if (arguments.size() == 2) { - /// Ничего не делаем. + std::string error_msg; + + if (typeid_cast(&*arguments[0]) == nullptr) + error_msg += "Illegal type " + arguments[0]->getName() + " of argument 1." + " Should be a date with time (timezones are not supported for dates)"; + if (typeid_cast(&*arguments[1]) == nullptr) + { + if (!error_msg.empty()) + error_msg += ". "; + error_msg += "Illegal type " + arguments[1]->getName() + " of argument 2." + " Should be a string describing a timezone"; + } + + if (!error_msg.empty()) + throw Exception{ + "In function " + getName() + ": " + error_msg, ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT + }; } else - throw Exception{ - "Illegal type " + arguments[0]->getName() + " of argument 1 of function " + getName(), - ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT - }; - - if ((arguments.size() == 2) && (typeid_cast(&*arguments[1]) == nullptr)) - { - throw Exception{ - "Illegal type " + arguments[1]->getName() + " of argument 2 of function " + getName(), - ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT - }; - } + throw Exception("Number of arguments for function " + getName() + " doesn't match: passed " + + toString(arguments.size()) + ", should be 1 or 2", + ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH); return new ToDataType; } From 3a9e7a7170ff8f4489dcb49a4b394caa20e60c7f Mon Sep 17 00:00:00 2001 From: Alexey Arno Date: Tue, 15 Dec 2015 13:13:07 +0300 Subject: [PATCH 2/3] dbms: Server: Added more explanatory error messages for date-time-related functions. [#METR-19416] --- dbms/include/DB/Functions/FunctionsDateTime.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dbms/include/DB/Functions/FunctionsDateTime.h b/dbms/include/DB/Functions/FunctionsDateTime.h index 0e1719c3cee..1a4bed299a0 100644 --- a/dbms/include/DB/Functions/FunctionsDateTime.h +++ b/dbms/include/DB/Functions/FunctionsDateTime.h @@ -595,9 +595,17 @@ public: { std::string error_msg; - if (typeid_cast(&*arguments[0]) == nullptr) + if (typeid_cast(&*arguments[0]) != nullptr) error_msg += "Illegal type " + arguments[0]->getName() + " of argument 1." " Should be a date with time (timezones are not supported for dates)"; + else if (typeid_cast(&*arguments[0]) != nullptr) + { + /// Ничего не делаем. + } + else + error_msg += "Illegal type " + arguments[0]->getName() + " of argument 1." + " Should be a date with time"; + if (typeid_cast(&*arguments[1]) == nullptr) { if (!error_msg.empty()) From 9d16702f840b75ae4c83aec7b0b10ccb10d8fac1 Mon Sep 17 00:00:00 2001 From: Andrey Mironov Date: Tue, 15 Dec 2015 17:13:30 +0300 Subject: [PATCH 3/3] dbms: unconditionally move evaluation of storage ALIASes to InterpterSelectQuery [#METR-19317] --- .../DB/Interpreters/ExpressionAnalyzer.h | 5 ----- dbms/src/Interpreters/ExpressionAnalyzer.cpp | 21 +------------------ .../Interpreters/InterpreterSelectQuery.cpp | 8 ++++++- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/dbms/include/DB/Interpreters/ExpressionAnalyzer.h b/dbms/include/DB/Interpreters/ExpressionAnalyzer.h index b1b221a0f4e..3d3eba9acb6 100644 --- a/dbms/include/DB/Interpreters/ExpressionAnalyzer.h +++ b/dbms/include/DB/Interpreters/ExpressionAnalyzer.h @@ -208,11 +208,6 @@ private: */ void collectJoinedColumns(NameSet & joined_columns, NamesAndTypesList & joined_columns_name_type); - /** Добавляет ALIAS столбцы из storage в aliases, если запрос не является SELECT с ARRAY JOIN. При наличии - * ARRAY JOIN их добавлять нельзя, иначе ломается логика его выполнения. - */ - void addStorageAliases(); - /** Создать словарь алиасов. */ void addASTAliases(ASTPtr & ast, int ignore_levels = 0); diff --git a/dbms/src/Interpreters/ExpressionAnalyzer.cpp b/dbms/src/Interpreters/ExpressionAnalyzer.cpp index 537d0328b78..d4a23103d83 100644 --- a/dbms/src/Interpreters/ExpressionAnalyzer.cpp +++ b/dbms/src/Interpreters/ExpressionAnalyzer.cpp @@ -120,9 +120,6 @@ void ExpressionAnalyzer::init() /// Создаёт словарь aliases: alias -> ASTPtr addASTAliases(ast); - /// Добавляет ALIAS столбцы из таблицы в aliases, если применимо. - addStorageAliases(); - /// Common subexpression elimination. Rewrite rules. normalizeTree(); @@ -428,22 +425,6 @@ NamesAndTypesList::iterator ExpressionAnalyzer::findColumn(const String & name, } -void ExpressionAnalyzer::addStorageAliases() -{ - if (select_query && select_query->array_join_expression_list) - return; - - if (!storage) - return; - - /// @todo: consider storing default expressions with alias set to avoid cloning - /// Добавляем ALIAS из таблицы, только если такого ALIAS еще не объявлено в запросе. - for (const auto & alias : storage->alias_columns) - if (!aliases.count(alias.name)) - aliases[alias.name] = setAlias(storage->column_defaults[alias.name].expression->clone(), alias.name); -} - - /// ignore_levels - алиасы в скольки верхних уровнях поддерева нужно игнорировать. /// Например, при ignore_levels=1 ast не может быть занесен в словарь, но его дети могут. void ExpressionAnalyzer::addASTAliases(ASTPtr & ast, int ignore_levels) @@ -686,7 +667,7 @@ void ExpressionAnalyzer::normalizeTreeImpl( void ExpressionAnalyzer::addAliasColumns() { - if (!(select_query && select_query->array_join_expression_list)) + if (!select_query) return; if (!storage) diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index 556e149a9c4..5335dd4706d 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -668,7 +668,13 @@ QueryProcessingStage::Enum InterpreterSelectQuery::executeFetchColumns() ASTPtr required_columns_expr_list{new ASTExpressionList}; for (const auto & column : required_columns) - required_columns_expr_list->children.emplace_back(new ASTIdentifier{{}, column}); + { + const auto default_it = storage->column_defaults.find(column); + if (default_it != std::end(storage->column_defaults) && default_it->second.type == ColumnDefaultType::Alias) + required_columns_expr_list->children.emplace_back(setAlias(default_it->second.expression->clone(), column)); + else + required_columns_expr_list->children.emplace_back(new ASTIdentifier{{}, column}); + } alias_actions = ExpressionAnalyzer{required_columns_expr_list, context, storage, table_column_names}.getActions(true);