refactor: Move toSubfieldFilter API to PlanBuilder (facebookincubator#15556)

Summary: Pull Request resolved: facebookincubator#15556

Reviewed By: Yuhta

Differential Revision: D87386102

fbshipit-source-id: 739b19ef834ec76e7457e56974be1872cc18c4ff

Author: mbasmanova

velox/exec/tests/utils/PlanBuilder.cpp

@@ -191,6 +191,58 @@ PlanBuilder& PlanBuilder::tpcdsTableScan(
       .endTableScan();
 }
 
+namespace {
+
+// Analyzes 'expr' to determine if it can be expressed as a subfield filter.
+// Returns a pair of subfield and filter if so. Otherwise, throws.
+//
+// Supports all expressions supported by
+// exec::ExprToSubfieldFilterParser::leafCallToSubfieldFilter + negations and
+// disjunctions over same subfield.
+//
+//  Examples:
+//    a = 1
+//    a = 1 OR a > 10
+//    not (a = 1)
+std::pair<common::Subfield, std::unique_ptr<common::Filter>> toSubfieldFilter(
+    const core::TypedExprPtr& expr,
+    core::ExpressionEvaluator* evaluator) {
+  if (expr->isCallKind();
+      auto* call = expr->asUnchecked<core::CallTypedExpr>()) {
+    if (call->name() == "or") {
+      VELOX_CHECK_EQ(call->inputs().size(), 2);
+      auto left = toSubfieldFilter(call->inputs()[0], evaluator);
+      auto right = toSubfieldFilter(call->inputs()[1], evaluator);
+      VELOX_CHECK(left.first == right.first);
+      return {
+          std::move(left.first),
+          exec::ExprToSubfieldFilterParser::makeOrFilter(
+              std::move(left.second), std::move(right.second))};
+    }
+
+    if (call->name() == "not") {
+      const auto& input = call->inputs()[0];
+      if (input->isCallKind();
+          auto* inner = input->asUnchecked<core::CallTypedExpr>()) {
+        if (auto result =
+                exec::ExprToSubfieldFilterParser::getInstance()
+                    ->leafCallToSubfieldFilter(*inner, evaluator, true)) {
+          return std::move(result.value());
+        }
+      }
+    } else {
+      if (auto result =
+              exec::ExprToSubfieldFilterParser::getInstance()
+                  ->leafCallToSubfieldFilter(*call, evaluator, false)) {
+        return std::move(result.value());
+      }
+    }
+  }
+  VELOX_UNSUPPORTED(
+      "Unsupported expression for range filter: {}", expr->toString());
+}
+} // namespace
+
 PlanBuilder::TableScanBuilder& PlanBuilder::TableScanBuilder::subfieldFilters(
     std::vector<std::string> subfieldFilters) {
   VELOX_CHECK(subfieldFiltersMap_.empty());
@@ -210,9 +262,7 @@ PlanBuilder::TableScanBuilder& PlanBuilder::TableScanBuilder::subfieldFilters(
     // Parse directly to subfieldFiltersMap_
     auto filterExpr = core::Expressions::inferTypes(
         untypedExpr, parseType, planBuilder_.pool_);
-    auto [subfield, subfieldFilter] =
-        exec::ExprToSubfieldFilterParser::getInstance()->toSubfieldFilter(
-            filterExpr, &evaluator);
+    auto [subfield, subfieldFilter] = toSubfieldFilter(filterExpr, &evaluator);
 
     auto it = columnAliases_.find(subfield.toString());
     if (it != columnAliases_.end()) {

velox/expression/ExprToSubfieldFilter.cpp

@@ -369,6 +369,7 @@ std::unique_ptr<common::Filter> ExprToSubfieldFilterParser::makeInFilter(
     case TypeKind::VARCHAR: {
       auto stringElements = elements->as<SimpleVector<StringView>>();
       std::vector<std::string> values;
+      values.reserve(size);
       for (auto i = 0; i < size; i++) {
         values.push_back(std::string(stringElements->valueAt(offset + i)));
       }
@@ -557,38 +558,4 @@ PrestoExprToSubfieldFilterParser::leafCallToSubfieldFilter(
   return std::nullopt;
 }
 
-std::pair<common::Subfield, std::unique_ptr<common::Filter>>
-PrestoExprToSubfieldFilterParser::toSubfieldFilter(
-    const core::TypedExprPtr& expr,
-    core::ExpressionEvaluator* evaluator) {
-  if (expr->isCallKind();
-      auto* call = expr->asUnchecked<core::CallTypedExpr>()) {
-    if (call->name() == "or") {
-      VELOX_CHECK_EQ(call->inputs().size(), 2);
-      auto left = toSubfieldFilter(call->inputs()[0], evaluator);
-      auto right = toSubfieldFilter(call->inputs()[1], evaluator);
-      VELOX_CHECK(left.first == right.first);
-      return {
-          std::move(left.first),
-          makeOrFilter(std::move(left.second), std::move(right.second))};
-    }
-
-    if (call->name() == "not") {
-      const auto& input = call->inputs()[0];
-      if (input->isCallKind();
-          auto* inner = input->asUnchecked<core::CallTypedExpr>()) {
-        if (auto result = leafCallToSubfieldFilter(*inner, evaluator, true)) {
-          return std::move(result.value());
-        }
-      }
-    } else {
-      if (auto result = leafCallToSubfieldFilter(*call, evaluator, false)) {
-        return std::move(result.value());
-      }
-    }
-  }
-  VELOX_UNSUPPORTED(
-      "Unsupported expression for range filter: {}", expr->toString());
-}
-
 } // namespace facebook::velox::exec

velox/expression/ExprToSubfieldFilter.h

@@ -424,24 +424,6 @@ class ExprToSubfieldFilterParser {
     parser_ = std::move(parser);
   }
 
-  /// Test-only API. Do not use in production code.
-  ///
-  /// Analyzes 'expr' to determine if it can be expressed as a subfield filter.
-  /// Returns a pair of subfield and filter if so. Otherwise, throws.
-  ///
-  /// Supports all expressions supported by leafCallToSubfieldFilter + negations
-  /// and disjunctions over same subfield.
-  ///  Examples:
-  ///    a = 1
-  ///    a = 1 OR a > 10
-  ///    not (a = 1)
-  ///
-  /// TODO Improve the API by returning std::optional instead of throwing.
-  virtual std::pair<common::Subfield, std::unique_ptr<common::Filter>>
-  toSubfieldFilter(
-      const core::TypedExprPtr& expr,
-      core::ExpressionEvaluator*) = 0;
-
   /// Analyzes 'call' expression to determine if it can be expressed as a
   /// subfield filter. Returns the subfield and filter if so. Otherwise, returns
   /// std::nullopt. If 'negated' is true, considers the negation of 'call'
@@ -454,6 +436,10 @@ class ExprToSubfieldFilterParser {
       core::ExpressionEvaluator* evaluator,
       bool negated = false) = 0;
 
+  static std::unique_ptr<common::Filter> makeOrFilter(
+      std::unique_ptr<common::Filter> a,
+      std::unique_ptr<common::Filter> b);
+
  protected:
   // Converts an expression into a subfield. Returns false if the expression
   // is not a valid field expression.
@@ -505,10 +491,6 @@ class ExprToSubfieldFilterParser {
       core::ExpressionEvaluator* evaluator,
       bool negated);
 
-  static std::unique_ptr<common::Filter> makeOrFilter(
-      std::unique_ptr<common::Filter> a,
-      std::unique_ptr<common::Filter> b);
-
  private:
   // Singleton parser instance.
   static std::shared_ptr<ExprToSubfieldFilterParser> parser_;
@@ -517,10 +499,6 @@ class ExprToSubfieldFilterParser {
 // Parser for Presto expressions.
 class PrestoExprToSubfieldFilterParser : public ExprToSubfieldFilterParser {
  public:
-  std::pair<common::Subfield, std::unique_ptr<common::Filter>> toSubfieldFilter(
-      const core::TypedExprPtr& expr,
-      core::ExpressionEvaluator* evaluator) override;
-
   std::optional<std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
   leafCallToSubfieldFilter(
       const core::CallTypedExpr& call,

velox/expression/tests/ExprToSubfieldFilterTest.cpp

@@ -77,12 +77,6 @@ class ExprToSubfieldFilterTest : public testing::Test {
     return std::make_pair(common::Subfield(), nullptr);
   }
 
-  std::pair<common::Subfield, std::unique_ptr<common::Filter>> toSubfieldFilter(
-      const core::TypedExprPtr& expr) {
-    return ExprToSubfieldFilterParser::getInstance()->toSubfieldFilter(
-        expr, evaluator());
-  }
-
  private:
   std::shared_ptr<memory::MemoryPool> pool_ =
       memory::memoryManager()->addLeafPool();
@@ -217,55 +211,6 @@ TEST_F(ExprToSubfieldFilterTest, isNull) {
   ASSERT_TRUE(filter->testNull());
 }
 
-TEST_F(ExprToSubfieldFilterTest, isNotNull) {
-  auto call = parseCallExpr("a is not null", ROW({{"a", BIGINT()}}));
-  auto [subfield, filter] = toSubfieldFilter(call);
-
-  ASSERT_TRUE(filter);
-  validateSubfield(subfield, {"a"});
-  ASSERT_TRUE(filter->testInt64(0));
-  ASSERT_TRUE(filter->testInt64(42));
-  ASSERT_FALSE(filter->testNull());
-}
-
-TEST_F(ExprToSubfieldFilterTest, or) {
-  auto call = parseExpr("a = 42 OR a = 43", ROW({{"a", BIGINT()}}));
-  auto [subfield, filter] = toSubfieldFilter(call);
-
-  ASSERT_TRUE(filter);
-  validateSubfield(subfield, {"a"});
-  ASSERT_TRUE(filter->testInt64(42));
-  ASSERT_TRUE(filter->testInt64(43));
-  ASSERT_FALSE(filter->testInt64(41));
-  ASSERT_FALSE(filter->testNull());
-}
-
-TEST_F(ExprToSubfieldFilterTest, unsupported) {
-  const auto type = ROW({"a", "b", "c"}, BIGINT());
-
-  VELOX_ASSERT_THROW(
-      toSubfieldFilter(parseExpr("123", type)),
-      "Unsupported expression for range filter");
-
-  VELOX_ASSERT_THROW(
-      toSubfieldFilter(parseCallExpr("a + b", type)),
-      "Unsupported expression for range filter");
-
-  VELOX_ASSERT_THROW(
-      toSubfieldFilter(parseCallExpr("(a + b > 0) OR (c = 1)", type)),
-      "Unsupported expression for range filter");
-
-  // TODO Improve error message for this specific use case. Then use
-  // VELOX_ASSERT_THROW.
-  EXPECT_THROW(
-      toSubfieldFilter(parseCallExpr("a = 1 OR c = 2", type)),
-      VeloxRuntimeError);
-
-  VELOX_ASSERT_THROW(
-      toSubfieldFilter(parseCallExpr("a = 1 AND b = 2", type)),
-      "Unsupported expression for range filter");
-}
-
 TEST_F(ExprToSubfieldFilterTest, like) {
   auto call = parseCallExpr("a like 'foo%'", ROW({{"a", VARCHAR()}}));
   auto [subfield, filter] = leafCallToSubfieldFilter(call);
@@ -306,115 +251,5 @@ TEST_F(ExprToSubfieldFilterTest, dereferenceWithEmptyField) {
   ASSERT_FALSE(filter);
 }
 
-class CustomExprToSubfieldFilterParser : public ExprToSubfieldFilterParser {
- public:
-  std::pair<common::Subfield, std::unique_ptr<common::Filter>> toSubfieldFilter(
-      const core::TypedExprPtr& expr,
-      core::ExpressionEvaluator* evaluator) override {
-    if (expr->isCallKind();
-        auto* call = expr->asUnchecked<core::CallTypedExpr>()) {
-      if (call->name() == "or") {
-        auto left = toSubfieldFilter(call->inputs()[0], evaluator);
-        auto right = toSubfieldFilter(call->inputs()[1], evaluator);
-        VELOX_CHECK(left.first == right.first);
-        return {
-            std::move(left.first),
-            makeOrFilter(std::move(left.second), std::move(right.second))};
-      }
-      if (call->name() == "not") {
-        if (auto* inner =
-                call->inputs()[0]->asUnchecked<core::CallTypedExpr>()) {
-          if (auto result = leafCallToSubfieldFilter(*inner, evaluator, true)) {
-            return std::move(result.value());
-          }
-        }
-      } else {
-        if (auto result = leafCallToSubfieldFilter(*call, evaluator, false)) {
-          return std::move(result.value());
-        }
-      }
-    }
-    VELOX_UNSUPPORTED(
-        "Unsupported expression for range filter: {}", expr->toString());
-  }
-
-  std::optional<std::pair<common::Subfield, std::unique_ptr<common::Filter>>>
-  leafCallToSubfieldFilter(
-      const core::CallTypedExpr& call,
-      core::ExpressionEvaluator* evaluator,
-      bool negated) override {
-    if (call.inputs().empty()) {
-      return std::nullopt;
-    }
-
-    const auto* leftSide = call.inputs()[0].get();
-
-    common::Subfield subfield;
-    if (call.name() == "custom_eq") {
-      if (toSubfield(leftSide, subfield)) {
-        auto filter = negated ? makeNotEqualFilter(call.inputs()[1], evaluator)
-                              : makeEqualFilter(call.inputs()[1], evaluator);
-        if (filter != nullptr) {
-          return std::make_pair(std::move(subfield), std::move(filter));
-        }
-        return std::nullopt;
-      }
-    } else if (call.name() == "is_null") {
-      if (toSubfield(call.inputs()[0].get(), subfield)) {
-        if (negated) {
-          return std::make_pair(std::move(subfield), isNotNull());
-        }
-        return std::make_pair(std::move(subfield), isNull());
-      }
-    }
-    return std::nullopt;
-  }
-};
-
-class CustomExprToSubfieldFilterTest : public ExprToSubfieldFilterTest {
- public:
-  static void SetUpTestSuite() {
-    functions::prestosql::registerAllScalarFunctions("custom_");
-    functions::registerIsNullFunction("is_null");
-    parse::registerTypeResolver();
-    memory::MemoryManager::testingSetInstance(memory::MemoryManager::Options{});
-    ExprToSubfieldFilterParser::registerParser(
-        std::make_unique<CustomExprToSubfieldFilterParser>());
-  }
-
-  static void TearDownTestSuite() {
-    ExprToSubfieldFilterParser::registerParser(
-        std::make_unique<PrestoExprToSubfieldFilterParser>());
-  }
-};
-
-TEST_F(CustomExprToSubfieldFilterTest, isNull) {
-  auto call = parseCallExpr("a is null", ROW({{"a", BIGINT()}}));
-  auto [subfield, filter] = toSubfieldFilter(call);
-  ASSERT_TRUE(filter);
-  validateSubfield(subfield, {"a"});
-  ASSERT_FALSE(filter->testInt64(0));
-  ASSERT_FALSE(filter->testInt64(42));
-  ASSERT_TRUE(filter->testNull());
-}
-
-TEST_F(CustomExprToSubfieldFilterTest, eq) {
-  auto call = parseCallExpr("custom_eq(a, 42)", ROW({{"a", BIGINT()}}));
-  auto [subfield, filter] = toSubfieldFilter(call);
-  ASSERT_TRUE(filter);
-  validateSubfield(subfield, {"a"});
-  auto bigintRange = dynamic_cast<common::BigintRange*>(filter.get());
-  ASSERT_TRUE(bigintRange);
-  ASSERT_EQ(bigintRange->lower(), 42);
-  ASSERT_EQ(bigintRange->upper(), 42);
-  ASSERT_FALSE(bigintRange->testNull());
-}
-
-TEST_F(CustomExprToSubfieldFilterTest, unsupported) {
-  auto call = parseCallExpr("custom_neq(a, 42)", ROW({{"a", BIGINT()}}));
-  VELOX_ASSERT_USER_THROW(
-      toSubfieldFilter(call), "Unsupported expression for range filter");
-}
-
 } // namespace
 } // namespace facebook::velox::exec