Avoid match full scan when has in predicate

fix small fix add test case small change add test cases small fix fmt
vesoft-inc · Oct 21, 2022 · ad746f5 · ad746f5
1 parent 3bafa14
commit ad746f5
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 17 deletions.
diff --git a/src/common/expression/ContainerExpression.h b/src/common/expression/ContainerExpression.h
@@ -6,6 +6,9 @@
 #ifndef COMMON_EXPRESSION_CONTAINEREXPRESSION_H_
 #define COMMON_EXPRESSION_CONTAINEREXPRESSION_H_
 
+#include "common/base/ObjectPool.h"
+#include "common/expression/ConstantExpression.h"
+#include "common/expression/ContainerExpression.h"
 #include "common/expression/Expression.h"
 
 namespace nebula {
@@ -63,7 +66,16 @@ class MapItemList final {
   std::vector<Pair> items_;
 };
 
-class ListExpression final : public Expression {
+class ContainerExpression : public Expression {
+ public:
+  virtual const std::vector<Expression *> getKeys() const = 0;
+  virtual size_t size() const = 0;
+
+ protected:
+  ContainerExpression(ObjectPool *pool, Expression::Kind kind) : Expression(pool, kind) {}
+};
+
+class ListExpression final : public ContainerExpression {
  public:
   ListExpression &operator=(const ListExpression &rhs) = delete;
   ListExpression &operator=(ListExpression &&) = delete;
@@ -84,15 +96,15 @@ class ListExpression final : public Expression {
     items_[index] = item;
   }
 
-  std::vector<Expression *> get() {
+  const std::vector<Expression *> getKeys() const override {
     return items_;
   }
 
   void setItems(std::vector<Expression *> items) {
     items_ = items;
   }
 
-  size_t size() const {
+  size_t size() const override {
     return items_.size();
   }
 
@@ -116,9 +128,9 @@ class ListExpression final : public Expression {
 
  private:
   friend ObjectPool;
-  explicit ListExpression(ObjectPool *pool) : Expression(pool, Kind::kList) {}
+  explicit ListExpression(ObjectPool *pool) : ContainerExpression(pool, Kind::kList) {}
 
-  ListExpression(ObjectPool *pool, ExpressionList *items) : Expression(pool, Kind::kList) {
+  ListExpression(ObjectPool *pool, ExpressionList *items) : ContainerExpression(pool, Kind::kList) {
     items_ = items->get();
   }
 
@@ -131,7 +143,7 @@ class ListExpression final : public Expression {
   Value result_;
 };
 
-class SetExpression final : public Expression {
+class SetExpression final : public ContainerExpression {
  public:
   SetExpression &operator=(const SetExpression &rhs) = delete;
   SetExpression &operator=(SetExpression &&) = delete;
@@ -152,15 +164,15 @@ class SetExpression final : public Expression {
     items_[index] = item;
   }
 
-  std::vector<Expression *> get() {
+  const std::vector<Expression *> getKeys() const override {
     return items_;
   }
 
   void setItems(std::vector<Expression *> items) {
     items_ = items;
   }
 
-  size_t size() const {
+  size_t size() const override {
     return items_.size();
   }
 
@@ -184,9 +196,9 @@ class SetExpression final : public Expression {
 
  private:
   friend ObjectPool;
-  explicit SetExpression(ObjectPool *pool) : Expression(pool, Kind::kSet) {}
+  explicit SetExpression(ObjectPool *pool) : ContainerExpression(pool, Kind::kSet) {}
 
-  SetExpression(ObjectPool *pool, ExpressionList *items) : Expression(pool, Kind::kSet) {
+  SetExpression(ObjectPool *pool, ExpressionList *items) : ContainerExpression(pool, Kind::kSet) {
     items_ = items->get();
   }
 
@@ -199,7 +211,7 @@ class SetExpression final : public Expression {
   Value result_;
 };
 
-class MapExpression final : public Expression {
+class MapExpression final : public ContainerExpression {
  public:
   MapExpression &operator=(const MapExpression &rhs) = delete;
   MapExpression &operator=(MapExpression &&) = delete;
@@ -230,7 +242,15 @@ class MapExpression final : public Expression {
     return items_;
   }
 
-  size_t size() const {
+  const std::vector<Expression *> getKeys() const override {
+    std::vector<Expression *> keys;
+    for (const auto &item : items_) {
+      keys.emplace_back(ConstantExpression::make(pool_, item.first));
+    }
+    return keys;
+  }
+
+  size_t size() const override {
     return items_.size();
   }
 
@@ -254,9 +274,9 @@ class MapExpression final : public Expression {
 
  private:
   friend ObjectPool;
-  explicit MapExpression(ObjectPool *pool) : Expression(pool, Kind::kMap) {}
+  explicit MapExpression(ObjectPool *pool) : ContainerExpression(pool, Kind::kMap) {}
 
-  MapExpression(ObjectPool *pool, MapItemList *items) : Expression(pool, Kind::kMap) {
+  MapExpression(ObjectPool *pool, MapItemList *items) : ContainerExpression(pool, Kind::kMap) {
     items_ = items->get();
   }
 

diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
@@ -4,8 +4,11 @@
 
 #include "graph/util/ExpressionUtils.h"
 
+#include "ExpressionUtils.h"
 #include "common/base/ObjectPool.h"
 #include "common/expression/ArithmeticExpression.h"
+#include "common/expression/ConstantExpression.h"
+#include "common/expression/ContainerExpression.h"
 #include "common/expression/Expression.h"
 #include "common/expression/PropertyExpression.h"
 #include "common/function/AggFunctionManager.h"
@@ -177,6 +180,50 @@ Expression *ExpressionUtils::rewriteParameter(const Expression *expr, QueryConte
   return graph::RewriteVisitor::transform(expr, matcher, rewriter);
 }
 
+Expression *ExpressionUtils::rewriteInnerInExpr(const Expression *expr) {
+  auto matcher = [](const Expression *e) -> bool {
+    if (e->kind() != Expression::Kind::kRelIn) {
+      return false;
+    }
+    auto rhs = static_cast<const RelationalExpression *>(e)->right();
+    if (rhs->kind() != Expression::Kind::kList && rhs->kind() != Expression::Kind::kSet) {
+      return false;
+    }
+    auto items = static_cast<const ContainerExpression *>(rhs)->getKeys();
+    for (const auto *item : items) {
+      if (!ExpressionUtils::isEvaluableExpr(item)) {
+        return false;
+      }
+    }
+    return true;
+  };
+  auto rewriter = [](const Expression *e) -> Expression * {
+    DCHECK_EQ(e->kind(), Expression::Kind::kRelIn);
+    const auto re = static_cast<const RelationalExpression *>(e);
+    auto lhs = re->left();
+    auto rhs = re->right();
+    DCHECK(rhs->kind() == Expression::Kind::kList || rhs->kind() == Expression::Kind::kSet);
+    auto ce = static_cast<const ContainerExpression *>(rhs);
+    auto pool = e->getObjPool();
+    auto *rewrittenExpr = LogicalExpression::makeOr(pool);
+    // Pointer to a single-level expression
+    Expression *singleExpr = nullptr;
+    auto items = ce->getKeys();
+    for (auto i = 0u; i < items.size(); ++i) {
+      auto *ee = RelationalExpression::makeEQ(pool, lhs->clone(), items[i]->clone());
+      rewrittenExpr->addOperand(ee);
+      if (i == 0) {
+        singleExpr = ee;
+      } else {
+        singleExpr = nullptr;
+      }
+    }
+    return singleExpr ? singleExpr : rewrittenExpr;
+  };
+
+  return graph::RewriteVisitor::transform(expr, matcher, rewriter);
+}
+
 Expression *ExpressionUtils::rewriteAgg2VarProp(const Expression *expr) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {
@@ -344,10 +391,10 @@ std::vector<Expression *> ExpressionUtils::getContainerExprOperands(const Expres
   std::vector<Expression *> containerOperands;
   switch (containerExpr->kind()) {
     case Expression::Kind::kList:
-      containerOperands = static_cast<ListExpression *>(containerExpr)->get();
+      containerOperands = static_cast<ListExpression *>(containerExpr)->getKeys();
       break;
     case Expression::Kind::kSet: {
-      containerOperands = static_cast<SetExpression *>(containerExpr)->get();
+      containerOperands = static_cast<SetExpression *>(containerExpr)->getKeys();
       break;
     }
     case Expression::Kind::kMap: {

diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
@@ -83,6 +83,9 @@ class ExpressionUtils {
   // Rewrites ParameterExpression to ConstantExpression
   static Expression* rewriteParameter(const Expression* expr, QueryContext* qctx);
 
+  // Rewrite RelInExpr with only one operand in expression tree
+  static Expression* rewriteInnerInExpr(const Expression* expr);
+
   // Rewrites relational expression, gather all evaluable expressions in the left operands and move
   // them to the right
   static Expression* rewriteRelExpr(const Expression* expr);

diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
@@ -303,7 +303,8 @@ Status MatchValidator::buildEdgeInfo(const MatchPath *path,
 // Rewrite expression to fit semantic, check type and check used aliases.
 Status MatchValidator::validateFilter(const Expression *filter,
                                       WhereClauseContext &whereClauseCtx) {
-  auto transformRes = ExpressionUtils::filterTransform(filter);
+  auto *newFilter = graph::ExpressionUtils::rewriteInnerInExpr(filter);
+  auto transformRes = ExpressionUtils::filterTransform(newFilter);
   NG_RETURN_IF_ERROR(transformRes);
   // rewrite Attribute to LabelTagProperty
   whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp(

diff --git a/tests/tck/features/match/IndexSelecting.feature b/tests/tck/features/match/IndexSelecting.feature
@@ -57,6 +57,35 @@ Feature: Index selecting for match statement
       | 2  | AppendVertices | 5            |                                                     |
       | 5  | IndexScan      | 0            | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
       | 0  | Start          |              |                                                     |
+    When profiling query:
+      """
+      MATCH (v:player) WHERE v.player.name in ["Yao Ming"] RETURN v.player.name AS name
+      """
+    Then the result should be, in any order, with relax comparison:
+      | name       |
+      | "Yao Ming" |
+    And the execution plan should be:
+      | id | name           | dependencies | operator info                                       |
+      | 6  | Project        | 2            |                                                     |
+      | 2  | Filter         | 5            |                                                     |
+      | 2  | AppendVertices | 5            |                                                     |
+      | 5  | IndexScan      | 0            | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
+      | 0  | Start          |              |                                                     |
+    When profiling query:
+      """
+      MATCH (v:player) WHERE v.player.name in ["Yao Ming", "Tim Duncan"] RETURN v.player.name AS name
+      """
+    Then the result should be, in any order, with relax comparison:
+      | name         |
+      | "Tim Duncan" |
+      | "Yao Ming"   |
+    And the execution plan should be:
+      | id | name           | dependencies | operator info                                       |
+      | 6  | Project        | 2            |                                                     |
+      | 2  | Filter         | 5            |                                                     |
+      | 2  | AppendVertices | 5            |                                                     |
+      | 5  | IndexScan      | 0            | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
+      | 0  | Start          |              |                                                     |
     When profiling query:
       """
       MATCH (v:player) WHERE v.player.name == "Tim Duncan" and v.player.name < "Zom" RETURN v.player.name AS name

diff --git a/tests/tck/features/match/SeekById.feature b/tests/tck/features/match/SeekById.feature
@@ -226,6 +226,7 @@ Feature: Match seek by id
       | "Rajon Rondo"      | "Lakers"    |
       | "LeBron James"     | "Lakers"    |
 
+  @czp
   Scenario: can't refer
     When executing query:
       """