Repository: incubator-quickstep Updated Branches: refs/heads/master 73d796dee -> 876f12ba5
Fixed the bug regarding EliminateEmptyNode on an aggregate. Project: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/commit/876f12ba Tree: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/tree/876f12ba Diff: http://git-wip-us.apache.org/repos/asf/incubator-quickstep/diff/876f12ba Branch: refs/heads/master Commit: 876f12ba587eed7b8cf7f2b3f7d45affc885e1ac Parents: 73d796d Author: Zuyu Zhang <z...@cs.wisc.edu> Authored: Thu May 3 17:34:51 2018 -0500 Committer: Zuyu Zhang <z...@cs.wisc.edu> Committed: Thu May 3 18:08:43 2018 -0500 ---------------------------------------------------------------------- cli/tests/command_executor/Analyze.test | 27 ++++++++ query_optimizer/ExecutionGenerator.cpp | 6 ++ query_optimizer/rules/CMakeLists.txt | 1 + query_optimizer/rules/EliminateEmptyNode.cpp | 78 +++++++++++++++++++---- 4 files changed, 99 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/876f12ba/cli/tests/command_executor/Analyze.test ---------------------------------------------------------------------- diff --git a/cli/tests/command_executor/Analyze.test b/cli/tests/command_executor/Analyze.test index 57b8312..b0bf090 100644 --- a/cli/tests/command_executor/Analyze.test +++ b/cli/tests/command_executor/Analyze.test @@ -27,6 +27,15 @@ INSERT INTO t VALUES(0, 0); -- == +SELECT COUNT(*) FROM r; +-- ++--------------------+ +|COUNT(*) | ++--------------------+ +| 0| ++--------------------+ +== + \analyze -- Analyzing r ... done @@ -34,6 +43,24 @@ Analyzing s ... done Analyzing t ... done == +SELECT COUNT(*) FROM r; +-- ++--------------------+ +|COUNT(*) | ++--------------------+ +| 0| ++--------------------+ +== + +SELECT MIN(src) FROM r; +-- ++-----------+ +|MIN(src) | ++-----------+ +| NULL| ++-----------+ +== + SELECT r.src, r.dst FROM r; -- +-----------+-----------+ http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/876f12ba/query_optimizer/ExecutionGenerator.cpp ---------------------------------------------------------------------- diff --git a/query_optimizer/ExecutionGenerator.cpp b/query_optimizer/ExecutionGenerator.cpp index f8f7c9d..cc1319c 100644 --- a/query_optimizer/ExecutionGenerator.cpp +++ b/query_optimizer/ExecutionGenerator.cpp @@ -1956,6 +1956,9 @@ void ExecutionGenerator::convertAggregate( execution_plan_->addDirectDependency(aggregation_operator_index, input_relation_info->producer_operator_index, false /* is_pipeline_breaker */); + } else if (input_relation.isTemporary()) { + // NOTE(zuyu): drop the temp relation created by EliminateEmptyNode rule. + temporary_relation_info_vec_.emplace_back(aggregation_operator_index, &input_relation); } if (use_parallel_initialization) { @@ -2451,6 +2454,9 @@ void ExecutionGenerator::convertWindowAggregate( execution_plan_->addDirectDependency(window_aggregation_operator_index, input_relation_info->producer_operator_index, true /* is_pipeline_breaker */); + } else if (input_relation.isTemporary()) { + // NOTE(zuyu): drop the temp relation created by EliminateEmptyNode rule. + temporary_relation_info_vec_.emplace_back(window_aggregation_operator_index, &input_relation); } insert_destination_proto->set_relational_op_index(window_aggregation_operator_index); http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/876f12ba/query_optimizer/rules/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/query_optimizer/rules/CMakeLists.txt b/query_optimizer/rules/CMakeLists.txt index 014a99b..8465c89 100644 --- a/query_optimizer/rules/CMakeLists.txt +++ b/query_optimizer/rules/CMakeLists.txt @@ -111,6 +111,7 @@ target_link_libraries(quickstep_queryoptimizer_rules_EliminateEmptyNode quickstep_queryoptimizer_OptimizerContext quickstep_queryoptimizer_expressions_Alias quickstep_queryoptimizer_expressions_AttributeReference + quickstep_queryoptimizer_expressions_ExprId quickstep_queryoptimizer_expressions_ExpressionUtil quickstep_queryoptimizer_expressions_NamedExpression quickstep_queryoptimizer_expressions_PatternMatcher http://git-wip-us.apache.org/repos/asf/incubator-quickstep/blob/876f12ba/query_optimizer/rules/EliminateEmptyNode.cpp ---------------------------------------------------------------------- diff --git a/query_optimizer/rules/EliminateEmptyNode.cpp b/query_optimizer/rules/EliminateEmptyNode.cpp index 716a167..03e6f33 100644 --- a/query_optimizer/rules/EliminateEmptyNode.cpp +++ b/query_optimizer/rules/EliminateEmptyNode.cpp @@ -24,6 +24,7 @@ #include <sstream> #include <string> #include <unordered_set> +#include <utility> #include <vector> #include "catalog/CatalogAttribute.hpp" @@ -33,6 +34,7 @@ #include "query_optimizer/OptimizerContext.hpp" #include "query_optimizer/expressions/Alias.hpp" #include "query_optimizer/expressions/AttributeReference.hpp" +#include "query_optimizer/expressions/ExprId.hpp" #include "query_optimizer/expressions/ExpressionUtil.hpp" #include "query_optimizer/expressions/NamedExpression.hpp" #include "query_optimizer/expressions/PatternMatcher.hpp" @@ -262,21 +264,15 @@ P::PhysicalPtr EliminateEmptyNode::apply(const P::PhysicalPtr &input) { std::vector<E::AttributeReferencePtr> project_expressions; switch (plan_type) { case P::PhysicalType::kAggregate: - case P::PhysicalType::kCrossReferenceCoalesceAggregate: case P::PhysicalType::kFilterJoin: case P::PhysicalType::kHashJoin: case P::PhysicalType::kNestedLoopsJoin: case P::PhysicalType::kSample: - case P::PhysicalType::kSelection: case P::PhysicalType::kSort: case P::PhysicalType::kUnionAll: case P::PhysicalType::kWindowAggregate: project_expressions = plan->getOutputAttributes(); break; - case P::PhysicalType::kCopyTo: - case P::PhysicalType::kInsertSelection: - project_expressions = plan->getReferencedAttributes(); - break; case P::PhysicalType::kCopyFrom: case P::PhysicalType::kCreateIndex: case P::PhysicalType::kCreateTable: @@ -288,11 +284,53 @@ P::PhysicalPtr EliminateEmptyNode::apply(const P::PhysicalPtr &input) { case P::PhysicalType::kUpdateTable: // TODO(quickstep-team): revisit the cases above. return input; + case P::PhysicalType::kCopyTo: + case P::PhysicalType::kInsertSelection: + project_expressions = plan->getReferencedAttributes(); + break; + case P::PhysicalType::kCrossReferenceCoalesceAggregate: case P::PhysicalType::kTableReference: case P::PhysicalType::kTopLevelPlan: LOG(FATAL) << "Unexpected PhysicalType."; + case P::PhysicalType::kSelection: { + const auto &selection_input = selection->input(); + switch (selection_input->getPhysicalType()) { + case P::PhysicalType::kAggregate: + case P::PhysicalType::kWindowAggregate: { + DCHECK(project_expressions.empty()); + const auto referenced_attributes = selection_input->getReferencedAttributes(); + std::unordered_set<E::ExprId> unique_expr_ids; + for (auto &referenced_attribute : referenced_attributes) { + const E::ExprId expr_id = referenced_attribute->id(); + if (unique_expr_ids.find(expr_id) == unique_expr_ids.end()) { + project_expressions.emplace_back(std::move(referenced_attribute)); + unique_expr_ids.insert(expr_id); + } + } + break; + } + case P::PhysicalType::kCrossReferenceCoalesceAggregate: + LOG(FATAL) << "Unexpected PhysicalType."; + default: + project_expressions = plan->getOutputAttributes(); + break; + } + break; + } + } + +#ifdef QUICKSTEP_DEBUG + { + CHECK(!project_expressions.empty()); + + std::unordered_set<E::ExprId> unique_expr_ids; + unique_expr_ids.reserve(project_expressions.size()); + for (const auto &project_expression : project_expressions) { + unique_expr_ids.insert(project_expression->id()); + } + CHECK_EQ(project_expressions.size(), unique_expr_ids.size()); } - DCHECK(!project_expressions.empty()); +#endif auto output = Apply(plan); if (output == plan) { @@ -329,23 +367,37 @@ P::PhysicalPtr EliminateEmptyNode::apply(const P::PhysicalPtr &input) { switch (plan_type) { case P::PhysicalType::kAggregate: - case P::PhysicalType::kCrossReferenceCoalesceAggregate: + case P::PhysicalType::kCopyTo: + case P::PhysicalType::kWindowAggregate: + output = plan->copyWithNewChildren({ temp_table }); + break; case P::PhysicalType::kFilterJoin: case P::PhysicalType::kHashJoin: case P::PhysicalType::kNestedLoopsJoin: case P::PhysicalType::kSample: - case P::PhysicalType::kSelection: case P::PhysicalType::kSort: case P::PhysicalType::kUnionAll: - case P::PhysicalType::kWindowAggregate: output = P::Selection::Create(temp_table, E::ToNamedExpressions(project_expressions), nullptr); break; - case P::PhysicalType::kCopyTo: - output = plan->copyWithNewChildren({ temp_table }); - break; case P::PhysicalType::kInsertSelection: output = plan->copyWithNewChildren({ plan->children().front(), temp_table }); break; + case P::PhysicalType::kSelection: { + const P::PhysicalPtr &selection_input = selection->input(); + switch (selection_input->getPhysicalType()) { + case P::PhysicalType::kAggregate: + case P::PhysicalType::kWindowAggregate: + output = selection_input->copyWithNewChildren({ temp_table }); + break; + case P::PhysicalType::kCrossReferenceCoalesceAggregate: + LOG(FATAL) << "Unexpected PhysicalType."; + default: + output = temp_table; + break; + } + output = selection->copyWithNewChildren({ output }); + break; + } default: LOG(FATAL) << "Unexpected PhysicalType."; }
