From 09010255adc777c79018f1be0011361f3e735a26 Mon Sep 17 00:00:00 2001 From: GustavoAngulo Date: Mon, 30 Jul 2018 21:17:09 -0700 Subject: [PATCH 1/3] False negative in rule bug --- src/optimizer/group_expression.cpp | 2 +- src/optimizer/memo.cpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/optimizer/group_expression.cpp b/src/optimizer/group_expression.cpp index 4d874bd27ef..a6fd46de4a4 100644 --- a/src/optimizer/group_expression.cpp +++ b/src/optimizer/group_expression.cpp @@ -95,7 +95,7 @@ bool GroupExpression::operator==(const GroupExpression &r) { } void GroupExpression::SetRuleExplored(Rule *rule) { - rule_mask_.set(rule->GetRuleIdx()) = true; + rule_mask_.set(rule->GetRuleIdx(), true); } bool GroupExpression::HasRuleExplored(Rule *rule) { diff --git a/src/optimizer/memo.cpp b/src/optimizer/memo.cpp index 28eed420726..2a6cc49bf4c 100644 --- a/src/optimizer/memo.cpp +++ b/src/optimizer/memo.cpp @@ -43,8 +43,6 @@ GroupExpression *Memo::InsertExpression(std::shared_ptr gexpr, auto it = group_expressions_.find(gexpr.get()); if (it != group_expressions_.end()) { - PELOTON_ASSERT(target_group == UNDEFINED_GROUP || - target_group == (*it)->GetGroupID()); gexpr->SetGroupID((*it)->GetGroupID()); return *it; } else { From 5e5dba29c9eb157cbbc7fb280ea4b87a420a086a Mon Sep 17 00:00:00 2001 From: GustavoAngulo Date: Mon, 30 Jul 2018 21:36:43 -0700 Subject: [PATCH 2/3] Fix for false positive rule bug --- src/optimizer/group_expression.cpp | 10 ++++++++-- test/optimizer/optimizer_rule_test.cpp | 27 ++++++++++++++++++++++++++ test/sql/testing_sql_util.cpp | 16 +++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/optimizer/group_expression.cpp b/src/optimizer/group_expression.cpp index a6fd46de4a4..b1f76b5b3c7 100644 --- a/src/optimizer/group_expression.cpp +++ b/src/optimizer/group_expression.cpp @@ -87,8 +87,14 @@ hash_t GroupExpression::Hash() const { bool GroupExpression::operator==(const GroupExpression &r) { bool eq = (op == r.Op()); - for (size_t i = 0; i < child_groups.size(); ++i) { - eq = eq && (child_groups[i] == r.child_groups[i]); + auto left_groups = child_groups; + auto right_groups = r.child_groups; + + std::sort(left_groups.begin(), left_groups.end()); + + std::sort(right_groups.begin(), right_groups.end()); + for (size_t i = 0; i < left_groups.size(); ++i) { + eq = eq && (left_groups[i] == right_groups[i]); } return eq; diff --git a/test/optimizer/optimizer_rule_test.cpp b/test/optimizer/optimizer_rule_test.cpp index 12d047ad51a..23f520596dc 100644 --- a/test/optimizer/optimizer_rule_test.cpp +++ b/test/optimizer/optimizer_rule_test.cpp @@ -261,5 +261,32 @@ TEST_F(OptimizerRuleTests, SimpleAssociativeRuleTest2) { delete root_context; } +TEST_F(OptimizerRuleTests, RuleBitmapTest) { + Optimizer optimizer; + auto &memo = optimizer.GetMetadata().memo; + + auto dummy_operator = std::make_shared(LogicalGet::make()); + auto dummy_group = memo.InsertExpression(optimizer.GetMetadata().MakeGroupExpression(dummy_operator), false); + + auto rule1 = new InnerJoinCommutativity(); + auto rule2 = new GetToSeqScan(); + + EXPECT_FALSE(dummy_group->HasRuleExplored(rule1)); + EXPECT_FALSE(dummy_group->HasRuleExplored(rule2)); + + dummy_group->SetRuleExplored(rule1); + + EXPECT_TRUE(dummy_group->HasRuleExplored(rule1)); + EXPECT_FALSE(dummy_group->HasRuleExplored(rule2)); + + dummy_group->SetRuleExplored(rule2); + + EXPECT_TRUE(dummy_group->HasRuleExplored(rule1)); + EXPECT_TRUE(dummy_group->HasRuleExplored(rule2)); + + delete rule1; + delete rule2; +} + } // namespace test } // namespace peloton diff --git a/test/sql/testing_sql_util.cpp b/test/sql/testing_sql_util.cpp index 9549c794a91..4d3ac63133f 100644 --- a/test/sql/testing_sql_util.cpp +++ b/test/sql/testing_sql_util.cpp @@ -98,6 +98,21 @@ ResultType TestingSQLUtil::ExecuteSQLQuery( return status; } +void PrintPlan(planner::AbstractPlan *plan, int level = 0) { + auto spacing = std::string(level, '\t'); + if (plan->GetPlanNodeType() == PlanNodeType::SEQSCAN) { + auto scan = dynamic_cast(plan); + LOG_INFO("%s%s(%s)", spacing.c_str(), scan->GetInfo().c_str(), + scan->GetTable()->GetName().c_str()); + } else { + LOG_INFO("%s%s", spacing.c_str(), plan->GetInfo().c_str()); + } + for (size_t i = 0; i < plan->GetChildren().size(); i++) { + PrintPlan(plan->GetChildren()[i].get(), level + 1); + } + return; +} + // Execute a SQL query end-to-end with the specific optimizer ResultType TestingSQLUtil::ExecuteSQLQueryWithOptimizer( std::unique_ptr &optimizer, @@ -116,6 +131,7 @@ ResultType TestingSQLUtil::ExecuteSQLQueryWithOptimizer( bind_node_visitor.BindNameToNode(parsed_stmt->GetStatement(0)); auto plan = optimizer->BuildPelotonPlanTree(parsed_stmt, txn); + PrintPlan(plan.get()); tuple_descriptor = traffic_cop_.GenerateTupleDescriptor(parsed_stmt->GetStatement(0)); auto result_format = std::vector(tuple_descriptor.size(), 0); From 9ec09e131f47adc7c7e9c6bf708d61e338c43c87 Mon Sep 17 00:00:00 2001 From: GustavoAngulo Date: Mon, 30 Jul 2018 21:38:23 -0700 Subject: [PATCH 3/3] Remove print --- test/sql/testing_sql_util.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/sql/testing_sql_util.cpp b/test/sql/testing_sql_util.cpp index 4d3ac63133f..feb9fad20c4 100644 --- a/test/sql/testing_sql_util.cpp +++ b/test/sql/testing_sql_util.cpp @@ -131,7 +131,6 @@ ResultType TestingSQLUtil::ExecuteSQLQueryWithOptimizer( bind_node_visitor.BindNameToNode(parsed_stmt->GetStatement(0)); auto plan = optimizer->BuildPelotonPlanTree(parsed_stmt, txn); - PrintPlan(plan.get()); tuple_descriptor = traffic_cop_.GenerateTupleDescriptor(parsed_stmt->GetStatement(0)); auto result_format = std::vector(tuple_descriptor.size(), 0);