fix crash of geo (#5475)

* fix crash of geo * change log(fatal) to log(error)
vesoft-inc · Apr 6, 2023 · fb5ea40 · fb5ea40
1 parent 10bcb5d
commit fb5ea40
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 14 deletions.
diff --git a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp
@@ -66,24 +66,39 @@ StatusOr<TransformResult> GeoPredicateIndexScanBaseRule::transform(
   }
 
   auto condition = filter->condition();
-  DCHECK(graph::ExpressionUtils::isGeoIndexAcceleratedPredicate(condition));
+  if (!graph::ExpressionUtils::isGeoIndexAcceleratedPredicate(condition)) {
+    return TransformResult::noTransform();
+  }
 
   auto* geoPredicate = static_cast<FunctionCallExpression*>(condition);
-  DCHECK_GE(geoPredicate->args()->numArgs(), 2);
+  if (geoPredicate->args()->numArgs() < 2) {
+    return TransformResult::noTransform();
+  }
   std::string geoPredicateName = geoPredicate->name();
   folly::toLowerAscii(geoPredicateName);
   auto* first = geoPredicate->args()->args()[0];
   auto* second = geoPredicate->args()->args()[1];
-  DCHECK(first->kind() == Expression::Kind::kTagProperty ||
-         first->kind() == Expression::Kind::kEdgeProperty);
-  DCHECK(second->kind() == Expression::Kind::kConstant);
-  const auto& secondVal = static_cast<const ConstantExpression*>(second)->value();
-  DCHECK(secondVal.isGeography());
+  if (first->kind() != Expression::Kind::kTagProperty &&
+      first->kind() != Expression::Kind::kEdgeProperty) {
+    return TransformResult::noTransform();
+  }
+
+  if (!graph::ExpressionUtils::isEvaluableExpr(second, qctx)) {
+    return TransformResult::noTransform();
+  }
+
+  auto secondVal = second->eval(graph::QueryExpressionContext(qctx->ectx())());
+  if (!secondVal.isGeography()) {
+    return TransformResult::noTransform();
+  }
+
   const auto& geog = secondVal.getGeography();
 
   auto indexItem = indexItems.back();
   const auto& fields = indexItem->get_fields();
-  DCHECK_EQ(fields.size(), 1);  // geo field
+  if (fields.size() != 1) {
+    return TransformResult::noTransform();
+  }
   auto& geoField = fields.back();
   auto& geoColumnTypeDef = geoField.get_type();
   bool isPointColumn = geoColumnTypeDef.geo_shape_ref().has_value() &&
@@ -108,7 +123,9 @@ StatusOr<TransformResult> GeoPredicateIndexScanBaseRule::transform(
   } else if (geoPredicateName == "st_coveredby") {
     scanRanges = geoIndex.covers(geog);
   } else if (geoPredicateName == "st_dwithin") {
-    DCHECK_GE(geoPredicate->args()->numArgs(), 3);
+    if (geoPredicate->args()->numArgs() < 3) {
+      return TransformResult::noTransform();
+    }
     auto* third = geoPredicate->args()->args()[2];
     if (!graph::ExpressionUtils::isEvaluableExpr(third, qctx)) {
       return TransformResult::noTransform();

diff --git a/src/graph/optimizer/rule/PushEFilterDownRule.cpp b/src/graph/optimizer/rule/PushEFilterDownRule.cpp
@@ -174,7 +174,7 @@ std::string PushEFilterDownRule::toString() const {
       ret = EdgePropertyExpression::make(pool, std::move(edgeNameResult).value(), exp->prop());
       break;
     default:
-      DLOG(FATAL) << "Unexpected expr: " << exp->kind();
+      DLOG(ERROR) << "Unexpected expr: " << exp->kind();
   }
   return ret;
 }

diff --git a/src/graph/optimizer/rule/PushFilterDownNodeRule.cpp b/src/graph/optimizer/rule/PushFilterDownNodeRule.cpp
@@ -55,7 +55,7 @@ StatusOr<OptRule::TransformResult> PushFilterDownNodeRule::transform(
     auto *append = static_cast<const AppendVertices *>(node);
     vFilter = append->vFilter()->clone();
   } else {
-    DLOG(FATAL) << "Unsupported node kind: " << node->kind();
+    DLOG(ERROR) << "Unsupported node kind: " << node->kind();
     return TransformResult::noTransform();
   }
   auto visitor = graph::ExtractFilterExprVisitor::makePushGetVertices(pool);
@@ -83,7 +83,7 @@ StatusOr<OptRule::TransformResult> PushFilterDownNodeRule::transform(
     append->setVertexFilter(remainedExpr);
     append->setFilter(vFilter);
   } else {
-    DLOG(FATAL) << "Unsupported node kind: " << newExplore->kind();
+    DLOG(ERROR) << "Unsupported node kind: " << newExplore->kind();
     return TransformResult::noTransform();
   }
 
@@ -111,7 +111,7 @@ bool PushFilterDownNodeRule::match(OptContext *octx, const MatchedResult &matche
       return false;
     }
   } else {
-    DLOG(FATAL) << "Unexpected node kind: " << node->kind();
+    DLOG(ERROR) << "Unexpected node kind: " << node->kind();
     return false;
   }
   return true;

diff --git a/src/graph/optimizer/rule/UnionAllIndexScanBaseRule.cpp b/src/graph/optimizer/rule/UnionAllIndexScanBaseRule.cpp
@@ -167,7 +167,7 @@ StatusOr<TransformResult> UnionAllIndexScanBaseRule::transform(OptContext* ctx,
       break;
     }
     default:
-      DLOG(FATAL) << "Invalid expression kind: " << static_cast<uint8_t>(conditionType);
+      DLOG(ERROR) << "Invalid expression kind: " << static_cast<uint8_t>(conditionType);
       return TransformResult::noTransform();
   }
 

diff --git a/tests/tck/features/geo/GeoBase.feature b/tests/tck/features/geo/GeoBase.feature
@@ -20,6 +20,7 @@ Feature: Geo base
       CREATE EDGE any_shape_edge(geo geography);
       """
     And wait 3 seconds
+    Given parameters: {"p4": {"s1":"test","s2":"2020-01-01 10:00:00","s3":[1,2,3,4,5],"longitude":[1.0,2.0,3.0],"latitude":[10.1,11.1,12.1]}}
 
   Scenario: test geo schema
     # Desc geo schema
@@ -543,6 +544,12 @@ Feature: Geo base
       | "LINESTRING(3 8, 4.7 73.23)"    |
       | "POLYGON((0 1, 1 2, 2 3, 0 1))" |
       | "POINT(72.3 84.6)"              |
+    When executing query:
+      """
+      LOOKUP ON any_shape WHERE ST_Distance(any_shape.geo, ST_Point($p4.longitude[0], $p4.latitude[1])) < $p4.latitude[2] YIELD id(vertex)
+      """
+    Then the result should be, in any order:
+      | id(VERTEX) |
     When executing query:
       """
       LOOKUP ON any_shape WHERE 8909524.383934560 > ST_Distance(any_shape.geo, ST_Point(3, 8)) YIELD ST_ASText(any_shape.geo);