Merge branch 'equiv-experimental' of https://github.com/17zhangw/peloton into erik_short_circuit

Author: Erik Sargent

src/include/common/internal_types.h

@@ -1388,9 +1388,9 @@ enum class RuleType : uint32_t {
   // Logical equivalent
   EQUIV_AND,
   EQUIV_OR,
+  EQUIV_COMPARE_EQUAL,
 
-  // Transitive column related rules
-  TRANSITIVE_SINGLE_DEPTH,  // (A.B = x) AND (A.B = y)
+  TV_EQUALITY_WITH_TWO_CV, // (A.B = x) AND (A.B = y) where x/y are constant
   TRANSITIVE_CLOSURE_CONSTANT,  // (A.B = x) AND (A.B = C.D)
 
   // Boolean short-circuit rules

src/include/optimizer/group.h

@@ -100,7 +100,10 @@ class Group : public Printable {
   // This is called in rewrite phase to erase the only logical expression in the
   // group
   inline void EraseLogicalExpression() {
-    PELOTON_ASSERT(logical_expressions_.size() == 1);
+    // During query rewriting (pre-optimizer), the rewriter can execute in a scenario
+    // where a group can have multiple logical expressions (due to AND/OR/= equivalence).
+    // TODO(): refine these assertions to distinguish between optimizer/rewrite stages
+    PELOTON_ASSERT(logical_expressions_.size() >= 1);
     PELOTON_ASSERT(physical_expressions_.size() == 0);
     logical_expressions_.clear();
   }

src/include/optimizer/memo.h

@@ -83,8 +83,11 @@ class Memo {
   // a new one, which reqires us to first remove the original gexpr from the
   // memo
   void EraseExpression(GroupID group_id) {
-    auto gexpr = groups_[group_id]->GetLogicalExpression();
-    group_expressions_.erase(gexpr);
+    std::vector<std::shared_ptr<GroupExpression<Node,OperatorType,OperatorExpr>>> gexprs = groups_[group_id]->GetLogicalExpressions();
+    for (auto gexpr : gexprs) {
+      group_expressions_.erase(gexpr.get());
+    }
+
     groups_[group_id]->EraseLogicalExpression();
   }
 

src/include/optimizer/rule_rewrite.h

@@ -52,9 +52,9 @@ class EquivalentTransform: public Rule<AbsExpr_Container,ExpressionType,AbsExpr_
                  OptimizeContextTemplate *context) const override;
 };
 
-class TransitiveSingleDepthTransform: public Rule<AbsExpr_Container,ExpressionType,AbsExpr_Expression> {
+class TVEqualityWithTwoCVTransform: public Rule<AbsExpr_Container,ExpressionType,AbsExpr_Expression> {
  public:
-  TransitiveSingleDepthTransform();
+  TVEqualityWithTwoCVTransform();
 
   int Promise(GroupExprTemplate *group_expr, OptimizeContextTemplate *context) const override;
   bool Check(std::shared_ptr<AbsExpr_Expression> plan, OptimizeContextTemplate *context) const override;

src/optimizer/optimizer_task.cpp

@@ -459,10 +459,13 @@ bool RewriteTask<Node,OperatorType,OperatorExpr>::OptimizeCurrentGroup(bool repl
     for (auto &r : valid_rules) {
       GroupExprBindingIterator<Node,OperatorType,OperatorExpr> iterator(this->GetMemo(), cur_group_expr,
                                                                         r.rule->GetMatchPattern());
-      if (iterator.HasNext()) {
+      // Keep trying to apply until we exhaust all the bindings.
+      // This could possibly be sub-optimal since the first binding that results
+      // in a transformation by a rule will be applied and become the group's
+      // "new" rewritten expression.
+      while (iterator.HasNext()) {
         // Binding succeeded to a given expression structure
         auto before = iterator.Next();
-        PELOTON_ASSERT(!iterator.HasNext());
 
         // Attempt to apply the transformation
         std::vector<std::shared_ptr<OperatorExpr>> after;

src/optimizer/rule.cpp

@@ -55,6 +55,7 @@ RuleSet<Operator,OperatorType,OperatorExpr>::RuleSet() {
 
 template <>
 RuleSet<AbsExpr_Container,ExpressionType,AbsExpr_Expression>::RuleSet() {
+  // Comparator Elimination related rules
   std::vector<std::pair<RuleType,ExpressionType>> comp_elim_pairs = {
     std::make_pair(RuleType::CONSTANT_COMPARE_EQUAL, ExpressionType::COMPARE_EQUAL),
     std::make_pair(RuleType::CONSTANT_COMPARE_NOTEQUAL, ExpressionType::COMPARE_NOTEQUAL),
@@ -71,17 +72,21 @@ RuleSet<AbsExpr_Container,ExpressionType,AbsExpr_Expression>::RuleSet() {
     );
   }
 
-  AddRewriteRule(
-    RewriteRuleSetName::EQUIVALENT_TRANSFORM,
-    new EquivalentTransform(RuleType::EQUIV_AND, ExpressionType::CONJUNCTION_AND)
-  );
-
-  AddRewriteRule(
-    RewriteRuleSetName::EQUIVALENT_TRANSFORM,
-    new EquivalentTransform(RuleType::EQUIV_OR, ExpressionType::CONJUNCTION_OR)
-  );
+  // Equivalent Transform related rules (flip AND, OR, EQUAL)
+  std::vector<std::pair<RuleType,ExpressionType>> equiv_pairs = {
+    std::make_pair(RuleType::EQUIV_AND, ExpressionType::CONJUNCTION_AND),
+    std::make_pair(RuleType::EQUIV_OR, ExpressionType::CONJUNCTION_OR),
+    std::make_pair(RuleType::EQUIV_COMPARE_EQUAL, ExpressionType::COMPARE_EQUAL)
+  };
+  for (auto &pair : equiv_pairs) {
+    AddRewriteRule(
+      RewriteRuleSetName::EQUIVALENT_TRANSFORM,
+      new EquivalentTransform(pair.first, pair.second)
+    );
+  }
 
-  AddRewriteRule(RewriteRuleSetName::TRANSITIVE_TRANSFORM, new TransitiveSingleDepthTransform());
+  // Additional rules
+  AddRewriteRule(RewriteRuleSetName::TRANSITIVE_TRANSFORM, new TVEqualityWithTwoCVTransform());
   AddRewriteRule(RewriteRuleSetName::TRANSITIVE_TRANSFORM, new TransitiveClosureConstantTransform());
 
   AddRewriteRule(RewriteRuleSetName::BOOLEAN_SHORT_CIRCUIT, new AndShortCircuit());

src/optimizer/rule_rewrite.cpp

@@ -177,8 +177,8 @@ void EquivalentTransform::Transform(std::shared_ptr<AbsExpr_Expression> input,
 // Transitive-Transform related functions
 //
 // ===========================================================
-TransitiveSingleDepthTransform::TransitiveSingleDepthTransform() {
-  type_ = RuleType::TRANSITIVE_SINGLE_DEPTH;
+TVEqualityWithTwoCVTransform::TVEqualityWithTwoCVTransform() {
+  type_ = RuleType::TV_EQUALITY_WITH_TWO_CV;
 
   // (A.B = x) AND (A.B = y)
   match_pattern = std::make_shared<Pattern<ExpressionType>>(ExpressionType::CONJUNCTION_AND);
@@ -199,23 +199,39 @@ TransitiveSingleDepthTransform::TransitiveSingleDepthTransform() {
   match_pattern->AddChild(r_eq);
 }
 
+<<<<<<< Updated upstream
 int TransitiveSingleDepthTransform::Promise(GroupExprTemplate *group_expr, OptimizeContextTemplate *context) const {
+=======
+int TVEqualityWithTwoCVTransform::Promise(GroupExprTemplate *group_expr, OptimizeContextTemplate *context) const {
+>>>>>>> Stashed changes
   (void)group_expr;
   (void)context;
   return static_cast<int>(RulePriority::HIGH);
 }
 
+<<<<<<< Updated upstream
 bool TransitiveSingleDepthTransform::Check(std::shared_ptr<AbsExpr_Expression> plan, OptimizeContextTemplate *context) const {
+=======
+bool TVEqualityWithTwoCVTransform::Check(std::shared_ptr<AbsExpr_Expression> plan, OptimizeContextTemplate *context) const {
+>>>>>>> Stashed changes
   (void)plan;
   (void)context;
   return true;
 }
 
+<<<<<<< Updated upstream
 void TransitiveSingleDepthTransform::Transform(std::shared_ptr<AbsExpr_Expression> input,
                                                std::vector<std::shared_ptr<AbsExpr_Expression>> &transformed,
                                                OptimizeContextTemplate *context) const {
   (void)context;
   //TODO(wz2): TransitiveClosureConstant should work beyond straight equality
+=======
+void TVEqualityWithTwoCVTransform::Transform(std::shared_ptr<AbsExpr_Expression> input,
+                                               std::vector<std::shared_ptr<AbsExpr_Expression>> &transformed,
+                                               OptimizeContextTemplate *context) const {
+  (void)context;
+  //TODO(wz2): TVEqualityWithTwoCVTransform should work beyond straight equality
+>>>>>>> Stashed changes
 
   // Asserting guarantees provided by the GroupExprBindingIterator
   // Structure: (A.B = x) AND (A.B = y)
@@ -344,7 +360,11 @@ void TransitiveClosureConstantTransform::Transform(std::shared_ptr<AbsExpr_Expre
   PELOTON_ASSERT(l_tv->Children().size() == 0);
   PELOTON_ASSERT(l_cv->Children().size() == 0);
   PELOTON_ASSERT(l_tv->Op().GetType() == ExpressionType::VALUE_TUPLE);
+<<<<<<< Updated upstream
   PELOTON_ASSERT(l_tv->Op().GetType() == ExpressionType::VALUE_CONSTANT);
+=======
+  PELOTON_ASSERT(l_cv->Op().GetType() == ExpressionType::VALUE_CONSTANT);
+>>>>>>> Stashed changes
 
   auto r_tv_l = r_eq->Children()[0];
   auto r_tv_r = r_eq->Children()[1];
@@ -386,7 +406,11 @@ void TransitiveClosureConstantTransform::Transform(std::shared_ptr<AbsExpr_Expre
     new_right_eq->PushChild(r_tv_r);
   } else {
     // At this stage, we have knowledge that A.B = E.F
+<<<<<<< Updated upstream
     new_right_eq->PushChild(r_tv_r);
+=======
+    new_right_eq->PushChild(r_tv_l);
+>>>>>>> Stashed changes
     new_right_eq->PushChild(right_val_copy);
   }
 

src/traffic_cop/traffic_cop.cpp

@@ -581,15 +581,15 @@ ResultType TrafficCop::ExecuteStatement(
         statement, std::move(param_stats));
   }
 
-  LOG_TRACE("Execute Statement of name: %s",
+  LOG_DEBUG("Execute Statement of name: %s",
             statement->GetStatementName().c_str());
-  LOG_TRACE("Execute Statement of query: %s",
+  LOG_DEBUG("Execute Statement of query: %s",
             statement->GetQueryString().c_str());
-  LOG_TRACE("Execute Statement Plan:\n%s",
+  LOG_DEBUG("Execute Statement Plan:\n%s",
             planner::PlanUtil::GetInfo(statement->GetPlanTree().get()).c_str());
-  LOG_TRACE("Execute Statement Query Type: %s",
+  LOG_DEBUG("Execute Statement Query Type: %s",
             statement->GetQueryTypeString().c_str());
-  LOG_TRACE("----QueryType: %d--------",
+  LOG_DEBUG("----QueryType: %d--------",
             static_cast<int>(statement->GetQueryType()));
 
   try {