Fix lookup in crash when the property has no corresponding filter (#3986

)

* Fix lookup in crash

* Fix typos

* Fix tck

Co-authored-by: Sophie <[email protected]>

src/graph/optimizer/rule/OptimizeTagIndexScanByFilterRule.cpp

@@ -130,7 +130,7 @@ StatusOr<TransformResult> OptimizeTagIndexScanByFilterRule::transform(
           return TransformResult::noTransform();
         } else {
           // If the inner IN expr has only 1 element, rewrite it to an relEQ expression and there is
-          // no need to check wether it has a index
+          // no need to check whether it has a index
           auto relEqExpr = graph::ExpressionUtils::rewriteInExpr(inExpr);
           static_cast<LogicalExpression*>(transformedExpr)->setOperand(operandIndex, relEqExpr);
           continue;

src/graph/optimizer/rule/UnionAllIndexScanBaseRule.cpp

@@ -135,6 +135,12 @@ StatusOr<TransformResult> UnionAllIndexScanBaseRule::transform(OptContext* ctx,
 
       // Reconstruct AND expr using distributive law
       transformedExpr = graph::ExpressionUtils::rewriteLogicalAndToLogicalOr(transformedExpr);
+
+      // If there is no OR expr in the transformedExpr, only one index scan should be done.
+      // OptimizeTagIndexScanByFilterRule should be applied
+      if (!graph::ExpressionUtils::findAny(transformedExpr, {Expression::Kind::kLogicalOr})) {
+        return TransformResult::noTransform();
+      }
       break;
     }
 

src/graph/util/ExpressionUtils.cpp

@@ -217,6 +217,12 @@ Expression *ExpressionUtils::rewriteInExpr(const Expression *expr) {
 
 Expression *ExpressionUtils::rewriteLogicalAndToLogicalOr(const Expression *expr) {
   DCHECK(expr->kind() == Expression::Kind::kLogicalAnd);
+
+  // If the given expression does not contain any inner logical OR expression, no need to rewrite
+  if (!findAny(expr, {Expression::Kind::kLogicalOr})) {
+    return const_cast<Expression *>(expr);
+  }
+
   auto pool = expr->getObjPool();
   auto logicalAndExpr = static_cast<LogicalExpression *>(expr->clone());
   auto logicalAndExprSize = (logicalAndExpr->operands()).size();

src/graph/util/ExpressionUtils.h

@@ -87,7 +87,8 @@ class ExpressionUtils {
   // Rewrites IN expression into OR expression or relEQ expression
   static Expression* rewriteInExpr(const Expression* expr);
 
-  // Rewrites Logical AND expr to Logical OR expr using distributive law
+  // Rewrite Logical AND expr that contains Logical OR expr to Logical OR expr using distributive
+  // law
   // Examples:
   // A and (B or C)  => (A and B) or (A and C)
   // (A or B) and (C or D)  =>  (A and C) or (A and D) or (B and C) or (B or D)

tests/tck/features/bugfix/LookupIn.feature

@@ -0,0 +1,40 @@
+# Copyright (c) 2022 vesoft inc. All rights reserved.
+#
+# This source code is licensed under Apache 2.0 License.
+Feature: Lookup_In
+
+  # Lookup ... WHERE xxx IN xxx causes service crash
+  # Fix https://github.com/vesoft-inc/nebula/issues/3983
+  Scenario: lookup in where prop has no index
+    Given an empty graph
+    And load "nba" csv data to a new space
+    When executing query:
+      """
+      DROP TAG INDEX IF EXISTS player_age_index
+      """
+    Then the execution should be successful
+    When submit a job:
+      """
+      REBUILD TAG INDEX
+      """
+    Then wait the job to finish
+    When executing query:
+      """
+      Show TAG INDEXES;
+      """
+    Then the result should be, in any order:
+      | Index Name          | By Tag     | Columns  |
+      | "bachelor_index"    | "bachelor" | []       |
+      | "player_name_index" | "player"   | ["name"] |
+      | "team_name_index"   | "team"     | ["name"] |
+    # player.age has no index
+    When executing query:
+      """
+      LOOKUP ON player WHERE player.age IN [40, 20] AND player.name > "" YIELD id(vertex) as id, player.age
+      """
+    Then the result should be, in any order:
+      | id              | player.age |
+      | "Dirk Nowitzki" | 40         |
+      | "Luka Doncic"   | 20         |
+      | "Kobe Bryant"   | 40         |
+    Then drop the used space