From e6e88241fc08ce513ba0495e1c2ef72368f52b01 Mon Sep 17 00:00:00 2001 From: chertus Date: Wed, 16 Oct 2019 17:47:58 +0300 Subject: [PATCH 01/25] SyntaxAnalyzer refactoring: better getTablesWithColumns() --- dbms/src/Interpreters/IdentifierSemantic.cpp | 22 ++++++ dbms/src/Interpreters/IdentifierSemantic.h | 1 + dbms/src/Interpreters/SyntaxAnalyzer.cpp | 69 ++++++++++++------- .../TranslateQualifiedNamesVisitor.cpp | 16 +---- .../TranslateQualifiedNamesVisitor.h | 6 +- 5 files changed, 73 insertions(+), 41 deletions(-) diff --git a/dbms/src/Interpreters/IdentifierSemantic.cpp b/dbms/src/Interpreters/IdentifierSemantic.cpp index 361462c0d1d..6b74dc2d8d1 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.cpp +++ b/dbms/src/Interpreters/IdentifierSemantic.cpp @@ -57,6 +57,28 @@ size_t IdentifierSemantic::getMembership(const ASTIdentifier & identifier) return identifier.semantic->membership; } +bool IdentifierSemantic::trySetMembership(ASTIdentifier & identifier, const std::vector & tables, + size_t & best_table_pos) +{ + best_table_pos = 0; + size_t best_match = 0; + for (size_t i = 0; i < tables.size(); ++i) + if (size_t match = canReferColumnToTable(identifier, tables[i].first)) + if (match > best_match) + { + best_match = match; + best_table_pos = i; + } + + if (best_match) + { + setMembership(identifier, best_table_pos + 1); + return true; + } + + return false; +} + std::pair IdentifierSemantic::extractDatabaseAndTable(const ASTIdentifier & identifier) { if (identifier.name_parts.size() > 2) diff --git a/dbms/src/Interpreters/IdentifierSemantic.h b/dbms/src/Interpreters/IdentifierSemantic.h index b4bf87e7fef..4fde404488c 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.h +++ b/dbms/src/Interpreters/IdentifierSemantic.h @@ -35,6 +35,7 @@ struct IdentifierSemantic static bool canBeAlias(const ASTIdentifier & identifier); static void setMembership(ASTIdentifier & identifier, size_t table_no); static size_t getMembership(const ASTIdentifier & identifier); + static bool trySetMembership(ASTIdentifier & identifier, const std::vector & tables, size_t & best_table_pos); private: static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & database, const String & table); diff --git a/dbms/src/Interpreters/SyntaxAnalyzer.cpp b/dbms/src/Interpreters/SyntaxAnalyzer.cpp index 228aea0b2f2..3d43b7f0b25 100644 --- a/dbms/src/Interpreters/SyntaxAnalyzer.cpp +++ b/dbms/src/Interpreters/SyntaxAnalyzer.cpp @@ -91,7 +91,10 @@ void collectSourceColumns(const ColumnsDescription & columns, NamesAndTypesList } } -std::vector getTablesWithColumns(const ASTSelectQuery & select_query, const Context & context) +std::vector getTablesWithColumns(const ASTSelectQuery & select_query, const Context & context, + const ASTTablesInSelectQueryElement * table_join_node, + NamesAndTypesList & columns_from_joined_table, + std::function get_column_names) { std::vector tables_with_columns = getDatabaseAndTablesWithColumnNames(select_query, context); @@ -104,6 +107,27 @@ std::vector getTablesWithColumns(const ASTSelectQuery & se ErrorCodes::ALIAS_REQUIRED); } + TableWithColumnNames joined_table; + + if (table_join_node) + { + const auto & joined_expression = table_join_node->table_expression->as(); + + columns_from_joined_table = getNamesAndTypeListFromTableExpression(joined_expression, context); + + joined_table.first = DatabaseAndTableWithAlias(joined_expression, context.getCurrentDatabase()); + for (const auto & column : columns_from_joined_table) + joined_table.second.push_back(column.name); + } + + /// If empty make table(s) with list of source and joined columns + if (tables_with_columns.empty()) + { + tables_with_columns.emplace_back(DatabaseAndTableWithAlias{}, get_column_names()); + if (!joined_table.second.empty()) + tables_with_columns.emplace_back(std::move(joined_table)); + } + return tables_with_columns; } @@ -805,41 +829,34 @@ SyntaxAnalyzerResultPtr SyntaxAnalyzer::analyze( if (remove_duplicates) renameDuplicatedColumns(select_query); - if (const ASTTablesInSelectQueryElement * node = select_query->join()) + const ASTTablesInSelectQueryElement * table_join_node = select_query->join(); + if (table_join_node) { if (!settings.any_join_distinct_right_table_keys) - checkJoin(node); + checkJoin(table_join_node); if (settings.enable_optimize_predicate_expression) - replaceJoinedTable(node); - - const auto & joined_expression = node->table_expression->as(); - DatabaseAndTableWithAlias table(joined_expression, context.getCurrentDatabase()); - - result.analyzed_join->columns_from_joined_table = getNamesAndTypeListFromTableExpression(joined_expression, context); - result.analyzed_join->deduplicateAndQualifyColumnNames(source_columns_set, table.getQualifiedNamePrefix()); + replaceJoinedTable(table_join_node); } - auto tables_with_columns = getTablesWithColumns(*select_query, context); - - /// If empty make fake table with list of source and joined columns - if (tables_with_columns.empty()) + auto get_column_names = [&]() -> Names { - Names columns_list; if (storage) - columns_list = storage->getColumns().getOrdinary().getNames(); - else - { - columns_list.reserve(result.source_columns.size()); - for (const auto & column : result.source_columns) - columns_list.emplace_back(column.name); - } + return storage->getColumns().getOrdinary().getNames(); - for (auto & column : result.analyzed_join->getQualifiedColumnsSet()) - columns_list.emplace_back(column); + Names columns; + columns.reserve(result.source_columns.size()); + for (const auto & column : result.source_columns) + columns.push_back(column.name); + return columns; + }; - tables_with_columns.emplace_back(DatabaseAndTableWithAlias{}, std::move(columns_list)); - } + auto tables_with_columns = getTablesWithColumns(*select_query, context, table_join_node, + result.analyzed_join->columns_from_joined_table, get_column_names); + + if (tables_with_columns.size() > 1) + result.analyzed_join->deduplicateAndQualifyColumnNames( + source_columns_set, tables_with_columns[1].first.getQualifiedNamePrefix()); translateQualifiedNames(query, *select_query, source_columns_set, std::move(tables_with_columns)); diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp index 7ae98d3e9c8..e05387b5aa0 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp +++ b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp @@ -62,22 +62,12 @@ void TranslateQualifiedNamesMatcher::visit(ASTIdentifier & identifier, ASTPtr &, if (IdentifierSemantic::getColumnName(identifier)) { size_t best_table_pos = 0; - size_t best_match = 0; - for (size_t i = 0; i < data.tables.size(); ++i) - if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, data.tables[i].first)) - if (match > best_match) - { - best_match = match; - best_table_pos = i; - } - - if (best_match) - IdentifierSemantic::setMembership(identifier, best_table_pos + 1); + IdentifierSemantic::trySetMembership(identifier, data.tables, best_table_pos); /// In case if column from the joined table are in source columns, change it's name to qualified. - if (best_table_pos && data.source_columns.count(identifier.shortName())) + if (best_table_pos && data.hasColumn(identifier.shortName())) IdentifierSemantic::setNeedLongName(identifier, true); - if (!data.tables.empty()) + if (data.hasTable()) IdentifierSemantic::setColumnNormalName(identifier, data.tables[best_table_pos].first); } } diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.h b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.h index 4bf18b59cb9..b3718170dda 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.h +++ b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.h @@ -35,6 +35,10 @@ public: , has_columns(has_columns_) {} + bool hasColumn(const String & name) const { return source_columns.count(name); } + bool hasTable() const { return !tables.empty(); } + bool processAsterisks() const { return hasTable() && has_columns; } + static std::vector tablesOnly(const std::vector & tables) { std::vector tables_with_columns; @@ -44,8 +48,6 @@ public: tables_with_columns.emplace_back(TableWithColumnNames{table, {}}); return tables_with_columns; } - - bool processAsterisks() const { return !tables.empty() && has_columns; } }; static void visit(ASTPtr & ast, Data & data); From 483108f46f7e9557e5df0cdcecbe578bd0b80c98 Mon Sep 17 00:00:00 2001 From: chertus Date: Wed, 16 Oct 2019 20:33:53 +0300 Subject: [PATCH 02/25] column to table matching refactoring --- .../Interpreters/CrossToInnerJoinVisitor.cpp | 55 ++++++------------ .../FindIdentifierBestTableVisitor.cpp | 14 +---- dbms/src/Interpreters/IdentifierSemantic.cpp | 57 +++++++++++++------ dbms/src/Interpreters/IdentifierSemantic.h | 3 +- .../JoinToSubqueryTransformVisitor.cpp | 8 ++- .../TranslateQualifiedNamesVisitor.cpp | 3 +- 6 files changed, 70 insertions(+), 70 deletions(-) diff --git a/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp b/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp index 52cb05d35ea..5ac28bc6dde 100644 --- a/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp +++ b/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp @@ -90,9 +90,12 @@ public: using TypeToVisit = const ASTFunction; CheckExpressionVisitorData(const std::vector & tables_) - : tables(tables_) + : joined_tables(tables_) , ands_only(true) - {} + { + for (auto & joined : joined_tables) + tables.push_back(joined.table); + } void visit(const ASTFunction & node, ASTPtr & ast) { @@ -156,7 +159,8 @@ public: } private: - const std::vector & tables; + const std::vector & joined_tables; + std::vector tables; std::map> asts_to_join_on; bool ands_only; @@ -180,31 +184,16 @@ private: /// @return table position to attach expression to or 0. size_t checkIdentifiers(const ASTIdentifier & left, const ASTIdentifier & right) { - /// {best_match, best_table_pos} - std::pair left_best{0, 0}; - std::pair right_best{0, 0}; + size_t left_table_pos = 0; + bool left_match = IdentifierSemantic::chooseTable(left, tables, left_table_pos); - for (size_t i = 0; i < tables.size(); ++i) + size_t right_table_pos = 0; + bool right_match = IdentifierSemantic::chooseTable(right, tables, right_table_pos); + + if (left_match && right_match && (left_table_pos != right_table_pos)) { - size_t match = IdentifierSemantic::canReferColumnToTable(left, tables[i].table); - if (match > left_best.first) - { - left_best.first = match; - left_best.second = i; - } - - match = IdentifierSemantic::canReferColumnToTable(right, tables[i].table); - if (match > right_best.first) - { - right_best.first = match; - right_best.second = i; - } - } - - if (left_best.first && right_best.first && (left_best.second != right_best.second)) - { - size_t table_pos = std::max(left_best.second, right_best.second); - if (tables[table_pos].canAttachOnExpression()) + size_t table_pos = std::max(left_table_pos, right_table_pos); + if (joined_tables[table_pos].canAttachOnExpression()) return table_pos; } return 0; @@ -212,20 +201,10 @@ private: size_t checkIdentifier(const ASTIdentifier & identifier) { - size_t best_match = 0; size_t best_table_pos = 0; + bool match = IdentifierSemantic::chooseTable(identifier, tables, best_table_pos); - for (size_t i = 0; i < tables.size(); ++i) - { - size_t match = IdentifierSemantic::canReferColumnToTable(identifier, tables[i].table); - if (match > best_match) - { - best_match = match; - best_table_pos = i; - } - } - - if (best_match && tables[best_table_pos].canAttachOnExpression()) + if (match && joined_tables[best_table_pos].canAttachOnExpression()) return best_table_pos; return 0; } diff --git a/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp b/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp index 8173ce3256a..daf9ca57fb9 100644 --- a/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp +++ b/dbms/src/Interpreters/FindIdentifierBestTableVisitor.cpp @@ -28,17 +28,9 @@ void FindIdentifierBestTableData::visit(ASTIdentifier & identifier, ASTPtr &) } else { - // FIXME: make a better matcher using `names`? - size_t best_match = 0; - for (const auto & table_names : tables) - { - if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, table_names.first)) - if (match > best_match) - { - best_match = match; - best_table = &table_names.first; - } - } + size_t best_table_pos = 0; + if (IdentifierSemantic::chooseTable(identifier, tables, best_table_pos)) + best_table = &tables[best_table_pos].first; } identifier_table.emplace_back(&identifier, best_table); diff --git a/dbms/src/Interpreters/IdentifierSemantic.cpp b/dbms/src/Interpreters/IdentifierSemantic.cpp index 6b74dc2d8d1..f5006159aa9 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.cpp +++ b/dbms/src/Interpreters/IdentifierSemantic.cpp @@ -5,6 +5,37 @@ namespace DB { +namespace +{ + +const DatabaseAndTableWithAlias & extractTable(const DatabaseAndTableWithAlias & table) +{ + return table; +} + +const DatabaseAndTableWithAlias & extractTable(const TableWithColumnNames & table) +{ + return table.first; +} + +template +bool tryChooseTable(const ASTIdentifier & identifier, const std::vector & tables, size_t & best_table_pos) +{ + best_table_pos = 0; + size_t best_match = 0; + for (size_t i = 0; i < tables.size(); ++i) + if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, extractTable(tables[i]))) + if (match > best_match) + { + best_match = match; + best_table_pos = i; + } + + return best_match; +} + +} + std::optional IdentifierSemantic::getColumnName(const ASTIdentifier & node) { if (!node.semantic->special) @@ -57,26 +88,16 @@ size_t IdentifierSemantic::getMembership(const ASTIdentifier & identifier) return identifier.semantic->membership; } -bool IdentifierSemantic::trySetMembership(ASTIdentifier & identifier, const std::vector & tables, - size_t & best_table_pos) +bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, + size_t & best_table_pos) { - best_table_pos = 0; - size_t best_match = 0; - for (size_t i = 0; i < tables.size(); ++i) - if (size_t match = canReferColumnToTable(identifier, tables[i].first)) - if (match > best_match) - { - best_match = match; - best_table_pos = i; - } + return tryChooseTable(identifier, tables, best_table_pos); +} - if (best_match) - { - setMembership(identifier, best_table_pos + 1); - return true; - } - - return false; +bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, + size_t & best_table_pos) +{ + return tryChooseTable(identifier, tables, best_table_pos); } std::pair IdentifierSemantic::extractDatabaseAndTable(const ASTIdentifier & identifier) diff --git a/dbms/src/Interpreters/IdentifierSemantic.h b/dbms/src/Interpreters/IdentifierSemantic.h index 4fde404488c..7403f5d2340 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.h +++ b/dbms/src/Interpreters/IdentifierSemantic.h @@ -35,7 +35,8 @@ struct IdentifierSemantic static bool canBeAlias(const ASTIdentifier & identifier); static void setMembership(ASTIdentifier & identifier, size_t table_no); static size_t getMembership(const ASTIdentifier & identifier); - static bool trySetMembership(ASTIdentifier & identifier, const std::vector & tables, size_t & best_table_pos); + static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos); + static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos); private: static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & database, const String & table); diff --git a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp index b60e6533921..f807bfb7acb 100644 --- a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp +++ b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp @@ -163,7 +163,13 @@ struct ColumnAliasesMatcher auto it = rev_aliases.find(long_name); if (it == rev_aliases.end()) { - bool last_table = IdentifierSemantic::canReferColumnToTable(*identifier, tables.back()); + bool last_table = false; + { + size_t best_table_pos = 0; + if (IdentifierSemantic::chooseTable(*identifier, tables, best_table_pos)) + last_table = (best_table_pos + 1 == tables.size()); + } + if (!last_table) { String alias = hide_prefix + long_name; diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp index e05387b5aa0..7226ce9d4dd 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp +++ b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp @@ -62,7 +62,8 @@ void TranslateQualifiedNamesMatcher::visit(ASTIdentifier & identifier, ASTPtr &, if (IdentifierSemantic::getColumnName(identifier)) { size_t best_table_pos = 0; - IdentifierSemantic::trySetMembership(identifier, data.tables, best_table_pos); + if (IdentifierSemantic::chooseTable(identifier, data.tables, best_table_pos)) + IdentifierSemantic::setMembership(identifier, best_table_pos + 1); /// In case if column from the joined table are in source columns, change it's name to qualified. if (best_table_pos && data.hasColumn(identifier.shortName())) From 0f6176cc2d5fd9b3f4475077095eeb12794fd547 Mon Sep 17 00:00:00 2001 From: chertus Date: Fri, 18 Oct 2019 00:08:28 +0300 Subject: [PATCH 03/25] throw on ambiguous qualified column --- dbms/src/Interpreters/IdentifierSemantic.cpp | 77 ++++++++++++++----- dbms/src/Interpreters/IdentifierSemantic.h | 23 +++++- .../JoinToSubqueryTransformVisitor.cpp | 2 +- .../TranslateQualifiedNamesVisitor.cpp | 3 +- dbms/src/Parsers/ASTIdentifier.cpp | 12 ++- .../0_stateless/00826_cross_to_inner_join.sql | 4 +- 6 files changed, 95 insertions(+), 26 deletions(-) diff --git a/dbms/src/Interpreters/IdentifierSemantic.cpp b/dbms/src/Interpreters/IdentifierSemantic.cpp index f5006159aa9..39ad58c3bc7 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.cpp +++ b/dbms/src/Interpreters/IdentifierSemantic.cpp @@ -5,6 +5,11 @@ namespace DB { +namespace ErrorCodes +{ + extern const int AMBIGUOUS_COLUMN_NAME; +} + namespace { @@ -19,18 +24,37 @@ const DatabaseAndTableWithAlias & extractTable(const TableWithColumnNames & tabl } template -bool tryChooseTable(const ASTIdentifier & identifier, const std::vector & tables, size_t & best_table_pos) +IdentifierSemantic::ColumnMatch tryChooseTable(const ASTIdentifier & identifier, const std::vector & tables, + size_t & best_table_pos, bool allow_ambiguous) { + using ColumnMatch = IdentifierSemantic::ColumnMatch; + best_table_pos = 0; - size_t best_match = 0; + auto best_match = ColumnMatch::NoMatch; + size_t same_match = 0; + for (size_t i = 0; i < tables.size(); ++i) - if (size_t match = IdentifierSemantic::canReferColumnToTable(identifier, extractTable(tables[i]))) - if (match > best_match) + { + auto match = IdentifierSemantic::canReferColumnToTable(identifier, extractTable(tables[i])); + if (value(match)) + { + if (value(match) > value(best_match)) { best_match = match; best_table_pos = i; + same_match = 0; } + else if (match == best_match) + ++same_match; + } + } + if (value(best_match) && same_match) + { + if (!allow_ambiguous) + throw Exception("Ambiguous column '" + identifier.name + "'", ErrorCodes::AMBIGUOUS_COLUMN_NAME); + return ColumnMatch::Ambiguous; + } return best_match; } @@ -89,15 +113,15 @@ size_t IdentifierSemantic::getMembership(const ASTIdentifier & identifier) } bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, - size_t & best_table_pos) + size_t & best_table_pos, bool ambiguous) { - return tryChooseTable(identifier, tables, best_table_pos); + return value(tryChooseTable(identifier, tables, best_table_pos, ambiguous)); } bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, - size_t & best_table_pos) + size_t & best_table_pos, bool ambiguous) { - return tryChooseTable(identifier, tables, best_table_pos); + return value(tryChooseTable(identifier, tables, best_table_pos, ambiguous)); } std::pair IdentifierSemantic::extractDatabaseAndTable(const ASTIdentifier & identifier) @@ -127,18 +151,22 @@ bool IdentifierSemantic::doesIdentifierBelongTo(const ASTIdentifier & identifier return false; } -size_t IdentifierSemantic::canReferColumnToTable(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) +IdentifierSemantic::ColumnMatch IdentifierSemantic::canReferColumnToTable(const ASTIdentifier & identifier, + const DatabaseAndTableWithAlias & db_and_table) { /// database.table.column if (doesIdentifierBelongTo(identifier, db_and_table.database, db_and_table.table)) - return 2; + return ColumnMatch::DatabaseAndTable; - /// table.column or alias.column. - if (doesIdentifierBelongTo(identifier, db_and_table.table) || - doesIdentifierBelongTo(identifier, db_and_table.alias)) - return 1; + /// alias.column + if (doesIdentifierBelongTo(identifier, db_and_table.alias)) + return ColumnMatch::TableAlias; - return 0; + /// table.column + if (doesIdentifierBelongTo(identifier, db_and_table.table)) + return ColumnMatch::TableName; + + return ColumnMatch::NoMatch; } /// Checks that ast is ASTIdentifier and remove num_qualifiers_to_strip components from left. @@ -162,10 +190,23 @@ void IdentifierSemantic::setColumnShortName(ASTIdentifier & identifier, size_t t void IdentifierSemantic::setColumnNormalName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) { - size_t match = IdentifierSemantic::canReferColumnToTable(identifier, db_and_table); + auto match = IdentifierSemantic::canReferColumnToTable(identifier, db_and_table); + size_t to_strip = 0; + switch (match) + { + case ColumnMatch::TableName: + case ColumnMatch::TableAlias: + to_strip = 1; + break; + case ColumnMatch::DatabaseAndTable: + to_strip = 2; + break; + default: + break; + } - setColumnShortName(identifier, match); - if (match) + setColumnShortName(identifier, to_strip); + if (value(match)) identifier.semantic->can_be_alias = false; if (identifier.semantic->need_long_name) diff --git a/dbms/src/Interpreters/IdentifierSemantic.h b/dbms/src/Interpreters/IdentifierSemantic.h index 7403f5d2340..8a48227d6fe 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.h +++ b/dbms/src/Interpreters/IdentifierSemantic.h @@ -17,6 +17,16 @@ struct IdentifierSemanticImpl /// Static calss to manipulate IdentifierSemanticImpl via ASTIdentifier struct IdentifierSemantic { + enum class ColumnMatch + { + NoMatch, + ColumnName, /// table has column with same name + TableName, /// column qualified with table name + DatabaseAndTable, /// column qualified with database and table name + TableAlias, /// column qualified with table alias + Ambiguous, + }; + /// @returns name for column identifiers static std::optional getColumnName(const ASTIdentifier & node); static std::optional getColumnName(const ASTPtr & ast); @@ -26,7 +36,7 @@ struct IdentifierSemantic static std::optional getTableName(const ASTPtr & ast); static std::pair extractDatabaseAndTable(const ASTIdentifier & identifier); - static size_t canReferColumnToTable(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); + static ColumnMatch canReferColumnToTable(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); static String columnNormalName(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); static String columnLongName(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); static void setColumnNormalName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); @@ -35,8 +45,10 @@ struct IdentifierSemantic static bool canBeAlias(const ASTIdentifier & identifier); static void setMembership(ASTIdentifier & identifier, size_t table_no); static size_t getMembership(const ASTIdentifier & identifier); - static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos); - static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos); + static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos, + bool ambiguous = false); + static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos, + bool ambiguous = false); private: static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & database, const String & table); @@ -44,4 +56,9 @@ private: static void setColumnShortName(ASTIdentifier & identifier, size_t match); }; +inline UInt32 value(IdentifierSemantic::ColumnMatch match) +{ + return static_cast(match); +} + } diff --git a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp index f807bfb7acb..b7bf7c9b983 100644 --- a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp +++ b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp @@ -226,7 +226,7 @@ struct ColumnAliasesMatcher String long_name; for (auto & table : data.tables) { - if (IdentifierSemantic::canReferColumnToTable(node, table)) + if (value(IdentifierSemantic::canReferColumnToTable(node, table))) { if (!long_name.empty()) throw Exception("Cannot refer column '" + node.name + "' to one table", ErrorCodes::AMBIGUOUS_COLUMN_NAME); diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp index 7226ce9d4dd..204fe454052 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp +++ b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp @@ -62,7 +62,8 @@ void TranslateQualifiedNamesMatcher::visit(ASTIdentifier & identifier, ASTPtr &, if (IdentifierSemantic::getColumnName(identifier)) { size_t best_table_pos = 0; - if (IdentifierSemantic::chooseTable(identifier, data.tables, best_table_pos)) + bool allow_ambiguous = data.join_using_columns.count(identifier.shortName()); + if (IdentifierSemantic::chooseTable(identifier, data.tables, best_table_pos, allow_ambiguous)) IdentifierSemantic::setMembership(identifier, best_table_pos + 1); /// In case if column from the joined table are in source columns, change it's name to qualified. diff --git a/dbms/src/Parsers/ASTIdentifier.cpp b/dbms/src/Parsers/ASTIdentifier.cpp index e3948f99f5b..6307db675fa 100644 --- a/dbms/src/Parsers/ASTIdentifier.cpp +++ b/dbms/src/Parsers/ASTIdentifier.cpp @@ -34,10 +34,20 @@ ASTIdentifier::ASTIdentifier(const String & name_, std::vector && name_p , name_parts(name_parts_) , semantic(std::make_shared()) { + if (name_parts.size() && name_parts[0] == "") + name_parts.erase(name_parts.begin()); + + if (name == "") + { + if (name_parts.size() == 2) + name = name_parts[0] + '.' + name_parts[1]; + else if (name_parts.size() == 1) + name = name_parts[0]; + } } ASTIdentifier::ASTIdentifier(std::vector && name_parts_) - : ASTIdentifier(name_parts_.at(0) + '.' + name_parts_.at(1), std::move(name_parts_)) + : ASTIdentifier("", std::move(name_parts_)) {} void ASTIdentifier::setShortName(const String & new_name) diff --git a/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql b/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql index fa16cf398da..035662a0a0d 100644 --- a/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql +++ b/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql @@ -1,9 +1,9 @@ SET enable_debug_queries = 1; set allow_experimental_cross_to_join_conversion = 0; -select * from system.one cross join system.one; +select * from system.one l cross join system.one r; set allow_experimental_cross_to_join_conversion = 1; -select * from system.one cross join system.one; +select * from system.one l cross join system.one r; DROP TABLE IF EXISTS t1_00826; DROP TABLE IF EXISTS t2_00826; From 25336528470b4e19b2300399196f5f07dee2611d Mon Sep 17 00:00:00 2001 From: chertus Date: Fri, 18 Oct 2019 19:16:57 +0300 Subject: [PATCH 04/25] columns match priority: table alias > table name> aliased table name --- .../Interpreters/CollectJoinOnKeysVisitor.cpp | 3 +- dbms/src/Interpreters/IdentifierSemantic.cpp | 80 +++++++------------ dbms/src/Interpreters/IdentifierSemantic.h | 19 ++--- dbms/src/Interpreters/SyntaxAnalyzer.cpp | 7 +- .../TranslateQualifiedNamesVisitor.cpp | 46 ++++++----- .../00849_multiple_comma_join.reference | 12 +-- .../01018_anbiguous_column.reference | 6 ++ .../0_stateless/01018_anbiguous_column.sql | 21 +++++ 8 files changed, 105 insertions(+), 89 deletions(-) create mode 100644 dbms/tests/queries/0_stateless/01018_anbiguous_column.reference create mode 100644 dbms/tests/queries/0_stateless/01018_anbiguous_column.sql diff --git a/dbms/src/Interpreters/CollectJoinOnKeysVisitor.cpp b/dbms/src/Interpreters/CollectJoinOnKeysVisitor.cpp index f8938f2a7d3..894e1ea3a5a 100644 --- a/dbms/src/Interpreters/CollectJoinOnKeysVisitor.cpp +++ b/dbms/src/Interpreters/CollectJoinOnKeysVisitor.cpp @@ -164,7 +164,8 @@ size_t CollectJoinOnKeysMatcher::getTableForIdentifiers(std::vector IdentifierSemantic::getTableName(const ASTPtr & ast) return {}; } -void IdentifierSemantic::setNeedLongName(ASTIdentifier & identifier, bool value) -{ - identifier.semantic->need_long_name = value; -} - bool IdentifierSemantic::canBeAlias(const ASTIdentifier & identifier) { return identifier.semantic->can_be_alias; } -void IdentifierSemantic::setMembership(ASTIdentifier & identifier, size_t table_no) +void IdentifierSemantic::setMembership(ASTIdentifier & identifier, size_t table_pos) { - identifier.semantic->membership = table_no; + identifier.semantic->membership = table_pos; + identifier.semantic->can_be_alias = false; } -size_t IdentifierSemantic::getMembership(const ASTIdentifier & identifier) +std::optional IdentifierSemantic::getMembership(const ASTIdentifier & identifier) { return identifier.semantic->membership; } @@ -156,7 +152,7 @@ IdentifierSemantic::ColumnMatch IdentifierSemantic::canReferColumnToTable(const { /// database.table.column if (doesIdentifierBelongTo(identifier, db_and_table.database, db_and_table.table)) - return ColumnMatch::DatabaseAndTable; + return ColumnMatch::DbAndTable; /// alias.column if (doesIdentifierBelongTo(identifier, db_and_table.alias)) @@ -164,15 +160,36 @@ IdentifierSemantic::ColumnMatch IdentifierSemantic::canReferColumnToTable(const /// table.column if (doesIdentifierBelongTo(identifier, db_and_table.table)) - return ColumnMatch::TableName; + { + if (!db_and_table.alias.empty()) + return ColumnMatch::AliasedTableName; + else + return ColumnMatch::TableName; + } return ColumnMatch::NoMatch; } -/// Checks that ast is ASTIdentifier and remove num_qualifiers_to_strip components from left. -/// Example: 'database.table.name' -> (num_qualifiers_to_strip = 2) -> 'name'. -void IdentifierSemantic::setColumnShortName(ASTIdentifier & identifier, size_t to_strip) +/// Strip qualificators from left side of column name. +/// Example: 'database.table.name' -> 'name'. +void IdentifierSemantic::setColumnShortName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) { + auto match = IdentifierSemantic::canReferColumnToTable(identifier, db_and_table); + size_t to_strip = 0; + switch (match) + { + case ColumnMatch::TableName: + case ColumnMatch::AliasedTableName: + case ColumnMatch::TableAlias: + to_strip = 1; + break; + case ColumnMatch::DbAndTable: + to_strip = 2; + break; + default: + break; + } + if (!to_strip) return; @@ -188,31 +205,6 @@ void IdentifierSemantic::setColumnShortName(ASTIdentifier & identifier, size_t t identifier.name.swap(new_name); } -void IdentifierSemantic::setColumnNormalName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) -{ - auto match = IdentifierSemantic::canReferColumnToTable(identifier, db_and_table); - size_t to_strip = 0; - switch (match) - { - case ColumnMatch::TableName: - case ColumnMatch::TableAlias: - to_strip = 1; - break; - case ColumnMatch::DatabaseAndTable: - to_strip = 2; - break; - default: - break; - } - - setColumnShortName(identifier, to_strip); - if (value(match)) - identifier.semantic->can_be_alias = false; - - if (identifier.semantic->need_long_name) - setColumnLongName(identifier, db_and_table); -} - void IdentifierSemantic::setColumnLongName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) { String prefix = db_and_table.getQualifiedNamePrefix(); @@ -225,16 +217,4 @@ void IdentifierSemantic::setColumnLongName(ASTIdentifier & identifier, const Dat } } -String IdentifierSemantic::columnNormalName(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) -{ - ASTPtr copy = identifier.clone(); - setColumnNormalName(copy->as(), db_and_table); - return copy->getAliasOrColumnName(); -} - -String IdentifierSemantic::columnLongName(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table) -{ - return db_and_table.getQualifiedNamePrefix() + identifier.shortName(); -} - } diff --git a/dbms/src/Interpreters/IdentifierSemantic.h b/dbms/src/Interpreters/IdentifierSemantic.h index 8a48227d6fe..832a3345b5a 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.h +++ b/dbms/src/Interpreters/IdentifierSemantic.h @@ -1,5 +1,7 @@ #pragma once +#include + #include #include @@ -9,9 +11,8 @@ namespace DB struct IdentifierSemanticImpl { bool special = false; /// for now it's 'not a column': tables, subselects and some special stuff like FORMAT - bool need_long_name = false;/// if column presents in multiple tables we need qualified names bool can_be_alias = true; /// if it's a cropped name it could not be an alias - size_t membership = 0; /// table position in join (starting from 1) detected by qualifier or 0 if not detected. + std::optional membership; /// table position in join }; /// Static calss to manipulate IdentifierSemanticImpl via ASTIdentifier @@ -20,9 +21,9 @@ struct IdentifierSemantic enum class ColumnMatch { NoMatch, - ColumnName, /// table has column with same name + AliasedTableName, /// column qualified with table name (but table has an alias so its priority is lower than TableName) TableName, /// column qualified with table name - DatabaseAndTable, /// column qualified with database and table name + DbAndTable, /// column qualified with database and table name TableAlias, /// column qualified with table alias Ambiguous, }; @@ -37,14 +38,11 @@ struct IdentifierSemantic static std::pair extractDatabaseAndTable(const ASTIdentifier & identifier); static ColumnMatch canReferColumnToTable(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); - static String columnNormalName(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); - static String columnLongName(const ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); - static void setColumnNormalName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); + static void setColumnShortName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); static void setColumnLongName(ASTIdentifier & identifier, const DatabaseAndTableWithAlias & db_and_table); - static void setNeedLongName(ASTIdentifier & identifier, bool); /// if set setColumnNormalName makes qualified name static bool canBeAlias(const ASTIdentifier & identifier); - static void setMembership(ASTIdentifier & identifier, size_t table_no); - static size_t getMembership(const ASTIdentifier & identifier); + static void setMembership(ASTIdentifier &, size_t table_no); + static std::optional getMembership(const ASTIdentifier & identifier); static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos, bool ambiguous = false); static bool chooseTable(const ASTIdentifier &, const std::vector & tables, size_t & best_table_pos, @@ -53,7 +51,6 @@ struct IdentifierSemantic private: static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & database, const String & table); static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & table); - static void setColumnShortName(ASTIdentifier & identifier, size_t match); }; inline UInt32 value(IdentifierSemantic::ColumnMatch match) diff --git a/dbms/src/Interpreters/SyntaxAnalyzer.cpp b/dbms/src/Interpreters/SyntaxAnalyzer.cpp index 3d43b7f0b25..c7cd789f882 100644 --- a/dbms/src/Interpreters/SyntaxAnalyzer.cpp +++ b/dbms/src/Interpreters/SyntaxAnalyzer.cpp @@ -565,11 +565,16 @@ void collectJoinedColumns(AnalyzedJoin & analyzed_join, const ASTSelectQuery & s } } -void replaceJoinedTable(const ASTTablesInSelectQueryElement* join) +void replaceJoinedTable(const ASTTablesInSelectQueryElement * join) { if (!join || !join->table_expression) return; + /// TODO: Push down for CROSS JOIN is not OK [disabled] + const auto & table_join = join->table_join->as(); + if (table_join.kind == ASTTableJoin::Kind::Cross) + return; + auto & table_expr = join->table_expression->as(); if (table_expr.database_and_table_name) { diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp index 204fe454052..549b1c0c1e6 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp +++ b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp @@ -61,16 +61,20 @@ void TranslateQualifiedNamesMatcher::visit(ASTIdentifier & identifier, ASTPtr &, { if (IdentifierSemantic::getColumnName(identifier)) { - size_t best_table_pos = 0; - bool allow_ambiguous = data.join_using_columns.count(identifier.shortName()); - if (IdentifierSemantic::chooseTable(identifier, data.tables, best_table_pos, allow_ambiguous)) - IdentifierSemantic::setMembership(identifier, best_table_pos + 1); + String short_name = identifier.shortName(); + size_t table_pos = 0; + bool allow_ambiguous = data.join_using_columns.count(short_name); + if (IdentifierSemantic::chooseTable(identifier, data.tables, table_pos, allow_ambiguous)) + { + IdentifierSemantic::setMembership(identifier, table_pos); - /// In case if column from the joined table are in source columns, change it's name to qualified. - if (best_table_pos && data.hasColumn(identifier.shortName())) - IdentifierSemantic::setNeedLongName(identifier, true); - if (data.hasTable()) - IdentifierSemantic::setColumnNormalName(identifier, data.tables[best_table_pos].first); + /// In case if column from the joined table are in source columns, change it's name to qualified. + auto & table = data.tables[table_pos].first; + if (table_pos && data.hasColumn(short_name)) + IdentifierSemantic::setColumnLongName(identifier, table); + else + IdentifierSemantic::setColumnShortName(identifier, table); + } } } @@ -126,8 +130,10 @@ void TranslateQualifiedNamesMatcher::visit(ASTSelectQuery & select, const ASTPtr Visitor(data).visit(select.refHaving()); } -static void addIdentifier(ASTs & nodes, const String & table_name, const String & column_name, AsteriskSemantic::RevertedAliasesPtr aliases) +static void addIdentifier(ASTs & nodes, const DatabaseAndTableWithAlias & table, const String & column_name, + AsteriskSemantic::RevertedAliasesPtr aliases) { + String table_name = table.getQualifiedNamePrefix(false); auto identifier = std::make_shared(std::vector{table_name, column_name}); bool added = false; @@ -189,8 +195,7 @@ void TranslateQualifiedNamesMatcher::visit(ASTExpressionList & node, const ASTPt { if (first_table || !data.join_using_columns.count(column_name)) { - String table_name = table.getQualifiedNamePrefix(false); - addIdentifier(node.children, table_name, column_name, AsteriskSemantic::getAliases(*asterisk)); + addIdentifier(node.children, table, column_name, AsteriskSemantic::getAliases(*asterisk)); } } @@ -206,8 +211,7 @@ void TranslateQualifiedNamesMatcher::visit(ASTExpressionList & node, const ASTPt { if (asterisk_pattern->isColumnMatching(column_name) && (first_table || !data.join_using_columns.count(column_name))) { - String table_name = table.getQualifiedNamePrefix(false); - addIdentifier(node.children, table_name, column_name, AsteriskSemantic::getAliases(*asterisk_pattern)); + addIdentifier(node.children, table, column_name, AsteriskSemantic::getAliases(*asterisk_pattern)); } } @@ -224,8 +228,7 @@ void TranslateQualifiedNamesMatcher::visit(ASTExpressionList & node, const ASTPt { for (const auto & column_name : table_columns) { - String table_name = table.getQualifiedNamePrefix(false); - addIdentifier(node.children, table_name, column_name, AsteriskSemantic::getAliases(*qualified_asterisk)); + addIdentifier(node.children, table, column_name, AsteriskSemantic::getAliases(*qualified_asterisk)); } break; } @@ -261,11 +264,14 @@ void TranslateQualifiedNamesMatcher::extractJoinUsingColumns(const ASTPtr ast, D void RestoreQualifiedNamesData::visit(ASTIdentifier & identifier, ASTPtr & ast) { - if (IdentifierSemantic::getColumnName(identifier) && - IdentifierSemantic::getMembership(identifier)) + if (IdentifierSemantic::getColumnName(identifier)) { - ast = identifier.clone(); - ast->as()->restoreCompoundName(); + auto opt_match = IdentifierSemantic::getMembership(identifier); + if (opt_match && *opt_match) + { + ast = identifier.clone(); + ast->as()->restoreCompoundName(); + } } } diff --git a/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference b/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference index e1256053739..65ec7f67718 100644 --- a/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference +++ b/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference @@ -1,4 +1,4 @@ -SELECT a\nFROM t1_00849\nCROSS JOIN \n(\n SELECT *\n FROM t2_00849\n) AS t2_00849 +SELECT a\nFROM t1_00849\nCROSS JOIN t2_00849 SELECT a\nFROM t1_00849\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00849\n) AS t2_00849 ON a = t2_00849.a\nWHERE a = t2_00849.a SELECT a\nFROM t1_00849\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00849\n) AS t2_00849 ON b = t2_00849.b\nWHERE b = t2_00849.b SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n WHERE `--t1_00849.a` = `--t2_00849.a`\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t3_00849\n) AS t3_00849 ON `--t1_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t1_00849.a` = a) @@ -6,12 +6,12 @@ SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1 SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n WHERE `--t1_00849.a` = `--t2_00849.a`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t1_00849.a` = `--t3_00849.a`\n WHERE (`--t1_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = `--t2_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t1_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t1_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = a) SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n `--t1_00849.b`, \n `t2_00849.a`, \n `--t2_00849.b`, \n a, \n b AS `--t3_00849.b`\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b AS `--t1_00849.b`, \n t2_00849.a, \n t2_00849.b AS `--t2_00849.b`\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.b` = `--t2_00849.b`\n WHERE `--t1_00849.b` = `--t2_00849.b`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t1_00849.b` = `--t3_00849.b`\n WHERE (`--t1_00849.b` = `--t3_00849.b`) AND (`--t1_00849.b` = `--t2_00849.b`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t1_00849.b` = b\nWHERE (`--t1_00849.b` = `--t2_00849.b`) AND (`--t1_00849.b` = `--t3_00849.b`) AND (`--t1_00849.b` = b) SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t2_00849.a` = `--t1_00849.a`\n WHERE `--t2_00849.a` = `--t1_00849.a`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t2_00849.a` = `--t3_00849.a`\n WHERE (`--t2_00849.a` = `--t3_00849.a`) AND (`--t2_00849.a` = `--t1_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t2_00849.a` = a\nWHERE (`--t2_00849.a` = `--t1_00849.a`) AND (`--t2_00849.a` = `--t3_00849.a`) AND (`--t2_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`)\n WHERE (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = `--t1_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849\n )\n CROSS JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`)\nWHERE (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n a AS `--t2_00849.a`, \n b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`)\n WHERE (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = `--t1_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = a) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`)\nWHERE (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`) SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n WHERE `--t1_00849.a` = `--t2_00849.a`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t2_00849.a` = `--t3_00849.a`\n WHERE (`--t2_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = `--t2_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t2_00849.a` = `--t3_00849.a`) AND (`--t3_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `t2_00849.a`, \n `t2_00849.b`, \n a, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849\n )\n CROSS JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849\n)\nCROSS JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `t2_00849.a`, \n `t2_00849.b`, \n a, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849\n )\n CROSS JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849\n)\nCROSS JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n)\nCROSS JOIN \n(\n SELECT *\n FROM t3_00849\n) AS t3_00849 +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `t2_00849.a`, \n `t2_00849.b`, \n a, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nCROSS JOIN t4_00849 +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `t2_00849.a`, \n `t2_00849.b`, \n a, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nCROSS JOIN t4_00849 +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n)\nCROSS JOIN t3_00849 SELECT * FROM t1, t2 1 1 1 1 1 1 1 \N diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference b/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference new file mode 100644 index 00000000000..90d1da5b891 --- /dev/null +++ b/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference @@ -0,0 +1,6 @@ +0 0 +0 0 +0 +0 +0 +0 diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql b/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql new file mode 100644 index 00000000000..2873d3e58a0 --- /dev/null +++ b/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql @@ -0,0 +1,21 @@ +select * from system.one cross join system.one; -- { serverError 352 } +select * from system.one cross join system.one r; +select * from system.one l cross join system.one; +select * from system.one left join system.one using dummy; +select dummy from system.one left join system.one using dummy; + +USE system; + +SELECT dummy FROM one AS A JOIN one ON A.dummy = one.dummy; +SELECT dummy FROM one JOIN one AS A ON A.dummy = one.dummy; +-- SELECT dummy FROM one l JOIN one r ON l.dummy = r.dummy; -- should be an error + +-- SELECT * from system.one +-- JOIN system.one one ON one.dummy = system.one.dummy +-- JOIN system.one two ON one.dummy = two.dummy +-- FORMAT PrettyCompact; + +-- SELECT * from system.one one +-- JOIN system.one ON one.dummy = system.one.dummy +-- JOIN system.one two ON one.dummy = two.dummy +-- FORMAT PrettyCompact; From 1bb81742852b3c5f6fa7fb02f4e8481d7152e90d Mon Sep 17 00:00:00 2001 From: chertus Date: Fri, 18 Oct 2019 19:34:06 +0300 Subject: [PATCH 05/25] disable push down for CROSS JOIN tests --- .../00826_cross_to_inner_join.reference | 28 +++++++++---------- .../0_stateless/00826_cross_to_inner_join.sql | 1 + .../0_stateless/00849_multiple_comma_join.sql | 1 + 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.reference b/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.reference index 24649ea3acb..df21becc999 100644 --- a/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.reference +++ b/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.reference @@ -56,26 +56,26 @@ comma nullable 1 1 1 1 2 2 1 2 cross -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE a = t2_00826.a -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826 ON a = t2_00826.a\nWHERE a = t2_00826.a +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE a = t2_00826.a +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN t2_00826 ON a = t2_00826.a\nWHERE a = t2_00826.a cross nullable -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\n, \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE a = t2_00826.a -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826 ON a = t2_00826.a\nWHERE a = t2_00826.a +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\n, t2_00826\nWHERE a = t2_00826.a +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN t2_00826 ON a = t2_00826.a\nWHERE a = t2_00826.a cross nullable vs not nullable -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE a = t2_00826.b -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826 ON a = t2_00826.b\nWHERE a = t2_00826.b +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE a = t2_00826.b +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN t2_00826 ON a = t2_00826.b\nWHERE a = t2_00826.b cross self SELECT \n a, \n b, \n y.a, \n y.b\nFROM t1_00826 AS x\nCROSS JOIN t1_00826 AS y\nWHERE (a = y.a) AND (b = y.b) SELECT \n a, \n b, \n y.a, \n y.b\nFROM t1_00826 AS x\nALL INNER JOIN t1_00826 AS y ON (a = y.a) AND (b = y.b)\nWHERE (a = y.a) AND (b = y.b) cross one table expr -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE a = b -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE a = b +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE a = b +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE a = b cross multiple ands -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826 ON (a = t2_00826.a) AND (b = t2_00826.b)\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN t2_00826 ON (a = t2_00826.a) AND (b = t2_00826.b)\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) cross and inside and -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826\nWHERE (a = t2_00826.a) AND ((a = t2_00826.a) AND ((a = t2_00826.a) AND (b = t2_00826.b))) -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00826\n) AS t2_00826 ON (a = t2_00826.a) AND (a = t2_00826.a) AND (a = t2_00826.a) AND (b = t2_00826.b)\nWHERE (a = t2_00826.a) AND ((a = t2_00826.a) AND ((a = t2_00826.a) AND (b = t2_00826.b))) +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE (a = t2_00826.a) AND ((a = t2_00826.a) AND ((a = t2_00826.a) AND (b = t2_00826.b))) +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN t2_00826 ON (a = t2_00826.a) AND (a = t2_00826.a) AND (a = t2_00826.a) AND (b = t2_00826.b)\nWHERE (a = t2_00826.a) AND ((a = t2_00826.a) AND ((a = t2_00826.a) AND (b = t2_00826.b))) cross split conjunction -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN \n(\n SELECT *\n FROM t2_00826\n WHERE b > 0\n) AS t2_00826\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) AND (a >= 1) AND (t2_00826.b > 0) -SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00826\n WHERE b > 0\n) AS t2_00826 ON (a = t2_00826.a) AND (b = t2_00826.b)\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) AND (a >= 1) AND (t2_00826.b > 0) +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nCROSS JOIN t2_00826\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) AND (a >= 1) AND (t2_00826.b > 0) +SELECT \n a, \n b, \n t2_00826.a, \n t2_00826.b\nFROM t1_00826\nALL INNER JOIN t2_00826 ON (a = t2_00826.a) AND (b = t2_00826.b)\nWHERE (a = t2_00826.a) AND (b = t2_00826.b) AND (a >= 1) AND (t2_00826.b > 0) diff --git a/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql b/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql index 035662a0a0d..e21d257d2da 100644 --- a/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql +++ b/dbms/tests/queries/0_stateless/00826_cross_to_inner_join.sql @@ -1,4 +1,5 @@ SET enable_debug_queries = 1; +SET enable_optimize_predicate_expression = 0; set allow_experimental_cross_to_join_conversion = 0; select * from system.one l cross join system.one r; diff --git a/dbms/tests/queries/0_stateless/00849_multiple_comma_join.sql b/dbms/tests/queries/0_stateless/00849_multiple_comma_join.sql index d1d247a0174..f80daecbe87 100644 --- a/dbms/tests/queries/0_stateless/00849_multiple_comma_join.sql +++ b/dbms/tests/queries/0_stateless/00849_multiple_comma_join.sql @@ -1,4 +1,5 @@ SET enable_debug_queries = 1; +SET enable_optimize_predicate_expression = 0; SET joined_subquery_requires_alias = 0; DROP TABLE IF EXISTS t1_00849; From 37f07213eec2d18f386634f49c72aecb94fd212c Mon Sep 17 00:00:00 2001 From: chertus Date: Fri, 18 Oct 2019 19:45:59 +0300 Subject: [PATCH 06/25] better test --- .../0_stateless/01018_anbiguous_column.sql | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql b/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql index 2873d3e58a0..496a616da42 100644 --- a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql +++ b/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql @@ -8,14 +8,17 @@ USE system; SELECT dummy FROM one AS A JOIN one ON A.dummy = one.dummy; SELECT dummy FROM one JOIN one AS A ON A.dummy = one.dummy; --- SELECT dummy FROM one l JOIN one r ON l.dummy = r.dummy; -- should be an error +SELECT dummy FROM one l JOIN one r ON dummy = r.dummy; -- { serverError 352 } +SELECT dummy FROM one l JOIN one r ON l.dummy = dummy; -- { serverError 352 } +SELECT dummy FROM one l JOIN one r ON one.dummy = r.dummy; -- { serverError 352 } +SELECT dummy FROM one l JOIN one r ON l.dummy = one.dummy; -- { serverError 352 } --- SELECT * from system.one --- JOIN system.one one ON one.dummy = system.one.dummy --- JOIN system.one two ON one.dummy = two.dummy +-- SELECT * from one +-- JOIN one A ON one.dummy = A.dummy +-- JOIN one B ON one.dummy = B.dummy -- FORMAT PrettyCompact; - --- SELECT * from system.one one --- JOIN system.one ON one.dummy = system.one.dummy --- JOIN system.one two ON one.dummy = two.dummy +-- +-- SELECT * from one +-- JOIN one A ON dummy = A.dummy +-- JOIN one B ON dummy = B.dummy -- FORMAT PrettyCompact; From c5a850240c6eaf948ba719f335f0e3f7f8078450 Mon Sep 17 00:00:00 2001 From: chertus Date: Fri, 18 Oct 2019 20:09:17 +0300 Subject: [PATCH 07/25] better ambiguous column detection in multiple join rewriter --- .../JoinToSubqueryTransformVisitor.cpp | 18 ++++++++---------- .../01018_anbiguous_column.reference | 6 ++++++ .../0_stateless/01018_anbiguous_column.sql | 18 +++++++++--------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp index b7bf7c9b983..a2536521a7d 100644 --- a/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp +++ b/dbms/src/Interpreters/JoinToSubqueryTransformVisitor.cpp @@ -224,17 +224,15 @@ struct ColumnAliasesMatcher bool last_table = false; String long_name; - for (auto & table : data.tables) + + size_t table_pos = 0; + if (IdentifierSemantic::chooseTable(node, data.tables, table_pos)) { - if (value(IdentifierSemantic::canReferColumnToTable(node, table))) - { - if (!long_name.empty()) - throw Exception("Cannot refer column '" + node.name + "' to one table", ErrorCodes::AMBIGUOUS_COLUMN_NAME); - IdentifierSemantic::setColumnLongName(node, table); /// table_name.column_name -> table_alias.column_name - long_name = node.name; - if (&table == &data.tables.back()) - last_table = true; - } + auto & table = data.tables[table_pos]; + IdentifierSemantic::setColumnLongName(node, table); /// table_name.column_name -> table_alias.column_name + long_name = node.name; + if (&table == &data.tables.back()) + last_table = true; } if (long_name.empty()) diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference b/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference index 90d1da5b891..90b24009d0f 100644 --- a/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference +++ b/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference @@ -4,3 +4,9 @@ 0 0 0 +┌─one.dummy─┬─A.dummy─┬─B.dummy─┐ +│ 0 │ 0 │ 0 │ +└───────────┴─────────┴─────────┘ +┌─A.dummy─┬─one.dummy─┬─two.dummy─┐ +│ 0 │ 0 │ 0 │ +└─────────┴───────────┴───────────┘ diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql b/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql index 496a616da42..ab291178f87 100644 --- a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql +++ b/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql @@ -13,12 +13,12 @@ SELECT dummy FROM one l JOIN one r ON l.dummy = dummy; -- { serverError 352 } SELECT dummy FROM one l JOIN one r ON one.dummy = r.dummy; -- { serverError 352 } SELECT dummy FROM one l JOIN one r ON l.dummy = one.dummy; -- { serverError 352 } --- SELECT * from one --- JOIN one A ON one.dummy = A.dummy --- JOIN one B ON one.dummy = B.dummy --- FORMAT PrettyCompact; --- --- SELECT * from one --- JOIN one A ON dummy = A.dummy --- JOIN one B ON dummy = B.dummy --- FORMAT PrettyCompact; +SELECT * from one +JOIN one A ON one.dummy = A.dummy +JOIN one B ON one.dummy = B.dummy +FORMAT PrettyCompact; + +SELECT * from one A +JOIN system.one one ON A.dummy = one.dummy +JOIN system.one two ON A.dummy = two.dummy +FORMAT PrettyCompact; From 260a9fba2b72512ae91a8784f07cf5f0b4fec2c5 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Fri, 18 Oct 2019 14:26:39 +0300 Subject: [PATCH 08/25] StorageDistributed: Drop extra inclusion of materializeBlock.h --- dbms/src/Storages/StorageDistributed.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/dbms/src/Storages/StorageDistributed.cpp b/dbms/src/Storages/StorageDistributed.cpp index 2c289dd714e..8361daefd87 100644 --- a/dbms/src/Storages/StorageDistributed.cpp +++ b/dbms/src/Storages/StorageDistributed.cpp @@ -1,7 +1,6 @@ #include #include -#include #include From 5ef7376cbf7a74437f3d316793a2dd45e1c31046 Mon Sep 17 00:00:00 2001 From: chertus Date: Mon, 21 Oct 2019 14:22:22 +0300 Subject: [PATCH 09/25] fix distributed join on --- dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp index 549b1c0c1e6..df0946f098a 100644 --- a/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp +++ b/dbms/src/Interpreters/TranslateQualifiedNamesVisitor.cpp @@ -266,8 +266,7 @@ void RestoreQualifiedNamesData::visit(ASTIdentifier & identifier, ASTPtr & ast) { if (IdentifierSemantic::getColumnName(identifier)) { - auto opt_match = IdentifierSemantic::getMembership(identifier); - if (opt_match && *opt_match) + if (IdentifierSemantic::getMembership(identifier)) { ast = identifier.clone(); ast->as()->restoreCompoundName(); From 0d708569e376e9d9f977a4212fc86fd68f317b11 Mon Sep 17 00:00:00 2001 From: chertus Date: Mon, 21 Oct 2019 14:31:16 +0300 Subject: [PATCH 10/25] fix test result (disabled push down) --- .../00849_multiple_comma_join.reference | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference b/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference index 65ec7f67718..453458a6ecf 100644 --- a/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference +++ b/dbms/tests/queries/0_stateless/00849_multiple_comma_join.reference @@ -1,17 +1,17 @@ SELECT a\nFROM t1_00849\nCROSS JOIN t2_00849 -SELECT a\nFROM t1_00849\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00849\n) AS t2_00849 ON a = t2_00849.a\nWHERE a = t2_00849.a -SELECT a\nFROM t1_00849\nALL INNER JOIN \n(\n SELECT *\n FROM t2_00849\n) AS t2_00849 ON b = t2_00849.b\nWHERE b = t2_00849.b -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n WHERE `--t1_00849.a` = `--t2_00849.a`\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t3_00849\n) AS t3_00849 ON `--t1_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t1_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b AS `--t1_00849.b`, \n t2_00849.a, \n t2_00849.b AS `--t2_00849.b`\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.b` = `--t2_00849.b`\n WHERE `--t1_00849.b` = `--t2_00849.b`\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t3_00849\n) AS t3_00849 ON `--t1_00849.b` = b\nWHERE (`--t1_00849.b` = `--t2_00849.b`) AND (`--t1_00849.b` = b) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n WHERE `--t1_00849.a` = `--t2_00849.a`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t1_00849.a` = `--t3_00849.a`\n WHERE (`--t1_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = `--t2_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t1_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t1_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n `--t1_00849.b`, \n `t2_00849.a`, \n `--t2_00849.b`, \n a, \n b AS `--t3_00849.b`\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b AS `--t1_00849.b`, \n t2_00849.a, \n t2_00849.b AS `--t2_00849.b`\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.b` = `--t2_00849.b`\n WHERE `--t1_00849.b` = `--t2_00849.b`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t1_00849.b` = `--t3_00849.b`\n WHERE (`--t1_00849.b` = `--t3_00849.b`) AND (`--t1_00849.b` = `--t2_00849.b`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t1_00849.b` = b\nWHERE (`--t1_00849.b` = `--t2_00849.b`) AND (`--t1_00849.b` = `--t3_00849.b`) AND (`--t1_00849.b` = b) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t2_00849.a` = `--t1_00849.a`\n WHERE `--t2_00849.a` = `--t1_00849.a`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t2_00849.a` = `--t3_00849.a`\n WHERE (`--t2_00849.a` = `--t3_00849.a`) AND (`--t2_00849.a` = `--t1_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t2_00849.a` = a\nWHERE (`--t2_00849.a` = `--t1_00849.a`) AND (`--t2_00849.a` = `--t3_00849.a`) AND (`--t2_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n a AS `--t2_00849.a`, \n b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`)\n WHERE (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = `--t1_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = a) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`)\nWHERE (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`) -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n WHERE `--t1_00849.a` = `--t2_00849.a`\n )\n ALL INNER JOIN \n (\n SELECT *\n FROM t3_00849\n ) AS t3_00849 ON `--t2_00849.a` = `--t3_00849.a`\n WHERE (`--t2_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = `--t2_00849.a`)\n)\nALL INNER JOIN \n(\n SELECT *\n FROM t4_00849\n) AS t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t2_00849.a` = `--t3_00849.a`) AND (`--t3_00849.a` = a) +SELECT a\nFROM t1_00849\nALL INNER JOIN t2_00849 ON a = t2_00849.a\nWHERE a = t2_00849.a +SELECT a\nFROM t1_00849\nALL INNER JOIN t2_00849 ON b = t2_00849.b\nWHERE b = t2_00849.b +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n)\nALL INNER JOIN t3_00849 ON `--t1_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t1_00849.a` = a) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b AS `--t1_00849.b`, \n t2_00849.a, \n t2_00849.b AS `--t2_00849.b`\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t1_00849.b` = `--t2_00849.b`\n)\nALL INNER JOIN t3_00849 ON `--t1_00849.b` = b\nWHERE (`--t1_00849.b` = `--t2_00849.b`) AND (`--t1_00849.b` = b) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n )\n ALL INNER JOIN t3_00849 ON `--t1_00849.a` = `--t3_00849.a`\n)\nALL INNER JOIN t4_00849 ON `--t1_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t1_00849.a` = `--t3_00849.a`) AND (`--t1_00849.a` = a) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n `--t1_00849.b`, \n `t2_00849.a`, \n `--t2_00849.b`, \n a, \n b AS `--t3_00849.b`\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b AS `--t1_00849.b`, \n t2_00849.a, \n t2_00849.b AS `--t2_00849.b`\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t1_00849.b` = `--t2_00849.b`\n )\n ALL INNER JOIN t3_00849 ON `--t1_00849.b` = `--t3_00849.b`\n)\nALL INNER JOIN t4_00849 ON `--t1_00849.b` = b\nWHERE (`--t1_00849.b` = `--t2_00849.b`) AND (`--t1_00849.b` = `--t3_00849.b`) AND (`--t1_00849.b` = b) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t2_00849.a` = `--t1_00849.a`\n )\n ALL INNER JOIN t3_00849 ON `--t2_00849.a` = `--t3_00849.a`\n)\nALL INNER JOIN t4_00849 ON `--t2_00849.a` = a\nWHERE (`--t2_00849.a` = `--t1_00849.a`) AND (`--t2_00849.a` = `--t3_00849.a`) AND (`--t2_00849.a` = a) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n ALL INNER JOIN t3_00849 ON (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`)\n)\nALL INNER JOIN t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t3_00849.a` = `--t1_00849.a`) AND (`--t3_00849.a` = `--t2_00849.a`) AND (`--t3_00849.a` = a) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nALL INNER JOIN t4_00849 ON (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`)\nWHERE (a = `--t1_00849.a`) AND (a = `--t2_00849.a`) AND (a = `--t3_00849.a`) +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `--t2_00849.a`, \n `t2_00849.b`, \n a AS `--t3_00849.a`, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n )\n ALL INNER JOIN t3_00849 ON `--t2_00849.a` = `--t3_00849.a`\n)\nALL INNER JOIN t4_00849 ON `--t3_00849.a` = a\nWHERE (`--t1_00849.a` = `--t2_00849.a`) AND (`--t2_00849.a` = `--t3_00849.a`) AND (`--t3_00849.a` = a) SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `t2_00849.a`, \n `t2_00849.b`, \n a, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nCROSS JOIN t4_00849 SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n `--t1_00849.a`, \n b, \n `t2_00849.a`, \n `t2_00849.b`, \n a, \n t3_00849.b\n FROM \n (\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a, \n t2_00849.b\n FROM t1_00849\n CROSS JOIN t2_00849\n )\n CROSS JOIN t3_00849\n)\nCROSS JOIN t4_00849 -SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN \n (\n SELECT *\n FROM t2_00849\n ) AS t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n)\nCROSS JOIN t3_00849 +SELECT `--t1_00849.a` AS `t1_00849.a`\nFROM \n(\n SELECT \n a AS `--t1_00849.a`, \n b, \n t2_00849.a AS `--t2_00849.a`, \n t2_00849.b\n FROM t1_00849\n ALL INNER JOIN t2_00849 ON `--t1_00849.a` = `--t2_00849.a`\n)\nCROSS JOIN t3_00849 SELECT * FROM t1, t2 1 1 1 1 1 1 1 \N From 9446fd2c4dcfd0f50b5befa12dceddb617dd7ca0 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Tue, 22 Oct 2019 17:45:01 +0300 Subject: [PATCH 11/25] Allowed to have some parts on destination disk or volume in MOVE PARTITION. #7424 --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index af985c02927..f530c537671 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -2723,14 +2723,19 @@ void MergeTreeData::movePartitionToDisk(const ASTPtr & partition, const String & if (!disk) throw Exception("Disk " + name + " does not exists on policy " + storage_policy->getName(), ErrorCodes::UNKNOWN_DISK); - for (const auto & part : parts) - { - if (part->disk->getName() == disk->getName()) - throw Exception("Part " + part->name + " already on disk " + name, ErrorCodes::UNKNOWN_DISK); - } + parts.erase(std::remove_if(parts.begin(), parts.end(), [&](auto part_ptr) { + return part_ptr->disk->getName() == disk->getName(); + }), parts.end()); - if (!movePartsToSpace(parts, std::static_pointer_cast(disk))) - throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); + if (!parts.empty()) + { + if (!movePartsToSpace(parts, std::static_pointer_cast(disk))) + throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); + } + else + { + LOG_DEBUG(log, "No parts of partition " << partition_id << " to move to disk " << disk->getName()); + } } @@ -2763,8 +2768,27 @@ void MergeTreeData::movePartitionToVolume(const ASTPtr & partition, const String if (part->disk->getName() == disk->getName()) throw Exception("Part " + part->name + " already on volume '" + name + "'", ErrorCodes::UNKNOWN_DISK); - if (!movePartsToSpace(parts, std::static_pointer_cast(volume))) - throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); + parts.erase(std::remove_if(parts.begin(), parts.end(), [&](auto part_ptr) { + for (const auto & disk : volume->disks) + { + if (part_ptr->disk->getName() == disk->getName()) + { + return true; + } + } + return false; + }), parts.end()); + + + if (!parts.empty()) + { + if (!movePartsToSpace(parts, std::static_pointer_cast(volume))) + throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); + } + else + { + LOG_DEBUG(log, "No parts of partition " << partition_id << " to move to volume " << volume->getName()); + } } From 7546c315a6089069c2864c77d575b4338a84a97b Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Wed, 23 Oct 2019 14:25:51 +0300 Subject: [PATCH 12/25] Added integration test for #7414. --- .../config.d/storage_configuration.xml | 21 +++++++++++++++ .../integration/test_multiple_disks/test.py | 27 +++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/dbms/tests/integration/test_multiple_disks/configs/config.d/storage_configuration.xml b/dbms/tests/integration/test_multiple_disks/configs/config.d/storage_configuration.xml index d41ba6066c4..2e6a1f80a6d 100644 --- a/dbms/tests/integration/test_multiple_disks/configs/config.d/storage_configuration.xml +++ b/dbms/tests/integration/test_multiple_disks/configs/config.d/storage_configuration.xml @@ -74,6 +74,27 @@ + + + + + default + 0 + + + external + + + jbod1 + 1024 + + + jbod2 + 1024000000 + + + + diff --git a/dbms/tests/integration/test_multiple_disks/test.py b/dbms/tests/integration/test_multiple_disks/test.py index 4ee337229c9..d0e2cb6ef19 100644 --- a/dbms/tests/integration/test_multiple_disks/test.py +++ b/dbms/tests/integration/test_multiple_disks/test.py @@ -1,8 +1,9 @@ -import time +import json import pytest import random +import re import string -import json +import time from multiprocessing.dummy import Pool from helpers.client import QueryRuntimeException from helpers.cluster import ClickHouseCluster @@ -193,6 +194,28 @@ def get_random_string(length): def get_used_disks_for_table(node, table_name): return node.query("select disk_name from system.parts where table == '{}' and active=1 order by modification_time".format(table_name)).strip().split('\n') +def test_no_warning_about_zero_max_data_part_size(start_cluster): + def get_log(node): + return node.exec_in_container(["bash", "-c", "cat /var/log/clickhouse-server/clickhouse-server.log"]) + + for node in (node1, node2): + node.query(""" + CREATE TABLE default.test_warning_table ( + s String + ) ENGINE = MergeTree + ORDER BY tuple() + SETTINGS storage_policy='small_jbod_with_external' + """) + node.query(""" + DROP TABLE default.test_warning_table + """) + log = get_log(node) + assert not re.search("Warning.*Volume.*special_warning_zero_volume", log) + assert not re.search("Warning.*Volume.*special_warning_default_volume", log) + assert re.search("Warning.*Volume.*special_warning_small_volume", log) + assert not re.search("Warning.*Volume.*special_warning_big_volume", log) + + @pytest.mark.parametrize("name,engine", [ ("mt_on_jbod","MergeTree()"), ("replicated_mt_on_jbod","ReplicatedMergeTree('/clickhouse/replicated_mt_on_jbod', '1')",), From 20093fa065862af691e8a9df42df544a493d9d1e Mon Sep 17 00:00:00 2001 From: chertus Date: Wed, 23 Oct 2019 16:59:03 +0300 Subject: [PATCH 13/25] extract more logic out of QueryNormalizer --- dbms/src/Interpreters/ActionsVisitor.cpp | 2 +- .../Interpreters/CrossToInnerJoinVisitor.cpp | 2 +- .../ExecuteScalarSubqueriesVisitor.cpp | 2 +- dbms/src/Interpreters/ExpressionAnalyzer.cpp | 2 +- .../MarkTableIdentifiersVisitor.cpp | 47 +++++++++++++++++++ .../MarkTableIdentifiersVisitor.h | 33 +++++++++++++ .../PredicateExpressionsOptimizer.cpp | 4 ++ dbms/src/Interpreters/QueryNormalizer.cpp | 43 +---------------- dbms/src/Interpreters/QueryNormalizer.h | 17 +------ dbms/src/Interpreters/SyntaxAnalyzer.cpp | 32 +++++++++++++ dbms/src/Interpreters/misc.h | 16 +++++++ dbms/src/Storages/MergeTree/KeyCondition.cpp | 2 +- .../MergeTreeIndexConditionBloomFilter.cpp | 2 +- .../MergeTree/MergeTreeIndexFullText.cpp | 2 +- .../MergeTree/MergeTreeWhereOptimizer.cpp | 2 +- 15 files changed, 142 insertions(+), 66 deletions(-) create mode 100644 dbms/src/Interpreters/MarkTableIdentifiersVisitor.cpp create mode 100644 dbms/src/Interpreters/MarkTableIdentifiersVisitor.h create mode 100644 dbms/src/Interpreters/misc.h diff --git a/dbms/src/Interpreters/ActionsVisitor.cpp b/dbms/src/Interpreters/ActionsVisitor.cpp index c587d1826e1..3b3f1ddde63 100644 --- a/dbms/src/Interpreters/ActionsVisitor.cpp +++ b/dbms/src/Interpreters/ActionsVisitor.cpp @@ -29,7 +29,7 @@ #include #include -#include +#include #include #include #include diff --git a/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp b/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp index 94b38b2c991..21f6610aa82 100644 --- a/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp +++ b/dbms/src/Interpreters/CrossToInnerJoinVisitor.cpp @@ -4,7 +4,7 @@ #include #include #include -#include // for functionIsInOperator +#include #include #include #include diff --git a/dbms/src/Interpreters/ExecuteScalarSubqueriesVisitor.cpp b/dbms/src/Interpreters/ExecuteScalarSubqueriesVisitor.cpp index 59f7f46be70..e2666993da8 100644 --- a/dbms/src/Interpreters/ExecuteScalarSubqueriesVisitor.cpp +++ b/dbms/src/Interpreters/ExecuteScalarSubqueriesVisitor.cpp @@ -6,7 +6,7 @@ #include #include -#include +#include #include #include #include diff --git a/dbms/src/Interpreters/ExpressionAnalyzer.cpp b/dbms/src/Interpreters/ExpressionAnalyzer.cpp index 14849763ef3..f694f74989a 100644 --- a/dbms/src/Interpreters/ExpressionAnalyzer.cpp +++ b/dbms/src/Interpreters/ExpressionAnalyzer.cpp @@ -54,7 +54,7 @@ #include #include #include -#include +#include #include diff --git a/dbms/src/Interpreters/MarkTableIdentifiersVisitor.cpp b/dbms/src/Interpreters/MarkTableIdentifiersVisitor.cpp new file mode 100644 index 00000000000..f110e0ba2df --- /dev/null +++ b/dbms/src/Interpreters/MarkTableIdentifiersVisitor.cpp @@ -0,0 +1,47 @@ +#include +#include +#include +#include +#include +#include +#include + +namespace DB +{ + +bool MarkTableIdentifiersMatcher::needChildVisit(ASTPtr & node, const ASTPtr & child) +{ + if (child->as()) + return false; + if (node->as()) + return false; + return true; +} + +void MarkTableIdentifiersMatcher::visit(ASTPtr & ast, Data & data) +{ + if (auto * node_func = ast->as()) + visit(*node_func, ast, data); + else if (auto * node_table = ast->as()) + visit(*node_table, ast, data); +} + +void MarkTableIdentifiersMatcher::visit(ASTTableExpression & table, ASTPtr &, Data &) +{ + if (table.database_and_table_name) + setIdentifierSpecial(table.database_and_table_name); +} + +void MarkTableIdentifiersMatcher::visit(const ASTFunction & func, ASTPtr &, Data & data) +{ + /// `IN t` can be specified, where t is a table, which is equivalent to `IN (SELECT * FROM t)`. + if (functionIsInOrGlobalInOperator(func.name)) + { + auto & ast = func.arguments->children.at(1); + if (auto opt_name = tryGetIdentifierName(ast)) + if (!data.aliases.count(*opt_name)) + setIdentifierSpecial(ast); + } +} + +} diff --git a/dbms/src/Interpreters/MarkTableIdentifiersVisitor.h b/dbms/src/Interpreters/MarkTableIdentifiersVisitor.h new file mode 100644 index 00000000000..f882f322bcf --- /dev/null +++ b/dbms/src/Interpreters/MarkTableIdentifiersVisitor.h @@ -0,0 +1,33 @@ +#pragma once + +#include +#include +#include + +namespace DB +{ + +class ASTFunction; +struct ASTTableExpression; + +class MarkTableIdentifiersMatcher +{ +public: + using Visitor = InDepthNodeVisitor; + + struct Data + { + const Aliases & aliases; + }; + + static bool needChildVisit(ASTPtr & node, const ASTPtr & child); + static void visit(ASTPtr & ast, Data & data); + +private: + static void visit(ASTTableExpression & table, ASTPtr &, Data &); + static void visit(const ASTFunction & func, ASTPtr &, Data &); +}; + +using MarkTableIdentifiersVisitor = MarkTableIdentifiersMatcher::Visitor; + +} diff --git a/dbms/src/Interpreters/PredicateExpressionsOptimizer.cpp b/dbms/src/Interpreters/PredicateExpressionsOptimizer.cpp index 2a307c6ed7f..27772b8fc94 100644 --- a/dbms/src/Interpreters/PredicateExpressionsOptimizer.cpp +++ b/dbms/src/Interpreters/PredicateExpressionsOptimizer.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -412,6 +413,9 @@ ASTs PredicateExpressionsOptimizer::getSelectQueryProjectionColumns(ASTPtr & ast QueryAliasesVisitor::Data query_aliases_data{aliases}; QueryAliasesVisitor(query_aliases_data).visit(ast); + MarkTableIdentifiersVisitor::Data mark_tables_data{aliases}; + MarkTableIdentifiersVisitor(mark_tables_data).visit(ast); + QueryNormalizer::Data normalizer_data(aliases, settings); QueryNormalizer(normalizer_data).visit(ast); diff --git a/dbms/src/Interpreters/QueryNormalizer.cpp b/dbms/src/Interpreters/QueryNormalizer.cpp index e109e4a63fd..9d6d28a68f6 100644 --- a/dbms/src/Interpreters/QueryNormalizer.cpp +++ b/dbms/src/Interpreters/QueryNormalizer.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include namespace DB @@ -63,34 +62,6 @@ private: }; -void QueryNormalizer::visit(ASTFunction & node, const ASTPtr &, Data & data) -{ - auto & aliases = data.aliases; - String & func_name = node.name; - ASTPtr & func_arguments = node.arguments; - - /// `IN t` can be specified, where t is a table, which is equivalent to `IN (SELECT * FROM t)`. - if (functionIsInOrGlobalInOperator(func_name)) - { - auto & ast = func_arguments->children.at(1); - if (auto opt_name = tryGetIdentifierName(ast)) - if (!aliases.count(*opt_name)) - setIdentifierSpecial(ast); - } - - /// Special cases for count function. - String func_name_lowercase = Poco::toLower(func_name); - if (startsWith(func_name_lowercase, "count")) - { - /// Select implementation of countDistinct based on settings. - /// Important that it is done as query rewrite. It means rewritten query - /// will be sent to remote servers during distributed query execution, - /// and on all remote servers, function implementation will be same. - if (endsWith(func_name, "Distinct") && func_name_lowercase == "countdistinct") - func_name = data.settings.count_distinct_implementation; - } -} - void QueryNormalizer::visit(ASTIdentifier & node, ASTPtr & ast, Data & data) { auto & current_asts = data.current_asts; @@ -144,16 +115,8 @@ void QueryNormalizer::visit(ASTIdentifier & node, ASTPtr & ast, Data & data) } } -/// mark table identifiers as 'not columns' void QueryNormalizer::visit(ASTTablesInSelectQueryElement & node, const ASTPtr &, Data & data) { - /// mark table Identifiers as 'not a column' - if (node.table_expression) - { - auto & expr = node.table_expression->as(); - setIdentifierSpecial(expr.database_and_table_name); - } - /// normalize JOIN ON section if (node.table_join) { @@ -177,7 +140,6 @@ void QueryNormalizer::visit(ASTSelectQuery & select, const ASTPtr &, Data & data if (needVisitChild(child)) visit(child, data); -#if 1 /// TODO: legacy? /// If the WHERE clause or HAVING consists of a single alias, the reference must be replaced not only in children, /// but also in where_expression and having_expression. if (select.prewhere()) @@ -186,7 +148,6 @@ void QueryNormalizer::visit(ASTSelectQuery & select, const ASTPtr &, Data & data visit(select.refWhere(), data); if (select.having()) visit(select.refHaving(), data); -#endif } /// Don't go into subqueries. @@ -243,9 +204,7 @@ void QueryNormalizer::visit(ASTPtr & ast, Data & data) data.current_alias = my_alias; } - if (auto * node_func = ast->as()) - visit(*node_func, ast, data); - else if (auto * node_id = ast->as()) + if (auto * node_id = ast->as()) visit(*node_id, ast, data); else if (auto * node_tables = ast->as()) visit(*node_tables, ast, data); diff --git a/dbms/src/Interpreters/QueryNormalizer.h b/dbms/src/Interpreters/QueryNormalizer.h index 6d6fea86e44..b842ae3f018 100644 --- a/dbms/src/Interpreters/QueryNormalizer.h +++ b/dbms/src/Interpreters/QueryNormalizer.h @@ -2,25 +2,13 @@ #include -#include #include #include namespace DB { -inline bool functionIsInOperator(const String & name) -{ - return name == "in" || name == "notIn"; -} - -inline bool functionIsInOrGlobalInOperator(const String & name) -{ - return functionIsInOperator(name) || name == "globalIn" || name == "globalNotIn"; -} - class ASTSelectQuery; -class ASTFunction; class ASTIdentifier; struct ASTTablesInSelectQueryElement; class Context; @@ -33,13 +21,11 @@ class QueryNormalizer { const UInt64 max_ast_depth; const UInt64 max_expanded_ast_elements; - const String count_distinct_implementation; template ExtractedSettings(const T & settings) : max_ast_depth(settings.max_ast_depth), - max_expanded_ast_elements(settings.max_expanded_ast_elements), - count_distinct_implementation(settings.count_distinct_implementation) + max_expanded_ast_elements(settings.max_expanded_ast_elements) {} }; @@ -80,7 +66,6 @@ private: static void visit(ASTPtr & query, Data & data); static void visit(ASTIdentifier &, ASTPtr &, Data &); - static void visit(ASTFunction &, const ASTPtr &, Data &); static void visit(ASTTablesInSelectQueryElement &, const ASTPtr &, Data &); static void visit(ASTSelectQuery &, const ASTPtr &, Data &); diff --git a/dbms/src/Interpreters/SyntaxAnalyzer.cpp b/dbms/src/Interpreters/SyntaxAnalyzer.cpp index 228aea0b2f2..1f7a7abaec3 100644 --- a/dbms/src/Interpreters/SyntaxAnalyzer.cpp +++ b/dbms/src/Interpreters/SyntaxAnalyzer.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -72,6 +73,26 @@ namespace using LogAST = DebugASTLog; /// set to true to enable logs +/// Select implementation of countDistinct based on settings. +/// Important that it is done as query rewrite. It means rewritten query +/// will be sent to remote servers during distributed query execution, +/// and on all remote servers, function implementation will be same. +struct CustomizeFunctionsData +{ + using TypeToVisit = ASTFunction; + + const String & count_distinct; + + void visit(ASTFunction & func, ASTPtr &) + { + if (Poco::toLower(func.name) == "countdistinct") + func.name = count_distinct; + } +}; + +using CustomizeFunctionsMatcher = OneTypeMatcher; +using CustomizeFunctionsVisitor = InDepthNodeVisitor; + /// Add columns from storage to source_columns list. void collectSourceColumns(const ColumnsDescription & columns, NamesAndTypesList & source_columns, bool add_virtuals) @@ -850,6 +871,11 @@ SyntaxAnalyzerResultPtr SyntaxAnalyzer::analyze( LogicalExpressionsOptimizer(select_query, settings.optimize_min_equality_disjunction_chain_length.value).perform(); } + { + CustomizeFunctionsVisitor::Data data{settings.count_distinct_implementation}; + CustomizeFunctionsVisitor(data).visit(query); + } + /// Creates a dictionary `aliases`: alias -> ASTPtr { LogAST log; @@ -857,6 +883,12 @@ SyntaxAnalyzerResultPtr SyntaxAnalyzer::analyze( QueryAliasesVisitor(query_aliases_data, log.stream()).visit(query); } + /// Mark table ASTIdentifiers with not a column marker + { + MarkTableIdentifiersVisitor::Data data{result.aliases}; + MarkTableIdentifiersVisitor(data).visit(query); + } + /// Common subexpression elimination. Rewrite rules. { QueryNormalizer::Data normalizer_data(result.aliases, context.getSettingsRef()); diff --git a/dbms/src/Interpreters/misc.h b/dbms/src/Interpreters/misc.h new file mode 100644 index 00000000000..d5e2894bb4c --- /dev/null +++ b/dbms/src/Interpreters/misc.h @@ -0,0 +1,16 @@ +#pragma once + +namespace DB +{ + +inline bool functionIsInOperator(const std::string & name) +{ + return name == "in" || name == "notIn"; +} + +inline bool functionIsInOrGlobalInOperator(const std::string & name) +{ + return functionIsInOperator(name) || name == "globalIn" || name == "globalNotIn"; +} + +} diff --git a/dbms/src/Storages/MergeTree/KeyCondition.cpp b/dbms/src/Storages/MergeTree/KeyCondition.cpp index b3e4c776605..a2789fe3063 100644 --- a/dbms/src/Storages/MergeTree/KeyCondition.cpp +++ b/dbms/src/Storages/MergeTree/KeyCondition.cpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/dbms/src/Storages/MergeTree/MergeTreeIndexConditionBloomFilter.cpp b/dbms/src/Storages/MergeTree/MergeTreeIndexConditionBloomFilter.cpp index 856354959f9..147071fc493 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeIndexConditionBloomFilter.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeIndexConditionBloomFilter.cpp @@ -1,5 +1,5 @@ #include -#include +#include #include #include #include diff --git a/dbms/src/Storages/MergeTree/MergeTreeIndexFullText.cpp b/dbms/src/Storages/MergeTree/MergeTreeIndexFullText.cpp index 264c91cd890..da3f1df8130 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeIndexFullText.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeIndexFullText.cpp @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/dbms/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp b/dbms/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp index a772e0a204b..dcca5baf311 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeWhereOptimizer.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include From d1a19d26e85fce6d50fa56a42dff4b50604ca343 Mon Sep 17 00:00:00 2001 From: Konstantin Podshumok Date: Wed, 23 Oct 2019 20:33:40 +0300 Subject: [PATCH 14/25] Remove hardcoded paths in unwind target In most cases they match defaults now, but it is too hard to override when one needs to (alternative builds) --- contrib/libunwind-cmake/CMakeLists.txt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/contrib/libunwind-cmake/CMakeLists.txt b/contrib/libunwind-cmake/CMakeLists.txt index f09d0979692..7901a990b85 100644 --- a/contrib/libunwind-cmake/CMakeLists.txt +++ b/contrib/libunwind-cmake/CMakeLists.txt @@ -30,9 +30,4 @@ target_include_directories(unwind SYSTEM BEFORE PUBLIC $ Date: Fri, 18 Oct 2019 00:33:26 +0300 Subject: [PATCH 15/25] Fix INSERT into Distributed non local node with MATERIALIZED columns Previous patch e527def18a1bbe5fba0920b7747e9c556fd21ff5 ("Fix INSERT into Distributed() table with MATERIALIZED column") fixes it only for cases when the node is local, i.e. direct insert. This patch address the problem when the node is not local (`is_local == false`), by erasing materialized columns on INSERT into Distributed. And this patch fixes two cases, depends on `insert_distributed_sync` setting: - `insert_distributed_sync=0` ``` Not found column value in block. There are only columns: date. Stack trace: 2. 0x7ffff7be92e0 DB::Exception::Exception() dbms/src/Common/Exception.h:27 3. 0x7fffec5d6cf6 DB::Block::getByName(...) dbms/src/Core/Block.cpp:187 4. 0x7fffec2fe067 DB::NativeBlockInputStream::readImpl() dbms/src/DataStreams/NativeBlockInputStream.cpp:159 5. 0x7fffec2d223f DB::IBlockInputStream::read() dbms/src/DataStreams/IBlockInputStream.cpp:61 6. 0x7ffff7c6d40d DB::TCPHandler::receiveData() dbms/programs/server/TCPHandler.cpp:971 7. 0x7ffff7c6cc1d DB::TCPHandler::receivePacket() dbms/programs/server/TCPHandler.cpp:855 8. 0x7ffff7c6a1ef DB::TCPHandler::readDataNext(unsigned long const&, int const&) dbms/programs/server/TCPHandler.cpp:406 9. 0x7ffff7c6a41b DB::TCPHandler::readData(DB::Settings const&) dbms/programs/server/TCPHandler.cpp:437 10. 0x7ffff7c6a5d9 DB::TCPHandler::processInsertQuery(DB::Settings const&) dbms/programs/server/TCPHandler.cpp:464 11. 0x7ffff7c687b5 DB::TCPHandler::runImpl() dbms/programs/server/TCPHandler.cpp:257 ``` - `insert_distributed_sync=1` ``` 2019.10.18 13:23:22.114578 [ 44 ] {a78f669f-0b08-4337-abf8-d31e958f6d12} executeQuery: Code: 171, e.displayText() = DB::Exception: Block structure mismatch in RemoteBlockOutputStream stream: different number of columns: date Date UInt16(size = 1), value Date UInt16(size = 1) date Date UInt16(size = 0): Insertion status: Wrote 1 blocks and 0 rows on shard 0 replica 0, 127.0.0.1:59000 (average 0 ms per block) Wrote 0 blocks and 0 rows on shard 1 replica 0, 127.0.0.2:59000 (average 2 ms per block) (version 19.16.1.1) (from [::1]:3624) (in query: INSERT INTO distributed_00952 VALUES ), Stack trace: 2. 0x7ffff7be92e0 DB::Exception::Exception() dbms/src/Common/Exception.h:27 3. 0x7fffec5da4e9 DB::checkBlockStructure(...)::{...}::operator()(...) const dbms/src/Core/Block.cpp:460 4. 0x7fffec5da671 void DB::checkBlockStructure(...) dbms/src/Core/Block.cpp:467 5. 0x7fffec5d8d58 DB::assertBlocksHaveEqualStructure(...) dbms/src/Core/Block.cpp:515 6. 0x7fffec326630 DB::RemoteBlockOutputStream::write(DB::Block const&) dbms/src/DataStreams/RemoteBlockOutputStream.cpp:68 7. 0x7fffe98bd154 DB::DistributedBlockOutputStream::runWritingJob(DB::DistributedBlockOutputStream::JobReplica&, DB::Block const&)::{lambda()#1}::operator()() const dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp:280 ```` Fixes: #7365 Fixes: #5429 Refs: #6891 --- .../DistributedBlockOutputStream.cpp | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp b/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp index 5dce68ec381..f295b6d4d3d 100644 --- a/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp +++ b/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp @@ -81,12 +81,25 @@ void DistributedBlockOutputStream::writePrefix() void DistributedBlockOutputStream::write(const Block & block) { - if (insert_sync) - writeSync(block); - else - writeAsync(block); -} + Block ordinary_block{ block }; + /* They are added by the AddingDefaultBlockOutputStream, and we will get + * different number of columns eventually */ + for (const auto & col : storage.getColumns().getMaterialized()) + if (ordinary_block.has(col.name)) + { + ordinary_block.erase(col.name); + LOG_DEBUG(log, storage.getTableName() + << ": column " + col.name + " will be removed, " + << "because it is MATERIALIZED"); + } + + + if (insert_sync) + writeSync(ordinary_block); + else + writeAsync(ordinary_block); +} void DistributedBlockOutputStream::writeAsync(const Block & block) { From 80cf86f100bfaa165d252ef875558e4f130cd170 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Fri, 18 Oct 2019 10:22:43 +0300 Subject: [PATCH 16/25] Cover INSERT into Distributed with MATERIALIZED columns and !is_local node I guess that adding new cluster into server-test.xml is not required, but it won't harm. --- dbms/programs/server/config.xml | 16 ++++++++- ...ributed_with_materialized_column.reference | 11 +++++++ ...o_distributed_with_materialized_column.sql | 33 +++++++++++++++++-- dbms/tests/server-test.xml | 14 ++++++++ 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/dbms/programs/server/config.xml b/dbms/programs/server/config.xml index c8d33922167..6e9bb527c97 100644 --- a/dbms/programs/server/config.xml +++ b/dbms/programs/server/config.xml @@ -180,7 +180,21 @@ 9000 - + + + + + 127.0.0.1 + 9000 + + + + + 127.0.0.2 + 9000 + + + diff --git a/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.reference b/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.reference index b01acf34583..11b42f40c7a 100644 --- a/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.reference +++ b/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.reference @@ -1,3 +1,14 @@ +insert_distributed_sync=0 2018-08-01 2018-08-01 2018-08-01 2017-08-01 +2018-08-01 2017-08-01 +2018-08-01 +2018-08-01 2017-08-01 +insert_distributed_sync=1 +2018-08-01 +2018-08-01 +2018-08-01 2017-08-01 +2018-08-01 2017-08-01 +2018-08-01 +2018-08-01 2017-08-01 diff --git a/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.sql b/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.sql index 9e5bc3cbdf9..6b70d927204 100644 --- a/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.sql +++ b/dbms/tests/queries/0_stateless/00952_insert_into_distributed_with_materialized_column.sql @@ -1,15 +1,42 @@ DROP TABLE IF EXISTS local_00952; DROP TABLE IF EXISTS distributed_00952; -CREATE TABLE local_00952 (date Date, value Date MATERIALIZED toDate('2017-08-01')) ENGINE = MergeTree(date, date, 8192); -CREATE TABLE distributed_00952 AS local_00952 ENGINE = Distributed('test_shard_localhost', currentDatabase(), local_00952, rand()); +-- +-- insert_distributed_sync=0 +-- +SELECT 'insert_distributed_sync=0'; +SET insert_distributed_sync=0; -SET insert_distributed_sync=1; +CREATE TABLE local_00952 (date Date, value Date MATERIALIZED toDate('2017-08-01')) ENGINE = MergeTree(date, date, 8192); +CREATE TABLE distributed_00952 AS local_00952 ENGINE = Distributed('test_cluster_two_shards', currentDatabase(), local_00952, rand()); INSERT INTO distributed_00952 VALUES ('2018-08-01'); +SYSTEM FLUSH DISTRIBUTED distributed_00952; + SELECT * FROM distributed_00952; +SELECT date, value FROM distributed_00952; SELECT * FROM local_00952; SELECT date, value FROM local_00952; DROP TABLE distributed_00952; DROP TABLE local_00952; + +-- +-- insert_distributed_sync=1 +-- +SELECT 'insert_distributed_sync=1'; +SET insert_distributed_sync=1; + +CREATE TABLE local_00952 (date Date, value Date MATERIALIZED toDate('2017-08-01')) ENGINE = MergeTree(date, date, 8192); +CREATE TABLE distributed_00952 AS local_00952 ENGINE = Distributed('test_cluster_two_shards', currentDatabase(), local_00952, rand()); + +INSERT INTO distributed_00952 VALUES ('2018-08-01'); + +SELECT * FROM distributed_00952; +SELECT date, value FROM distributed_00952; +SELECT * FROM local_00952; +SELECT date, value FROM local_00952; + +DROP TABLE distributed_00952; +DROP TABLE local_00952; + diff --git a/dbms/tests/server-test.xml b/dbms/tests/server-test.xml index d68cbca53c1..d9e547b4d55 100644 --- a/dbms/tests/server-test.xml +++ b/dbms/tests/server-test.xml @@ -75,6 +75,20 @@ + + + + 127.0.0.1 + 59000 + + + + + 127.0.0.2 + 59000 + + + From e8e5cefc354de553f3ff7fd0485778aeab82aa95 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Thu, 24 Oct 2019 08:58:06 +0300 Subject: [PATCH 17/25] Fixed integration test for #7414. --- .../integration/test_multiple_disks/test.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/dbms/tests/integration/test_multiple_disks/test.py b/dbms/tests/integration/test_multiple_disks/test.py index d0e2cb6ef19..9c69a99ef1b 100644 --- a/dbms/tests/integration/test_multiple_disks/test.py +++ b/dbms/tests/integration/test_multiple_disks/test.py @@ -129,6 +129,38 @@ def test_system_tables(start_cluster): "max_data_part_size": "20971520", "move_factor": 0.1, }, + { + "policy_name": "special_warning_policy", + "volume_name": "special_warning_zero_volume", + "volume_priority": "1", + "disks": ["default"], + "max_data_part_size": "0", + "move_factor": 0.1, + }, + { + "policy_name": "special_warning_policy", + "volume_name": "special_warning_default_volume", + "volume_priority": "2", + "disks": ["external"], + "max_data_part_size": "0", + "move_factor": 0.1, + }, + { + "policy_name": "special_warning_policy", + "volume_name": "special_warning_small_volume", + "volume_priority": "3", + "disks": ["jbod1"], + "max_data_part_size": "1024", + "move_factor": 0.1, + }, + { + "policy_name": "special_warning_policy", + "volume_name": "special_warning_big_volume", + "volume_priority": "4", + "disks": ["jbod2"], + "max_data_part_size": "1024000000", + "move_factor": 0.1, + }, ] clickhouse_policies_data = json.loads(node1.query("SELECT * FROM system.storage_policies WHERE policy_name != 'default' FORMAT JSON"))["data"] From 3d2eab75353960b7ed0c03e037540ca5d4a05e0c Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Thu, 24 Oct 2019 09:49:58 +0200 Subject: [PATCH 18/25] Add PARTITION ID to OPTIMIZE documentation --- docs/en/query_language/misc.md | 4 ++-- docs/ru/query_language/misc.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/en/query_language/misc.md b/docs/en/query_language/misc.md index 22d67044619..9bcac5cdbfd 100644 --- a/docs/en/query_language/misc.md +++ b/docs/en/query_language/misc.md @@ -174,7 +174,7 @@ Changes already made by the mutation are not rolled back. ## OPTIMIZE {#misc_operations-optimize} ```sql -OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition] [FINAL] +OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] ``` This query tries to initialize an unscheduled merge of data parts for tables with a table engine from the [MergeTree](../operations/table_engines/mergetree.md) family. Other kinds of table engines aren't supported. @@ -182,7 +182,7 @@ This query tries to initialize an unscheduled merge of data parts for tables wit When `OPTIMIZE` is used with the [ReplicatedMergeTree](../operations/table_engines/replication.md) family of table engines, ClickHouse creates a task for merging and waits for execution on all nodes (if the `replication_alter_partitions_sync` setting is enabled). - If `OPTIMIZE` doesn't perform a merge for any reason, it doesn't notify the client. To enable notifications, use the [optimize_throw_if_noop](../operations/settings/settings.md#setting-optimize_throw_if_noop) setting. -- If you specify a `PARTITION`, only the specified partition is optimized. +- If you specify a `PARTITION`, only the specified partition is optimized. [How to set partition expression](alter.md#alter-how-to-specify-part-expr). - If you specify `FINAL`, optimization is performed even when all the data is already in one part. !!! warning "Warning" diff --git a/docs/ru/query_language/misc.md b/docs/ru/query_language/misc.md index 00cb0e7fd93..ce73a5aafdb 100644 --- a/docs/ru/query_language/misc.md +++ b/docs/ru/query_language/misc.md @@ -173,7 +173,7 @@ KILL MUTATION WHERE database = 'default' AND table = 'table' AND mutation_id = ' ## OPTIMIZE {#misc_operations-optimize} ```sql -OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition] [FINAL] +OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition | PARTITION ID 'partition_id'] [FINAL] ``` Запрос пытается запустить внеплановый мёрж кусков данных для таблиц семейства [MergeTree](../operations/table_engines/mergetree.md). Другие движки таблиц не поддерживаются. @@ -181,7 +181,7 @@ OPTIMIZE TABLE [db.]name [ON CLUSTER cluster] [PARTITION partition] [FINAL] Если `OPTIMIZE` применяется к таблицам семейства [ReplicatedMergeTree](../operations/table_engines/replication.md), ClickHouse создаёт задачу на мёрж и ожидает её исполнения на всех узлах (если активирована настройка `replication_alter_partitions_sync`). - Если `OPTIMIZE` не выполняет мёрж по любой причине, ClickHouse не оповещает об этом клиента. Чтобы включить оповещения, используйте настройку [optimize_throw_if_noop](../operations/settings/settings.md#setting-optimize_throw_if_noop). -- Если указать `PARTITION`, то оптимизация выполняется только для указанной партиции. +- Если указать `PARTITION`, то оптимизация выполняется только для указанной партиции. [Как задавать имя партиции в запросах](alter.md#alter-how-to-specify-part-expr). - Если указать `FINAL`, то оптимизация выполняется даже в том случае, если все данные уже лежат в одном куске. !!! warning "Внимание" From 3debdc211904eb0b5fe6cca1a4af99d8db635892 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Thu, 24 Oct 2019 11:52:33 +0300 Subject: [PATCH 19/25] Added integration tests for ALTER MOVE PARTITION and fixed minor things. --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 54 +++++------- .../integration/test_multiple_disks/test.py | 87 ++++++++++++++++++- 2 files changed, 109 insertions(+), 32 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index f530c537671..8cee32cb004 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -2723,19 +2723,18 @@ void MergeTreeData::movePartitionToDisk(const ASTPtr & partition, const String & if (!disk) throw Exception("Disk " + name + " does not exists on policy " + storage_policy->getName(), ErrorCodes::UNKNOWN_DISK); - parts.erase(std::remove_if(parts.begin(), parts.end(), [&](auto part_ptr) { - return part_ptr->disk->getName() == disk->getName(); - }), parts.end()); + parts.erase(std::remove_if(parts.begin(), parts.end(), [&](auto part_ptr) + { + return part_ptr->disk->getName() == disk->getName(); + }), parts.end()); - if (!parts.empty()) + if (parts.empty()) { - if (!movePartsToSpace(parts, std::static_pointer_cast(disk))) - throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); - } - else - { - LOG_DEBUG(log, "No parts of partition " << partition_id << " to move to disk " << disk->getName()); + throw Exception("All parts of partition " + partition_id + " are already on disk '" + disk->getName() + "'", ErrorCodes::UNKNOWN_DISK); } + + if (!movePartsToSpace(parts, std::static_pointer_cast(disk))) + throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); } @@ -2763,32 +2762,25 @@ void MergeTreeData::movePartitionToVolume(const ASTPtr & partition, const String if (!volume) throw Exception("Volume " + name + " does not exists on policy " + storage_policy->getName(), ErrorCodes::UNKNOWN_DISK); - for (const auto & part : parts) - for (const auto & disk : volume->disks) - if (part->disk->getName() == disk->getName()) - throw Exception("Part " + part->name + " already on volume '" + name + "'", ErrorCodes::UNKNOWN_DISK); - - parts.erase(std::remove_if(parts.begin(), parts.end(), [&](auto part_ptr) { - for (const auto & disk : volume->disks) + parts.erase(std::remove_if(parts.begin(), parts.end(), [&](auto part_ptr) { - if (part_ptr->disk->getName() == disk->getName()) + for (const auto & disk : volume->disks) { - return true; + if (part_ptr->disk->getName() == disk->getName()) + { + return true; + } } - } - return false; - }), parts.end()); + return false; + }), parts.end()); + if (parts.empty()) + { + throw Exception("All parts of partition " + partition_id + " are already on volume '" + volume->getName() + "'", ErrorCodes::UNKNOWN_DISK); + } - if (!parts.empty()) - { - if (!movePartsToSpace(parts, std::static_pointer_cast(volume))) - throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); - } - else - { - LOG_DEBUG(log, "No parts of partition " << partition_id << " to move to volume " << volume->getName()); - } + if (!movePartsToSpace(parts, std::static_pointer_cast(volume))) + throw Exception("Cannot move parts because moves are manually disabled.", ErrorCodes::ABORTED); } diff --git a/dbms/tests/integration/test_multiple_disks/test.py b/dbms/tests/integration/test_multiple_disks/test.py index 4ee337229c9..ed8ad699472 100644 --- a/dbms/tests/integration/test_multiple_disks/test.py +++ b/dbms/tests/integration/test_multiple_disks/test.py @@ -462,7 +462,7 @@ def test_alter_move(start_cluster, name, engine): node1.query("INSERT INTO {} VALUES(toDate('2019-04-10'), 42)".format(name)) node1.query("INSERT INTO {} VALUES(toDate('2019-04-11'), 43)".format(name)) used_disks = get_used_disks_for_table(node1, name) - assert all(d.startswith("jbod") for d in used_disks), "All writes shoud go to jbods" + assert all(d.startswith("jbod") for d in used_disks), "All writes should go to jbods" first_part = node1.query("SELECT name FROM system.parts WHERE table = '{}' and active = 1 ORDER BY modification_time LIMIT 1".format(name)).strip() @@ -498,6 +498,91 @@ def test_alter_move(start_cluster, name, engine): finally: node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) + +@pytest.mark.parametrize("volume_or_disk", [ + "DISK", + "VOLUME" +]) +def test_alter_move_half_of_partition(start_cluster, volume_or_disk): + name = "alter_move_half_of_partition" + engine = "MergeTree()" + try: + node1.query(""" + CREATE TABLE {name} ( + EventDate Date, + number UInt64 + ) ENGINE = {engine} + ORDER BY tuple() + PARTITION BY toYYYYMM(EventDate) + SETTINGS storage_policy='jbods_with_external' + """.format(name=name, engine=engine)) + + node1.query("SYSTEM STOP MERGES {}".format(name)) + + node1.query("INSERT INTO {} VALUES(toDate('2019-03-15'), 65)".format(name)) + node1.query("INSERT INTO {} VALUES(toDate('2019-03-16'), 42)".format(name)) + used_disks = get_used_disks_for_table(node1, name) + assert all(d.startswith("jbod") for d in used_disks), "All writes should go to jbods" + + time.sleep(1) + parts = node1.query("SELECT name FROM system.parts WHERE table = '{}' and active = 1".format(name)).splitlines() + assert len(parts) == 2 + + node1.query("ALTER TABLE {} MOVE PART '{}' TO VOLUME 'external'".format(name, parts[0])) + disks = node1.query("SELECT disk_name FROM system.parts WHERE table = '{}' and name = '{}' and active = 1".format(name, parts[0])).splitlines() + assert disks == ["external"] + + time.sleep(1) + node1.query("ALTER TABLE {} MOVE PARTITION 201903 TO {volume_or_disk} 'external'".format(name, volume_or_disk=volume_or_disk)) + disks = node1.query("SELECT disk_name FROM system.parts WHERE table = '{}' and partition = '201903' and active = 1".format(name)).splitlines() + assert disks == ["external"]*2 + + assert node1.query("SELECT COUNT() FROM {}".format(name)) == "2\n" + + finally: + node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) + + +@pytest.mark.parametrize("volume_or_disk", [ + "DISK", + "VOLUME" +]) +def test_alter_double_move_partition(start_cluster, volume_or_disk): + name = "alter_double_move_partition" + engine = "MergeTree()" + try: + node1.query(""" + CREATE TABLE {name} ( + EventDate Date, + number UInt64 + ) ENGINE = {engine} + ORDER BY tuple() + PARTITION BY toYYYYMM(EventDate) + SETTINGS storage_policy='jbods_with_external' + """.format(name=name, engine=engine)) + + node1.query("SYSTEM STOP MERGES {}".format(name)) + + node1.query("INSERT INTO {} VALUES(toDate('2019-03-15'), 65)".format(name)) + node1.query("INSERT INTO {} VALUES(toDate('2019-03-16'), 42)".format(name)) + used_disks = get_used_disks_for_table(node1, name) + assert all(d.startswith("jbod") for d in used_disks), "All writes should go to jbods" + + time.sleep(1) + node1.query("ALTER TABLE {} MOVE PARTITION 201903 TO {volume_or_disk} 'external'".format(name, volume_or_disk=volume_or_disk)) + disks = node1.query("SELECT disk_name FROM system.parts WHERE table = '{}' and partition = '201903' and active = 1".format(name)).splitlines() + assert disks == ["external"]*2 + + assert node1.query("SELECT COUNT() FROM {}".format(name)) == "2\n" + + time.sleep(1) + with pytest.raises(QueryRuntimeException): + node1.query("ALTER TABLE {} MOVE PARTITION 201903 TO {volume_or_disk} 'external'".format(name, volume_or_disk=volume_or_disk)) + + finally: + node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) + + def produce_alter_move(node, name): move_type = random.choice(["PART", "PARTITION"]) if move_type == "PART": From 255da8f5e03d0afcd16abc541ed83b715c367e19 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Thu, 24 Oct 2019 12:11:06 +0300 Subject: [PATCH 20/25] Fixed style. --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index 8cee32cb004..b08717e60e9 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -2730,7 +2730,7 @@ void MergeTreeData::movePartitionToDisk(const ASTPtr & partition, const String & if (parts.empty()) { - throw Exception("All parts of partition " + partition_id + " are already on disk '" + disk->getName() + "'", ErrorCodes::UNKNOWN_DISK); + throw Exception("All parts of partition '" + partition_id + "' are already on disk '" + disk->getName() + "'", ErrorCodes::UNKNOWN_DISK); } if (!movePartsToSpace(parts, std::static_pointer_cast(disk))) @@ -2776,7 +2776,7 @@ void MergeTreeData::movePartitionToVolume(const ASTPtr & partition, const String if (parts.empty()) { - throw Exception("All parts of partition " + partition_id + " are already on volume '" + volume->getName() + "'", ErrorCodes::UNKNOWN_DISK); + throw Exception("All parts of partition '" + partition_id + "' are already on volume '" + volume->getName() + "'", ErrorCodes::UNKNOWN_DISK); } if (!movePartsToSpace(parts, std::static_pointer_cast(volume))) From 050de71ef47fc06caa7ce029fd9eebe83413f806 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 24 Oct 2019 12:33:45 +0300 Subject: [PATCH 21/25] Update DistributedBlockOutputStream.cpp --- dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp b/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp index f295b6d4d3d..ee3ebfd9964 100644 --- a/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp +++ b/dbms/src/Storages/Distributed/DistributedBlockOutputStream.cpp @@ -86,6 +86,7 @@ void DistributedBlockOutputStream::write(const Block & block) /* They are added by the AddingDefaultBlockOutputStream, and we will get * different number of columns eventually */ for (const auto & col : storage.getColumns().getMaterialized()) + { if (ordinary_block.has(col.name)) { ordinary_block.erase(col.name); @@ -93,6 +94,7 @@ void DistributedBlockOutputStream::write(const Block & block) << ": column " + col.name + " will be removed, " << "because it is MATERIALIZED"); } + } if (insert_sync) From 64f158ff28b04ece575e881e7810618fa5d05e2b Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Thu, 24 Oct 2019 13:56:32 +0300 Subject: [PATCH 22/25] Fixed message for ALTER MOVE PART. --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index b08717e60e9..e3de86f5a78 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -2730,7 +2730,13 @@ void MergeTreeData::movePartitionToDisk(const ASTPtr & partition, const String & if (parts.empty()) { - throw Exception("All parts of partition '" + partition_id + "' are already on disk '" + disk->getName() + "'", ErrorCodes::UNKNOWN_DISK); + String no_parts_to_move_message; + if (moving_part) + no_parts_to_move_message = "Part '" + partition_id + "' is already on disk '" + disk->getName() + "'"; + else + no_parts_to_move_message = "All parts of partition '" + partition_id + "' are already on disk '" + disk->getName() + "'"; + + throw Exception(no_parts_to_move_message, ErrorCodes::UNKNOWN_DISK); } if (!movePartsToSpace(parts, std::static_pointer_cast(disk))) @@ -2776,7 +2782,13 @@ void MergeTreeData::movePartitionToVolume(const ASTPtr & partition, const String if (parts.empty()) { - throw Exception("All parts of partition '" + partition_id + "' are already on volume '" + volume->getName() + "'", ErrorCodes::UNKNOWN_DISK); + String no_parts_to_move_message; + if (moving_part) + no_parts_to_move_message = "Part '" + partition_id + "' is already on volume '" + volume->getName() + "'"; + else + no_parts_to_move_message = "All parts of partition '" + partition_id + "' are already on volume '" + volume->getName() + "'"; + + throw Exception(no_parts_to_move_message, ErrorCodes::UNKNOWN_DISK); } if (!movePartsToSpace(parts, std::static_pointer_cast(volume))) From c250db4922faa7cfa723cb8ecff9252585d6851b Mon Sep 17 00:00:00 2001 From: Ivan <5627721+abyss7@users.noreply.github.com> Date: Thu, 24 Oct 2019 15:56:30 +0300 Subject: [PATCH 23/25] Update Docker Image for Binary Packager (#7474) --- docker/packager/binary/Dockerfile | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/docker/packager/binary/Dockerfile b/docker/packager/binary/Dockerfile index d88a2767efd..c15de71e848 100644 --- a/docker/packager/binary/Dockerfile +++ b/docker/packager/binary/Dockerfile @@ -57,20 +57,28 @@ RUN apt-get update -y \ rename \ wget -# Build and install tools for cross-linking to Darwin - ENV CC=clang-8 ENV CXX=clang++-8 # libtapi is required to support .tbh format from recent MacOS SDKs RUN git clone https://github.com/tpoechtrager/apple-libtapi.git RUN cd apple-libtapi && INSTALLPREFIX=/cctools ./build.sh && ./install.sh +RUN rm -rf apple-libtapi +# Build and install tools for cross-linking to Darwin RUN git clone https://github.com/tpoechtrager/cctools-port.git RUN cd cctools-port/cctools && ./configure --prefix=/cctools --with-libtapi=/cctools --target=x86_64-apple-darwin && make install +RUN rm -rf cctools-port +# Download toolchain for Darwin +RUN mkdir -p /build/cmake/toolchain/darwin-x86_64 RUN wget https://github.com/phracker/MacOSX-SDKs/releases/download/10.14-beta4/MacOSX10.14.sdk.tar.xz -RUN tar xJf MacOSX10.14.sdk.tar.xz -C /cctools +RUN tar xJf MacOSX10.14.sdk.tar.xz -C /build/cmake/toolchain/darwin-x86_64 --strip-components=1 + +# Download toolchain for ARM +RUN mkdir -p /build/cmake/toolchain/linux-aarch64 +RUN wget "https://developer.arm.com/-/media/Files/downloads/gnu-a/8.3-2019.03/binrel/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz?revision=2e88a73f-d233-4f96-b1f4-d8b36e9bb0b9&la=en" -O gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz +RUN tar xJf gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz -C /build/cmake/toolchain/linux-aarch64 --strip-components=1 COPY build.sh / CMD ["/bin/bash", "/build.sh"] From f2028e901d07a85404af07e855ca05fd9707fde1 Mon Sep 17 00:00:00 2001 From: chertus Date: Thu, 24 Oct 2019 16:04:50 +0300 Subject: [PATCH 24/25] review related changes --- dbms/src/Interpreters/IdentifierSemantic.cpp | 12 +++++++----- dbms/src/Interpreters/IdentifierSemantic.h | 5 ----- ...mn.reference => 01018_ambiguous_column.reference} | 1 + ...biguous_column.sql => 01018_ambiguous_column.sql} | 3 +++ 4 files changed, 11 insertions(+), 10 deletions(-) rename dbms/tests/queries/0_stateless/{01018_anbiguous_column.reference => 01018_ambiguous_column.reference} (99%) rename dbms/tests/queries/0_stateless/{01018_anbiguous_column.sql => 01018_ambiguous_column.sql} (86%) diff --git a/dbms/src/Interpreters/IdentifierSemantic.cpp b/dbms/src/Interpreters/IdentifierSemantic.cpp index 9930da0e699..34910ef039f 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.cpp +++ b/dbms/src/Interpreters/IdentifierSemantic.cpp @@ -36,9 +36,9 @@ IdentifierSemantic::ColumnMatch tryChooseTable(const ASTIdentifier & identifier, for (size_t i = 0; i < tables.size(); ++i) { auto match = IdentifierSemantic::canReferColumnToTable(identifier, extractTable(tables[i])); - if (value(match)) + if (match != ColumnMatch::NoMatch) { - if (value(match) > value(best_match)) + if (match > best_match) { best_match = match; best_table_pos = i; @@ -49,7 +49,7 @@ IdentifierSemantic::ColumnMatch tryChooseTable(const ASTIdentifier & identifier, } } - if (value(best_match) && same_match) + if ((best_match != ColumnMatch::NoMatch) && same_match) { if (!allow_ambiguous) throw Exception("Ambiguous column '" + identifier.name + "'", ErrorCodes::AMBIGUOUS_COLUMN_NAME); @@ -111,13 +111,15 @@ std::optional IdentifierSemantic::getMembership(const ASTIdentifier & id bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, size_t & best_table_pos, bool ambiguous) { - return value(tryChooseTable(identifier, tables, best_table_pos, ambiguous)); + static constexpr auto no_match = IdentifierSemantic::ColumnMatch::NoMatch; + return tryChooseTable(identifier, tables, best_table_pos, ambiguous) != no_match; } bool IdentifierSemantic::chooseTable(const ASTIdentifier & identifier, const std::vector & tables, size_t & best_table_pos, bool ambiguous) { - return value(tryChooseTable(identifier, tables, best_table_pos, ambiguous)); + static constexpr auto no_match = IdentifierSemantic::ColumnMatch::NoMatch; + return tryChooseTable(identifier, tables, best_table_pos, ambiguous) != no_match; } std::pair IdentifierSemantic::extractDatabaseAndTable(const ASTIdentifier & identifier) diff --git a/dbms/src/Interpreters/IdentifierSemantic.h b/dbms/src/Interpreters/IdentifierSemantic.h index 832a3345b5a..82b5ff31dde 100644 --- a/dbms/src/Interpreters/IdentifierSemantic.h +++ b/dbms/src/Interpreters/IdentifierSemantic.h @@ -53,9 +53,4 @@ private: static bool doesIdentifierBelongTo(const ASTIdentifier & identifier, const String & table); }; -inline UInt32 value(IdentifierSemantic::ColumnMatch match) -{ - return static_cast(match); -} - } diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference b/dbms/tests/queries/0_stateless/01018_ambiguous_column.reference similarity index 99% rename from dbms/tests/queries/0_stateless/01018_anbiguous_column.reference rename to dbms/tests/queries/0_stateless/01018_ambiguous_column.reference index 90b24009d0f..a2a1d6ea4f6 100644 --- a/dbms/tests/queries/0_stateless/01018_anbiguous_column.reference +++ b/dbms/tests/queries/0_stateless/01018_ambiguous_column.reference @@ -10,3 +10,4 @@ ┌─A.dummy─┬─one.dummy─┬─two.dummy─┐ │ 0 │ 0 │ 0 │ └─────────┴───────────┴───────────┘ +0 diff --git a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql b/dbms/tests/queries/0_stateless/01018_ambiguous_column.sql similarity index 86% rename from dbms/tests/queries/0_stateless/01018_anbiguous_column.sql rename to dbms/tests/queries/0_stateless/01018_ambiguous_column.sql index ab291178f87..54603aab810 100644 --- a/dbms/tests/queries/0_stateless/01018_anbiguous_column.sql +++ b/dbms/tests/queries/0_stateless/01018_ambiguous_column.sql @@ -22,3 +22,6 @@ SELECT * from one A JOIN system.one one ON A.dummy = one.dummy JOIN system.one two ON A.dummy = two.dummy FORMAT PrettyCompact; + +-- SELECT one.dummy FROM one AS A FULL JOIN (SELECT 0 AS dymmy) AS one USING dummy; +SELECT one.dummy FROM one AS A JOIN (SELECT 0 AS dummy) B USING dummy; From 7e5b05bbe8b9ffa7b0c38b90eff5211724aa0fe7 Mon Sep 17 00:00:00 2001 From: alesapin Date: Thu, 24 Oct 2019 23:55:41 +0300 Subject: [PATCH 25/25] Revert "Remove hardcoded paths in unwind target" --- contrib/libunwind-cmake/CMakeLists.txt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contrib/libunwind-cmake/CMakeLists.txt b/contrib/libunwind-cmake/CMakeLists.txt index 7901a990b85..f09d0979692 100644 --- a/contrib/libunwind-cmake/CMakeLists.txt +++ b/contrib/libunwind-cmake/CMakeLists.txt @@ -30,4 +30,9 @@ target_include_directories(unwind SYSTEM BEFORE PUBLIC $