From 5208a70203e6acd8593733b9e329fb161f84f05d Mon Sep 17 00:00:00 2001 From: chertus Date: Tue, 27 Aug 2019 22:41:51 +0300 Subject: [PATCH] one less place with changed nullability columns --- dbms/src/Interpreters/AnalyzedJoin.cpp | 17 ---------- dbms/src/Interpreters/AnalyzedJoin.h | 3 -- dbms/src/Interpreters/ExpressionActions.cpp | 4 +-- dbms/src/Interpreters/SyntaxAnalyzer.cpp | 37 +++++++++++++-------- dbms/src/Interpreters/SyntaxAnalyzer.h | 4 +-- 5 files changed, 28 insertions(+), 37 deletions(-) diff --git a/dbms/src/Interpreters/AnalyzedJoin.cpp b/dbms/src/Interpreters/AnalyzedJoin.cpp index 4176b0b8012..6a3b9b8ac1b 100644 --- a/dbms/src/Interpreters/AnalyzedJoin.cpp +++ b/dbms/src/Interpreters/AnalyzedJoin.cpp @@ -7,7 +7,6 @@ #include #include -#include namespace DB { @@ -102,22 +101,6 @@ std::unordered_map AnalyzedJoin::getOriginalColumnsMap(const Nam return out; } -void AnalyzedJoin::calculateAvailableJoinedColumns(bool make_nullable) -{ - if (!make_nullable) - { - available_joined_columns = columns_from_joined_table; - return; - } - - for (auto & column : columns_from_joined_table) - { - auto type = column.type->canBeInsideNullable() ? makeNullable(column.type) : column.type; - available_joined_columns.emplace_back(NameAndTypePair(column.name, std::move(type))); - } -} - - NamesAndTypesList getNamesAndTypeListFromTableExpression(const ASTTableExpression & table_expression, const Context & context) { NamesAndTypesList names_and_type_list; diff --git a/dbms/src/Interpreters/AnalyzedJoin.h b/dbms/src/Interpreters/AnalyzedJoin.h index af010aaca11..1ce11da95e0 100644 --- a/dbms/src/Interpreters/AnalyzedJoin.h +++ b/dbms/src/Interpreters/AnalyzedJoin.h @@ -42,8 +42,6 @@ private: /// All columns which can be read from joined table. Duplicating names are qualified. NamesAndTypesList columns_from_joined_table; - /// Columns from joined table which may be added to block. It's columns_from_joined_table with possibly modified types. - NamesAndTypesList available_joined_columns; /// Name -> original name. Names are the same as in columns_from_joined_table list. std::unordered_map original_names; /// Original name -> name. Only ranamed columns. @@ -61,7 +59,6 @@ public: std::unordered_map getOriginalColumnsMap(const NameSet & required_columns) const; void deduplicateAndQualifyColumnNames(const NameSet & left_table_columns, const String & right_table_prefix); - void calculateAvailableJoinedColumns(bool make_nullable); size_t rightKeyInclusion(const String & name) const; }; diff --git a/dbms/src/Interpreters/ExpressionActions.cpp b/dbms/src/Interpreters/ExpressionActions.cpp index c7b510abcf0..5ef05569f91 100644 --- a/dbms/src/Interpreters/ExpressionActions.cpp +++ b/dbms/src/Interpreters/ExpressionActions.cpp @@ -278,8 +278,8 @@ void ExpressionAction::prepare(Block & sample_block, const Settings & settings, case JOIN: { bool is_null_used_as_default = settings.join_use_nulls; - bool right_or_full_join = join_kind == ASTTableJoin::Kind::Right || join_kind == ASTTableJoin::Kind::Full; - bool left_or_full_join = join_kind == ASTTableJoin::Kind::Left || join_kind == ASTTableJoin::Kind::Full; + bool right_or_full_join = isRightOrFull(join_kind); + bool left_or_full_join = isLeftOrFull(join_kind); for (auto & col : sample_block) { diff --git a/dbms/src/Interpreters/SyntaxAnalyzer.cpp b/dbms/src/Interpreters/SyntaxAnalyzer.cpp index c7f89153b44..0c73beeef16 100644 --- a/dbms/src/Interpreters/SyntaxAnalyzer.cpp +++ b/dbms/src/Interpreters/SyntaxAnalyzer.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -488,13 +489,14 @@ void getArrayJoinedColumns(ASTPtr & query, SyntaxAnalyzerResult & result, const } } -void setJoinStrictness(ASTSelectQuery & select_query, JoinStrictness join_default_strictness) +void setJoinStrictness(ASTSelectQuery & select_query, JoinStrictness join_default_strictness, ASTTableJoin::Kind & join_kind) { const ASTTablesInSelectQueryElement * node = select_query.join(); if (!node) return; auto & table_join = const_cast(node)->table_join->as(); + join_kind = table_join.kind; if (table_join.strictness == ASTTableJoin::Strictness::Unspecified && table_join.kind != ASTTableJoin::Kind::Cross) @@ -511,7 +513,7 @@ void setJoinStrictness(ASTSelectQuery & select_query, JoinStrictness join_defaul /// Find the columns that are obtained by JOIN. void collectJoinedColumns(AnalyzedJoin & analyzed_join, const ASTSelectQuery & select_query, const NameSet & source_columns, - const Aliases & aliases, bool join_use_nulls) + const Aliases & aliases) { const ASTTablesInSelectQueryElement * node = select_query.join(); if (!node) @@ -537,10 +539,6 @@ void collectJoinedColumns(AnalyzedJoin & analyzed_join, const ASTSelectQuery & s if (is_asof) data.asofToJoinKeys(); } - - bool make_nullable = join_use_nulls && isLeftOrFull(table_join.kind); - - analyzed_join.calculateAvailableJoinedColumns(make_nullable); } void replaceJoinedTable(const ASTTablesInSelectQueryElement* join) @@ -611,7 +609,8 @@ std::vector getAggregates(const ASTPtr & query) /// Calculate which columns are required to execute the expression. /// Then, delete all other columns from the list of available columns. /// After execution, columns will only contain the list of columns needed to read from the table. -void SyntaxAnalyzerResult::collectUsedColumns(const ASTPtr & query, const NamesAndTypesList & additional_source_columns) +void SyntaxAnalyzerResult::collectUsedColumns(const ASTPtr & query, const NamesAndTypesList & additional_source_columns, + bool make_joined_columns_nullable) { /// We caclulate required_source_columns with source_columns modifications and swap them on exit required_source_columns = source_columns; @@ -639,7 +638,7 @@ void SyntaxAnalyzerResult::collectUsedColumns(const ASTPtr & query, const NamesA /// Add columns obtained by JOIN (if needed). columns_added_by_join.clear(); - for (const auto & joined_column : analyzed_join.available_joined_columns) + for (const auto & joined_column : analyzed_join.columns_from_joined_table) { auto & name = joined_column.name; if (avaliable_columns.count(name)) @@ -649,7 +648,15 @@ void SyntaxAnalyzerResult::collectUsedColumns(const ASTPtr & query, const NamesA { /// Optimisation: do not add columns needed only in JOIN ON section. if (columns_context.nameInclusion(name) > analyzed_join.rightKeyInclusion(name)) - columns_added_by_join.push_back(joined_column); + { + if (make_joined_columns_nullable) + { + auto type = joined_column.type->canBeInsideNullable() ? makeNullable(joined_column.type) : joined_column.type; + columns_added_by_join.emplace_back(NameAndTypePair(joined_column.name, std::move(type))); + } + else + columns_added_by_join.push_back(joined_column); + } required.erase(name); } } @@ -759,7 +766,7 @@ void SyntaxAnalyzerResult::collectUsedColumns(const ASTPtr & query, const NamesA if (columns_context.has_table_join) { ss << ", joined columns:"; - for (const auto & column : analyzed_join.available_joined_columns) + for (const auto & column : analyzed_join.columns_from_joined_table) ss << " '" << column.name << "'"; } @@ -865,6 +872,7 @@ SyntaxAnalyzerResultPtr SyntaxAnalyzer::analyze( /// Optimize if with constant condition after constants was substituted instead of scalar subqueries. OptimizeIfWithConstantConditionVisitor(result.aliases).visit(query); + bool make_joined_columns_nullable = false; if (select_query) { /// GROUP BY injective function elimination. @@ -885,12 +893,15 @@ SyntaxAnalyzerResultPtr SyntaxAnalyzer::analyze( /// Push the predicate expression down to the subqueries. result.rewrite_subqueries = PredicateExpressionsOptimizer(select_query, settings, context).optimize(); - setJoinStrictness(*select_query, settings.join_default_strictness); - collectJoinedColumns(result.analyzed_join, *select_query, source_columns_set, result.aliases, settings.join_use_nulls); + ASTTableJoin::Kind join_kind = ASTTableJoin::Kind::Comma; + setJoinStrictness(*select_query, settings.join_default_strictness, join_kind); + make_joined_columns_nullable = settings.join_use_nulls && isLeftOrFull(join_kind); + + collectJoinedColumns(result.analyzed_join, *select_query, source_columns_set, result.aliases); } result.aggregates = getAggregates(query); - result.collectUsedColumns(query, additional_source_columns); + result.collectUsedColumns(query, additional_source_columns, make_joined_columns_nullable); return std::make_shared(result); } diff --git a/dbms/src/Interpreters/SyntaxAnalyzer.h b/dbms/src/Interpreters/SyntaxAnalyzer.h index a31dfef7e82..e95d7354e8a 100644 --- a/dbms/src/Interpreters/SyntaxAnalyzer.h +++ b/dbms/src/Interpreters/SyntaxAnalyzer.h @@ -20,7 +20,7 @@ struct SyntaxAnalyzerResult NamesAndTypesList source_columns; /// Set of columns that are enough to read from the table to evaluate the expression. It does not include joined columns. NamesAndTypesList required_source_columns; - /// Columns will be added to block by JOIN. It's a subset of analyzed_join.available_joined_columns + /// Columns will be added to block by JOIN. It's a subset of analyzed_join.columns_from_joined_table with corrected Nullability NamesAndTypesList columns_added_by_join; Aliases aliases; @@ -42,7 +42,7 @@ struct SyntaxAnalyzerResult /// Predicate optimizer overrides the sub queries bool rewrite_subqueries = false; - void collectUsedColumns(const ASTPtr & query, const NamesAndTypesList & additional_source_columns); + void collectUsedColumns(const ASTPtr & query, const NamesAndTypesList & additional_source_columns, bool make_joined_columns_nullable); Names requiredSourceColumns() const { return required_source_columns.getNames(); } };