Don't rewrite vid in list expr to or expr

vesoft-inc · Jan 31, 2023 · 2076ba1 · 2076ba1
1 parent db7606b
commit 2076ba1
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 13 deletions.
diff --git a/src/graph/planner/match/MatchClausePlanner.cpp b/src/graph/planner/match/MatchClausePlanner.cpp
@@ -10,6 +10,7 @@
 #include "graph/planner/match/SegmentsConnector.h"
 #include "graph/planner/match/ShortestPathPlanner.h"
 #include "graph/planner/plan/Query.h"
+#include "graph/util/ExpressionUtils.h"
 
 namespace nebula {
 namespace graph {
@@ -29,6 +30,12 @@ StatusOr<SubPlan> MatchClausePlanner::transform(CypherClauseContextBase* clauseC
     auto& nodeInfos = iter->nodeInfos;
     SubPlan pathPlan;
     if (iter->pathType == Path::PathType::kDefault) {
+      // This filter is just used for index scan module which embedded in planner, e.g.
+      // PropIndexSeek, LabelIndexSeek So we need to rewrite the in list expr to or expr.
+      if (matchClauseCtx->where && matchClauseCtx->where->filter) {
+        matchClauseCtx->where->filter =
+            ExpressionUtils::rewriteInnerInExpr(matchClauseCtx->where->filter);
+      }
       MatchPathPlanner matchPathPlanner(matchClauseCtx, *iter);
       auto result = matchPathPlanner.transform(matchClauseCtx->where.get(), nodeAliasesSeen);
       NG_RETURN_IF_ERROR(result);

diff --git a/src/graph/planner/match/VertexIdSeek.cpp b/src/graph/planner/match/VertexIdSeek.cpp
@@ -48,7 +48,7 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) {
       if (nodeVid.first == node.alias) {
         nodeCtx->ids = std::move(nodeVid.second.vids);
         // Eliminate Filter when it's complete predication about Vertex Id
-        if (ExpressionUtils::isVidPredication(nodeCtx->bindWhereClause->filter)) {
+        if (ExpressionUtils::isVidPredication(nodeCtx->bindWhereClause->filter, nodeCtx->qctx)) {
           nodeCtx->bindWhereClause->filter = nullptr;
         }
         return true;

diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
@@ -1527,7 +1527,7 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) {
   return true;
 }
 
-/*static*/ bool ExpressionUtils::isVidPredication(const Expression *expr) {
+/*static*/ bool ExpressionUtils::isVidPredication(const Expression *expr, QueryContext *qctx) {
   if (DCHECK_NOTNULL(expr)->kind() != Expression::Kind::kRelIn &&
       expr->kind() != Expression::Kind::kRelEQ) {
     return false;
@@ -1543,7 +1543,13 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) {
   }
   if (expr->kind() == Expression::Kind::kRelIn) {
     // id(V) IN [List]
-    if (relExpr->right()->kind() != Expression::Kind::kList) {
+    if (!ExpressionUtils::isEvaluableExpr(relExpr->right(), qctx)) {
+      return false;
+    }
+
+    auto rightListValue = const_cast<Expression *>(relExpr->right())
+                              ->eval(graph::QueryExpressionContext(qctx->ectx())());
+    if (!rightListValue.isList()) {
       return false;
     }
   } else if (expr->kind() == Expression::Kind::kRelEQ) {

diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
@@ -240,7 +240,7 @@ class ExpressionUtils {
 
   // Whether the whole expression is vertex id predication
   // e.g. id(v) == 1, id(v) IN [...]
-  static bool isVidPredication(const Expression* expr);
+  static bool isVidPredication(const Expression* expr, QueryContext* qctx);
 
   // Check if the expr looks like `$-.e[0].likeness`
   static bool isOneStepEdgeProp(const std::string& edgeAlias, const Expression* expr);

diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
@@ -334,7 +334,6 @@ Status MatchValidator::buildEdgeInfo(const MatchPath *path,
 Status MatchValidator::validateFilter(const Expression *filter,
                                       WhereClauseContext &whereClauseCtx) {
   auto *newFilter = graph::ExpressionUtils::rewriteParameter(filter, qctx_);
-  newFilter = graph::ExpressionUtils::rewriteInnerInExpr(filter);
   auto transformRes = ExpressionUtils::filterTransform(newFilter);
   NG_RETURN_IF_ERROR(transformRes);
   // rewrite Attribute to LabelTagProperty

diff --git a/src/graph/visitor/VidExtractVisitor.cpp b/src/graph/visitor/VidExtractVisitor.cpp
@@ -151,23 +151,24 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
                                {{label, {VidPattern::Vids::Kind::kOtherSource, {}}}}};
       return;
     }
-    if (expr->left()->kind() != Expression::Kind::kFunctionCall ||
-        !(expr->right()->kind() == Expression::Kind::kList ||
-          expr->right()->kind() == Expression::Kind::kAttribute) ||
-        !ExpressionUtils::isEvaluableExpr(expr->right(), qctx_)) {
+    if (expr->left()->kind() != Expression::Kind::kFunctionCall) {
       vidPattern_ = VidPattern{};
       return;
     }
-
     const auto *fCallExpr = static_cast<const FunctionCallExpression *>(expr->left());
     if (fCallExpr->name() != "id" && fCallExpr->args()->numArgs() != 1 &&
         fCallExpr->args()->args().front()->kind() != Expression::Kind::kLabel) {
       vidPattern_ = VidPattern{};
       return;
     }
+    if (!ExpressionUtils::isEvaluableExpr(expr->right(), qctx_)) {
+      vidPattern_ = VidPattern{};
+      return;
+    }
 
     auto rightListValue = expr->right()->eval(graph::QueryExpressionContext(qctx_->ectx())());
     if (!rightListValue.isList()) {
+      vidPattern_ = VidPattern{};
       return;
     }
     vidPattern_ = VidPattern{VidPattern::Special::kInUsed,

diff --git a/tests/tck/features/basic/data.feature b/tests/tck/features/basic/data.feature
@@ -51,3 +51,15 @@ Feature: data
     Then the result should be, in any order, with relax comparison:
       | a    | b    | c    |
       | true | true | true |
+    When profiling query:
+      """
+      WITH 1 AS a WHERE a IN [2, 3, 4] RETURN a
+      """
+    Then the result should be, in any order, with relax comparison:
+      | a |
+    And the execution plan should be:
+      | id | name    | dependencies | operator info                    |
+      | 4  | Project | 2            |                                  |
+      | 2  | Filter  | 1            | {"condition": "($a IN [2,3,4])"} |
+      | 1  | Project | 3            |                                  |
+      | 3  | Start   |              |                                  |
diff --git a/tests/tck/features/match/SeekById.feature b/tests/tck/features/match/SeekById.feature
@@ -477,6 +477,7 @@ Feature: Match seek by id
       | 11 | Project        | 8            |               |
       | 8  | Filter         | 4            |               |
       | 4  | AppendVertices | 10           |               |
-      | 10 | Traverse       | 1            |               |
-      | 1  | IndexScan      | 2            |               |
-      | 2  | Start          |              |               |
+      | 10 | Traverse       | 2            |               |
+      | 2  | Dedup          | 4            |               |
+      | 1  | PassThrough    | 3            |               |
+      | 3  | Start          |              |               |