Refactor query_to_operator_transformer a bit to support better error handling

Author: chenboy

src/common/internal_types.cpp

@@ -929,6 +929,9 @@ std::string ExpressionTypeToString(ExpressionType type, bool short_str) {
     case ExpressionType::CAST: {
       return ("CAST");
     }
+    case ExpressionType::OPERATOR_IS_NOT_NULL: {
+      return ("IS_NOT_NULL");
+    }
     default: {
       throw ConversionException(StringUtil::Format(
           "No string conversion for ExpressionType value '%d'",

src/include/optimizer/query_to_operator_transformer.h

@@ -66,6 +66,7 @@ class QueryToOperatorTransformer : public SqlNodeVisitor {
   void Visit(expression::ComparisonExpression *expr) override;
   void Visit(expression::OperatorExpression *expr) override;
 
+ private:
   inline oid_t GetAndIncreaseGetId() { return get_id++; }
 
   /**
@@ -96,14 +97,28 @@ class QueryToOperatorTransformer : public SqlNodeVisitor {
    * @return If the expression could be transformed into sub-query, return true,
    *  return false otherwise
    */
-  bool GenerateSubquerytree(
-      expression::AbstractExpression *expr,
-      std::vector<expression::AbstractExpression *> &select_list,
-      bool single_join = false);
+  bool GenerateSubquerytree(expression::AbstractExpression *expr,
+                            oid_t child_id, bool single_join = false);
 
+  /**
+   * @brief Decide if a conjunctive predicate is supported. We need to extract
+   * conjunction predicate first then call this function to decide if the
+   * predicate is supported by our system
+   *
+   * @param expr The conjunctive predicate provided
+   *
+   * @return True if supported, false otherwise
+   */
+  bool IsSupportedConjunctivePredicate(expression::AbstractExpression *expr);
+  /**
+   * @brief Check if a sub-select statement is supported.
+   *
+   * @param op The select statement
+   *
+   * @return True if supported, false otherwise
+   */
+  bool IsSupportedSubSelect(const parser::SelectStatement *op);
   static bool RequireAggregation(const parser::SelectStatement *op);
-
- private:
   std::shared_ptr<OperatorExpression> output_expr_;
 
   concurrency::TransactionContext *txn_;

src/include/planner/abstract_scan_plan.h

@@ -74,6 +74,10 @@ class AbstractScan : public AbstractPlan {
 
   void SetForUpdateFlag(bool flag) { is_for_update = flag; }
 
+  const std::string GetPredicateInfo() const {
+    return predicate_ != nullptr ? predicate_->GetInfo() : "";
+  }
+
  private:
   // Pointer to table to scan from
   storage::DataTable *target_table_ = nullptr;

src/include/planner/seq_scan_plan.h

@@ -53,7 +53,9 @@ class SeqScanPlan : public AbstractScan {
     return PlanNodeType::SEQSCAN;
   }
 
-  const std::string GetInfo() const override { return "SeqScan"; }
+  const std::string GetInfo() const override {
+    return "SeqScan(" + GetPredicateInfo() + ")";
+  }
 
   void SetParameterValues(std::vector<type::Value> *values) override;
 
@@ -81,7 +83,8 @@ class SeqScanPlan : public AbstractScan {
     return !(*this == rhs);
   }
 
-  virtual void VisitParameters(codegen::QueryParametersMap &map,
+  virtual void VisitParameters(
+      codegen::QueryParametersMap &map,
       std::vector<peloton::type::Value> &values,
       const std::vector<peloton::type::Value> &values_from_user) override;
 

src/optimizer/query_to_operator_transformer.cpp

@@ -304,15 +304,9 @@ void QueryToOperatorTransformer::Visit(
 void QueryToOperatorTransformer::Visit(expression::ComparisonExpression *expr) {
   auto expr_type = expr->GetExpressionType();
   if (expr->GetExpressionType() == ExpressionType::COMPARE_IN) {
-    std::vector<expression::AbstractExpression *> select_list;
-    if (GenerateSubquerytree(expr->GetModifiableChild(1), select_list) ==
-        true) {
-      if (select_list.size() != 1) {
-        throw Exception("Array in predicates not supported");
-      }
-
-      // Set the right child as the output of the subquery
-      expr->SetChild(1, select_list.at(0)->Copy());
+    if (GenerateSubquerytree(expr, 1)) {
+      // TODO(boweic): Should use IN to preserve the semantic, for now we do not
+      // have semi-join so use = to transform into inner join
       expr->SetExpressionType(ExpressionType::COMPARE_EQUAL);
     }
 
@@ -321,40 +315,25 @@ void QueryToOperatorTransformer::Visit(expression::ComparisonExpression *expr) {
              expr_type == ExpressionType::COMPARE_GREATERTHANOREQUALTO ||
              expr_type == ExpressionType::COMPARE_LESSTHAN ||
              expr_type == ExpressionType::COMPARE_LESSTHANOREQUALTO) {
-    std::vector<expression::AbstractExpression *> select_list;
-    if (GenerateSubquerytree(expr->GetModifiableChild(0), select_list, true) ==
-        true) {
-      if (select_list.size() != 1) {
-        throw Exception("Array in predicates not supported");
-      }
-
-      // Set the left child as the output of the subquery
-      expr->SetChild(0, select_list.at(0)->Copy());
-    }
-    select_list.clear();
-    if (GenerateSubquerytree(expr->GetModifiableChild(1), select_list, true) ==
-        true) {
-      if (select_list.size() != 1) {
-        throw Exception("Array in predicates not supported");
-      }
-
-      // Set the right child as the output of the subquery
-      expr->SetChild(1, select_list.at(0)->Copy());
+    if (expr->GetChild(0)->GetExpressionType() ==
+            ExpressionType::ROW_SUBQUERY &&
+        expr->GetChild(1)->GetExpressionType() ==
+            ExpressionType::ROW_SUBQUERY) {
+      throw Exception("Do not support comparison between sub-select");
     }
+    // Transform if either child is sub-query
+    GenerateSubquerytree(expr, 0, true) || GenerateSubquerytree(expr, 1, true);
   }
   expr->AcceptChildren(this);
 }
 
 void QueryToOperatorTransformer::Visit(expression::OperatorExpression *expr) {
+  // TODO(boweic): We may want to do the rewrite (exist -> in) in the binder
   if (expr->GetExpressionType() == ExpressionType::OPERATOR_EXISTS) {
-    std::vector<expression::AbstractExpression *> select_list;
-    if (GenerateSubquerytree(expr->GetModifiableChild(0), select_list) ==
-        true) {
-      PL_ASSERT(!select_list.empty());
-
-      // Set the right child as the output of the subquery
+    if (GenerateSubquerytree(expr, 0)) {
+      // Already reset the child to column, we need to transform exist to
+      // not-null to preserve semantic
       expr->SetExpressionType(ExpressionType::OPERATOR_IS_NOT_NULL);
-      expr->SetChild(0, select_list.at(0)->Copy());
     }
   }
 
@@ -391,24 +370,117 @@ bool QueryToOperatorTransformer::RequireAggregation(
 
 void QueryToOperatorTransformer::CollectPredicates(
     expression::AbstractExpression *expr) {
+  // First check if all conjunctive predicates are supported before
+  // transfoming
+  // predicate with sub-select into regular predicates
+  std::vector<expression::AbstractExpression *> predicates;
+  util::SplitPredicates(expr, predicates);
+  for (const auto &pred : predicates) {
+    if (!IsSupportedConjunctivePredicate(pred)) {
+      throw Exception("Predicate type not supported yet");
+    }
+  }
+  // Accept will change the expression, e.g. (a in (select b from test)) into
+  // (a IN test.b), after the rewrite, we can extract the table aliases
+  // information correctly
   expr->Accept(this);
   util::ExtractPredicates(expr, predicates_);
 }
 
-bool QueryToOperatorTransformer::GenerateSubquerytree(
-    expression::AbstractExpression *expr,
-    std::vector<expression::AbstractExpression *> &select_list,
-    bool single_join) {
-  if (expr->GetExpressionType() != ExpressionType::ROW_SUBQUERY) {
-    return false;
+bool QueryToOperatorTransformer::IsSupportedConjunctivePredicate(
+    expression::AbstractExpression *expr) {
+  // Currently support : 1. expr without subquery
+  // 2. subquery without disjunction. Since the expr is already one of the
+  // conjunctive exprs, we'll only need to check if the root level is an
+  // operator with subquery
+  if (!expr->HasSubquery()) {
+    return true;
+  }
+  auto expr_type = expr->GetExpressionType();
+  // Subquery with IN
+  if (expr_type == ExpressionType::COMPARE_IN &&
+      expr->GetChild(1)->GetExpressionType() == ExpressionType::ROW_SUBQUERY) {
+    return true;
+  }
+  // Subquery with EXIST
+  if (expr_type == ExpressionType::OPERATOR_EXISTS &&
+      expr->GetChild(0)->GetExpressionType() == ExpressionType::ROW_SUBQUERY) {
+    return true;
+  }
+  // Subquery with other operator
+  if (expr_type == ExpressionType::COMPARE_EQUAL ||
+      expr_type == ExpressionType::COMPARE_GREATERTHAN ||
+      expr_type == ExpressionType::COMPARE_GREATERTHANOREQUALTO ||
+      expr_type == ExpressionType::COMPARE_LESSTHAN ||
+      expr_type == ExpressionType::COMPARE_LESSTHANOREQUALTO) {
+    // Supported is one child is subquery and the other is not
+    if ((!expr->GetChild(0)->HasSubquery() &&
+         expr->GetChild(1)->GetExpressionType() ==
+             ExpressionType::ROW_SUBQUERY) ||
+        (!expr->GetChild(1)->HasSubquery() &&
+         expr->GetChild(0)->GetExpressionType() ==
+             ExpressionType::ROW_SUBQUERY)) {
+      return true;
+    }
   }
-  auto subquery_expr = dynamic_cast<expression::SubqueryExpression *>(expr);
-  auto sub_select = subquery_expr->GetSubSelect();
+  return false;
+}
 
-  for (auto &ele : sub_select->select_list) {
-    select_list.push_back(ele.get());
+bool QueryToOperatorTransformer::IsSupportedSubSelect(
+    const parser::SelectStatement *op) {
+  // Supported if 1. No aggregation. 2. With aggregation and WHERE clause only
+  // have correlated columns in conjunctive predicates in the form of
+  // "outer_relation.a = ..."
+  // TODO(boweic): Add support for arbitary expressions, this would require
+  // the
+  // support for mark join & some special operators, see Hyper's unnesting
+  // arbitary query slides
+  if (!RequireAggregation(op)) {
+    return true;
+  }
+
+  std::vector<expression::AbstractExpression *> predicates;
+  util::SplitPredicates(op->where_clause.get(), predicates);
+  for (const auto &pred : predicates) {
+    // If correlated predicate
+    if (pred->GetDepth() < op->depth) {
+      if (pred->GetExpressionType() != ExpressionType::COMPARE_EQUAL) {
+        return false;
+      }
+      // Check if in the form of
+      // "outer_relation.a = (expr only columns in inner relation)"
+      if (!((pred->GetChild(0)->GetDepth() == op->depth &&
+             pred->GetChild(0)->GetExpressionType() ==
+                 ExpressionType::VALUE_TUPLE) ||
+            (pred->GetChild(1)->GetDepth() == op->depth &&
+             pred->GetChild(1)->GetExpressionType() ==
+                 ExpressionType::VALUE_TUPLE))) {
+        return false;
+      }
+    }
   }
+  return true;
+}
 
+bool QueryToOperatorTransformer::GenerateSubquerytree(
+    expression::AbstractExpression *expr, oid_t child_id, bool single_join) {
+  // Get potential subquery
+  auto subquery_expr = expr->GetChild(child_id);
+  if (subquery_expr->GetExpressionType() != ExpressionType::ROW_SUBQUERY) {
+    return false;
+  }
+  auto sub_select =
+      static_cast<const expression::SubqueryExpression *>(subquery_expr)
+          ->GetSubSelect()
+          .get();
+  if (!IsSupportedSubSelect(sub_select)) {
+    throw Exception("Sub-select not supported");
+  }
+  // We only support subselect with single row
+  if (sub_select->select_list.size() != 1) {
+    throw Exception("Array in predicates not supported");
+  }
+  std::vector<expression::AbstractExpression *> select_list;
   // Construct join
   std::shared_ptr<OperatorExpression> op_expr;
   if (single_join) {
@@ -426,6 +498,8 @@ bool QueryToOperatorTransformer::GenerateSubquerytree(
   op_expr->PushChild(output_expr_);
 
   output_expr_ = op_expr;
+  // Convert subquery to the selected column in the sub-select
+  expr->SetChild(child_id, sub_select->select_list.at(0)->Copy());
   return true;
 }
 

src/optimizer/rule_impls.cpp

@@ -261,9 +261,9 @@ void GetToIndexScan::Transform(
                   ->GetValue());
           LOG_TRACE("Value Type: %d",
                     static_cast<int>(
-                      reinterpret_cast<expression::ConstantValueExpression *>(
-                        expr->GetModifiableChild(1))
-                        ->GetValueType()));
+                        reinterpret_cast<expression::ConstantValueExpression *>(
+                            expr->GetModifiableChild(1))
+                            ->GetValueType()));
         } else {
           value_list.push_back(
               type::ValueFactory::GetParameterOffsetValue(
@@ -748,12 +748,14 @@ void PushFilterThroughJoin::Transform(
   // E.g. An expression (test.a = test1.b and test.a = 5) would become
   // {test.a = test1.b, test.a = 5}
   for (auto &predicate : predicates) {
-    if (util::IsSubset(left_group_aliases_set, predicate.table_alias_set))
+    if (util::IsSubset(left_group_aliases_set, predicate.table_alias_set)) {
       left_predicates.emplace_back(predicate);
-    else if (util::IsSubset(right_group_aliases_set, predicate.table_alias_set))
+    } else if (util::IsSubset(right_group_aliases_set,
+                              predicate.table_alias_set)) {
       right_predicates.emplace_back(predicate);
-    else
+    } else {
       join_predicates.emplace_back(predicate);
+    }
   }
 
   // Construct join operator

test/sql/testing_sql_util.cpp

@@ -132,6 +132,7 @@ ResultType TestingSQLUtil::ExecuteSQLQueryWithOptimizer(
     return status.m_result;
   } catch (Exception &e) {
     error_message = e.what();
+    LOG_DEBUG("Testing SQL Util Caught Exception : %s", e.what());
     return ResultType::FAILURE;
   }
 }