Ported equivalent transforms and other rules onto AbstractNodes

Author: 17zhangw

src/include/common/internal_types.h

@@ -1,15 +1,3 @@
-//===----------------------------------------------------------------------===//
-//
-//                         Peloton
-//
-// internal_types.h
-//
-// Identification: src/include/common/internal_types.h
-//
-// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
-//
-//===----------------------------------------------------------------------===//
-
 //===----------------------------------------------------------------------===//
 //
 //                         Peloton
@@ -258,7 +246,12 @@ enum class ExpressionType {
   // -----------------------------
   // Miscellaneous
   // -----------------------------
-  CAST = 600
+  CAST = 600,
+
+  // -----------------------------
+  // Rewriter-specific identifier
+  // -----------------------------
+  GROUP_MARKER = 721
 };
 
 // When short_str is true, return a short version of ExpressionType string
@@ -1384,8 +1377,21 @@ enum class RuleType : uint32_t {
   PULL_FILTER_THROUGH_AGGREGATION,
 
   // AST rewrite rules (logical -> logical)
-  // Removes ConstantValueExpression = ConstantValueExpression
-  COMP_EQUALITY_ELIMINATION,
+  // Removes ConstantValue =/!=/</>/<=/>= ConstantValue
+  CONSTANT_COMPARE_EQUAL,
+  CONSTANT_COMPARE_NOTEQUAL,
+  CONSTANT_COMPARE_LESSTHAN,
+  CONSTANT_COMPARE_GREATERTHAN,
+  CONSTANT_COMPARE_LESSTHANOREQUALTO,
+  CONSTANT_COMPARE_GREATERTHANOREQUALTO,
+
+  // Logical equivalent
+  EQUIV_AND,
+  EQUIV_OR,
+  EQUIV_COMPARE_EQUAL,
+
+  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)
 
   // Place holder to generate number of rules compile time
   NUM_RULES

src/include/expression/abstract_expression.h

@@ -111,6 +111,24 @@ class AbstractExpression : public Printable {
     children_[index].reset(expr);
   }
 
+  void ClearChildren() {
+    // The rewriter copies the AbstractExpression presented to the rewriter.
+    // This function is used by the rewriter to properly wipe all the children
+    // of AbstractExpression once the AbstractExpression tree has been converted
+    // to the intermediary AbsExpr_Container/Expression tree.
+    //
+    // This function should only be invoked on copied AbstractExpressions and
+    // never on original ones passed into the [rewriter] otherwise we would
+    // violate immutable properties. We do not believe this function is strictly
+    // necessary in terrier, however this function does serve some usefulness.
+    //
+    // This allows us to reduce our memory footprint while also providing better
+    // implementation constraints within the rewriter (i.e: the rewriter should
+    // not be trying to operate directly on AbstractExpression but rather on the
+    // intermediary representation).
+    children_.clear();
+  }
+
   void SetExpressionType(ExpressionType type) { exp_type_ = type; }
 
   //////////////////////////////////////////////////////////////////////////////

src/include/expression/group_marker_expression.h

@@ -0,0 +1,64 @@
+//===----------------------------------------------------------------------===//
+//
+//                         Peloton
+//
+// group_marker_expression.h
+//
+// Identification: src/include/expression/group_marker_expression.h
+//
+// Copyright (c) 2015-2018, Carnegie Mellon University Database Group
+//
+//===----------------------------------------------------------------------===//
+
+#pragma once
+
+#include "expression/abstract_expression.h"
+#include "optimizer/group_expression.h"
+#include "util/hash_util.h"
+
+namespace peloton {
+
+namespace executor {
+class ExecutorContext;
+}  // namespace executor
+
+namespace expression {
+
+//===----------------------------------------------------------------------===//
+// GroupMarkerExpression
+//===----------------------------------------------------------------------===//
+
+class GroupMarkerExpression : public AbstractExpression {
+ public:
+  GroupMarkerExpression(optimizer::GroupID group_id) :
+    AbstractExpression(ExpressionType::GROUP_MARKER),
+    group_id_(group_id) {};
+
+  optimizer::GroupID GetGroupID() { return group_id_; }
+
+  AbstractExpression *Copy() const override {
+    return new GroupMarkerExpression(group_id_);
+  }
+
+  type::Value Evaluate(const AbstractTuple *tuple1,
+                       const AbstractTuple *tuple2,
+                       executor::ExecutorContext *context) const {
+    (void)tuple1;
+    (void)tuple2;
+    (void)context;
+    PELOTON_ASSERT(0);
+  }
+
+  void Accept(SqlNodeVisitor *) {
+    PELOTON_ASSERT(0);
+  }
+
+ protected:
+  optimizer::GroupID group_id_;
+
+  GroupMarkerExpression(const GroupMarkerExpression &other)
+      : AbstractExpression(other), group_id_(other.group_id_) {}
+};
+
+}  // namespace expression
+}  // namespace peloton

src/include/optimizer/absexpr_expression.h

@@ -10,7 +10,8 @@
 
 #pragma once
 
-// AbstractExpression Definition
+#include "optimizer/abstract_node_expression.h"
+#include "optimizer/abstract_node.h"
 #include "expression/abstract_expression.h"
 #include "expression/conjunction_expression.h"
 #include "expression/comparison_expression.h"
@@ -22,41 +23,45 @@
 namespace peloton {
 namespace optimizer {
 
-// (TODO): rethink the AbsExpr_Container/Expression approach in comparion to abstract
-// Most of the core rule/optimizer code relies on the concept of an Operator /
-// OperatorExpression and the interface that the two functions respectively expose.
+// AbsExpr_Container and AbsExpr_Expression provides and serves an analogous purpose
+// to Operator and OperatorExpression. Each AbsExpr_Container wraps a single
+// AbstractExpression node with the children placed inside the AbsExpr_Expression.
 //
-// The annoying part is that an AbstractExpression blends together an Operator
-// and OperatorExpression. Second part, the AbstractExpression does not export the
-// correct interface that the rest of the system depends on.
-//
-// As an extreme level of simplification (sort of hacky), an AbsExpr_Container is
-// analogous to Operator and wraps a single AbstractExpression node. AbsExpr_Expression
-// is analogous to OperatorExpression.
-//
-// AbsExpr_Container does *not* handle memory correctly w.r.t internal instantiations
-// from Rule transformation. This is since Peloton itself mixes unique_ptrs and
-// hands out raw pointers which makes adding a shared_ptr here extremely problematic.
-// terrier uses only shared_ptr when dealing with AbstractExpression trees.
-
-class AbsExpr_Container {
+// This is done to export the correct interface from the wrapped AbstractExpression
+// to the rest of the core rule/optimizer code/logic.
+class AbsExpr_Container: public AbstractNode {
  public:
-  AbsExpr_Container();
+  // Default constructors
+  AbsExpr_Container() = default;
+  AbsExpr_Container(const AbsExpr_Container &other):
+    AbstractNode() {
+    expr = other.expr;
+  }
 
-  AbsExpr_Container(const expression::AbstractExpression *expr) {
-    node = expr;
+  AbsExpr_Container(std::shared_ptr<expression::AbstractExpression> expr_) {
+    expr = expr_;
+  }
+
+  OpType GetOpType() const {
+    return OpType::Undefined;
   }
 
   // Return operator type
-  ExpressionType GetType() const {
+  ExpressionType GetExpType() const {
     if (IsDefined()) {
-      return node->GetExpressionType();
+      return expr->GetExpressionType();
     }
     return ExpressionType::INVALID;
   }
 
-  const expression::AbstractExpression *GetExpr() const {
-    return node;
+  std::shared_ptr<expression::AbstractExpression> GetExpr() const {
+    return expr;
+  }
+
+  // Dummy Accept
+  void Accept(OperatorVisitor *v) const {
+    (void)v;
+    PELOTON_ASSERT(0);
   }
 
   // Operator contains Logical node
@@ -71,38 +76,39 @@ class AbsExpr_Container {
 
   std::string GetName() const {
     if (IsDefined()) {
-      return node->GetExpressionName();
+      return expr->GetExpressionName();
     }
 
     return "Undefined";
   }
 
   hash_t Hash() const {
     if (IsDefined()) {
-      return node->Hash();
+      return expr->Hash();
     }
     return 0;
   }
 
+  bool operator==(const AbstractNode &r) {
+    if (r.GetExpType() != ExpressionType::INVALID) {
+      const AbsExpr_Container &cnt = dynamic_cast<const AbsExpr_Container&>(r);
+      return (*this == cnt);
+    }
+
+    return false;
+  }
+
   bool operator==(const AbsExpr_Container &r) {
     if (IsDefined() && r.IsDefined()) {
-      // (TODO): need a better way to determine deep equality
-
-      // NOTE:
-      // Without proper equality determinations, the groups will
-      // not be assigned correctly. Arguably, terrier does this
-      // better because a blind ExactlyEquals on different types
-      // of ConstantValueExpression under Peloton will crash!
-
-      // For now, just return (false).
-      // I don't anticipate this will affect correctness, just
-      // performance, since duplicate trees will have to evaluated
-      // over and over again, rather than being able to "borrow"
-      // a previous tree's rewrite.
-      //
-      // Probably not worth to create a "validator" since porting
-      // this to terrier anyways (?). == does not check Value
-      // so it's broken. ExactlyEqual requires precondition checking.
+      //TODO(): proper equality check when migrate to terrier
+      // Equality check relies on performing the following:
+      // - Check each node's ExpressionType
+      // - Check other parameters for a given node
+      // We believe that in terrier so long as the AbstractExpression
+      // are children-less, operator== provides sufficient checking.
+      // The reason behind why the children-less guarantee is required,
+      // is that the "real" children are actually tracked by the
+      // AbsExpr_Expression class.
       return false;
     } else if (!IsDefined() && !r.IsDefined()) {
       return true;
@@ -112,72 +118,59 @@ class AbsExpr_Container {
 
   // Operator contains physical or logical operator node
   bool IsDefined() const {
-    return node != nullptr;
-  }
-
-  //(TODO): fix memory management once go to terrier
-  expression::AbstractExpression *Rebuild(std::vector<expression::AbstractExpression*> children) {
-    switch (GetType()) {
-      case ExpressionType::COMPARE_EQUAL:
-      case ExpressionType::COMPARE_NOTEQUAL:
-      case ExpressionType::COMPARE_LESSTHAN:
-      case ExpressionType::COMPARE_GREATERTHAN:
-      case ExpressionType::COMPARE_LESSTHANOREQUALTO:
-      case ExpressionType::COMPARE_GREATERTHANOREQUALTO:
-      case ExpressionType::COMPARE_LIKE:
-      case ExpressionType::COMPARE_NOTLIKE:
-      case ExpressionType::COMPARE_IN:
-      case ExpressionType::COMPARE_DISTINCT_FROM: {
-        PELOTON_ASSERT(children.size() == 2);
-        return new expression::ComparisonExpression(GetType(), children[0], children[1]);
-      }
-      case ExpressionType::CONJUNCTION_AND:
-      case ExpressionType::CONJUNCTION_OR: {
-        PELOTON_ASSERT(children.size() == 2);
-        return new expression::ConjunctionExpression(GetType(), children[0], children[1]);
-      }
-      case ExpressionType::VALUE_CONSTANT: {
-        PELOTON_ASSERT(children.size() == 0);
-        auto cve = static_cast<const expression::ConstantValueExpression*>(node);
-        return new expression::ConstantValueExpression(cve->GetValue());
-      }
-      default: {
-        int type = static_cast<int>(GetType());
-        LOG_ERROR("Unimplemented Rebuild() for %d found", type);
-        return nullptr;
-      }
-    }
+    return expr != nullptr;
   }
 
+  //TODO(): Function should use std::shared_ptr when migrate to terrier
+  expression::AbstractExpression *CopyWithChildren(std::vector<expression::AbstractExpression*> children);
+
  private:
-  const expression::AbstractExpression *node;
+  // Internal wrapped AbstractExpression
+  std::shared_ptr<expression::AbstractExpression> expr;
 };
 
-class AbsExpr_Expression {
+
+class AbsExpr_Expression: public AbstractNodeExpression {
  public:
-  AbsExpr_Expression(AbsExpr_Container op): op(op) {};
+  AbsExpr_Expression(std::shared_ptr<AbstractNode> n) {
+    std::shared_ptr<AbsExpr_Container> cnt = std::dynamic_pointer_cast<AbsExpr_Container>(n);
+    PELOTON_ASSERT(cnt != nullptr);
+
+    node = n;
+  }
 
-  void PushChild(std::shared_ptr<AbsExpr_Expression> op) {
+  // Disallow copy and move constructor
+  DISALLOW_COPY_AND_MOVE(AbsExpr_Expression);
+
+  void PushChild(std::shared_ptr<AbstractNodeExpression> op) {
     children.push_back(op);
   }
 
   void PopChild() {
     children.pop_back();
   }
 
-  const std::vector<std::shared_ptr<AbsExpr_Expression>> &Children() const {
+  const std::vector<std::shared_ptr<AbstractNodeExpression>> &Children() const {
     return children;
   }
 
-  const AbsExpr_Container &Op() const {
-    return op;
+  const std::shared_ptr<AbstractNode> Node() const {
+    // Integrity constraint
+    std::shared_ptr<AbsExpr_Container> cnt = std::dynamic_pointer_cast<AbsExpr_Container>(node);
+    PELOTON_ASSERT(cnt != nullptr);
+
+    return node;
+  }
+
+  const std::string GetInfo() const {
+    //TODO(): create proper info statement?
+    return "";
   }
 
  private:
-  AbsExpr_Container op;
-  std::vector<std::shared_ptr<AbsExpr_Expression>> children;
+  std::shared_ptr<AbstractNode> node;
+  std::vector<std::shared_ptr<AbstractNodeExpression>> children;
 };
 
 }  // namespace optimizer
 }  // namespace peloton
-

src/include/optimizer/abstract_node.h

@@ -83,6 +83,7 @@ enum class OpType {
 class OperatorVisitor;
 
 struct AbstractNode {
+  AbstractNode() {}
   AbstractNode(std::shared_ptr<AbstractNode> node) : node(node) {}
 
   ~AbstractNode() {}