Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/237#discussion_r112848194
  
    --- Diff: query_optimizer/rules/ExtractCommonSubexpression.hpp ---
    @@ -0,0 +1,179 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *   http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + **/
    +
    +#ifndef QUICKSTEP_QUERY_OPTIMIZER_RULES_EXTRACT_COMMON_SUBEXPRESSION_HPP_
    +#define QUICKSTEP_QUERY_OPTIMIZER_RULES_EXTRACT_COMMON_SUBEXPRESSION_HPP_
    +
    +#include <cstddef>
    +#include <functional>
    +#include <memory>
    +#include <string>
    +#include <type_traits>
    +#include <unordered_map>
    +#include <unordered_set>
    +#include <vector>
    +
    +#include "query_optimizer/expressions/CommonSubexpression.hpp"
    +#include "query_optimizer/expressions/Expression.hpp"
    +#include "query_optimizer/expressions/ExpressionType.hpp"
    +#include "query_optimizer/expressions/Scalar.hpp"
    +#include "query_optimizer/physical/Physical.hpp"
    +#include "query_optimizer/rules/Rule.hpp"
    +#include "utility/Macros.hpp"
    +
    +namespace quickstep {
    +namespace optimizer {
    +
    +class OptimizerContext;
    +
    +/** \addtogroup OptimizerRules
    + *  @{
    + */
    +
    +/**
    + * @brief Rule that applies to a physical plan to identify and label common
    + *        subexpressions.
    + *
    + * @note This is essentially a logical optimization pass. However, we need 
some
    + *       of the physical passes (e.g. ReuseAggregateExpressions) to be 
finalized
    + *       before this one to maximize optimization opportunities.
    + */
    +class ExtractCommonSubexpression : public Rule<physical::Physical> {
    + public:
    +  /**
    +   * @brief Constructor.
    +   *
    +   * @param optimizer_context The optimizer context.
    +   */
    +  explicit ExtractCommonSubexpression(OptimizerContext *optimizer_context);
    +
    +  ~ExtractCommonSubexpression() override {}
    +
    +  std::string getName() const override {
    +    return "ExtractCommonSubexpression";
    +  }
    +
    +  physical::PhysicalPtr apply(const physical::PhysicalPtr &input) override;
    +
    + private:
    +  physical::PhysicalPtr applyInternal(const physical::PhysicalPtr &input);
    +
    +  struct ScalarHash {
    +    inline std::size_t operator()(const expressions::ScalarPtr &scalar) 
const {
    +      return scalar->hash();
    +    }
    +  };
    +
    +  struct ScalarEqual {
    +    inline bool operator()(const expressions::ScalarPtr &lhs,
    +                           const expressions::ScalarPtr &rhs) const {
    +      return lhs->equals(rhs);
    +    }
    +  };
    +
    +  // For memorizing whether a subexpression is hashable.
    +  using ScalarHashable = std::unordered_set<expressions::ScalarPtr>;
    +
    +  // For counting the number of occurrences of each subexpression.
    +  using ScalarCounter =
    +      std::unordered_map<expressions::ScalarPtr, std::size_t, ScalarHash, 
ScalarEqual>;
    +
    +  // For mapping each subexpression to its transformed version.
    +  using ScalarMap =
    +      std::unordered_map<expressions::ScalarPtr,
    +                         expressions::CommonSubexpressionPtr,
    +                         ScalarHash,
    +                         ScalarEqual>;
    +
    +  std::vector<expressions::ExpressionPtr> transformExpressions(
    +      const std::vector<expressions::ExpressionPtr> &expressions);
    +
    +  expressions::ExpressionPtr transformExpression(
    +      const expressions::ExpressionPtr &expression);
    +
    +  // Traverse the expression tree and increase the count of each 
subexpression.
    +  bool visitAndCount(
    +      const expressions::ExpressionPtr &expression,
    +      ScalarCounter *counter,
    +      ScalarHashable *hashable);
    +
    +  // Traverse the expression tree and transform subexpressions (to common
    +  // subexpressions) if applicable.
    +  expressions::ExpressionPtr visitAndTransform(
    --- End diff --
    
    I believe multiple methods above including `transformExpressions ` could be 
marked as `const`, as they do not modify the data members in the class.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to