fix geo predicate rule and geography isvalid

src/common/datatypes/Geography.cpp

@@ -50,6 +50,9 @@ bool LineString::isValid() const {
   if (coordList.size() < 2) {
     return false;
   }
+  for (const auto& coord : coordList) {
+    if (!coord.isValid()) return false;
+  }
   auto s2Region = geo::GeoUtils::s2RegionFromGeography(*this);
   CHECK_NOTNULL(s2Region);
   return static_cast<S2Polyline*>(s2Region.get())->IsValid();
@@ -71,6 +74,9 @@ bool Polygon::isValid() const {
     if (coordList.front() != coordList.back()) {
       return false;
     }
+    for (const auto& coord : coordList) {
+      if (!coord.isValid()) return false;
+    }
   }
   auto s2Region = geo::GeoUtils::s2RegionFromGeography(*this);
   CHECK_NOTNULL(s2Region);

src/common/function/FunctionManager.cpp

@@ -368,11 +368,16 @@ std::unordered_map<std::string, std::vector<TypeSignature>> FunctionManager::typ
      {
          TypeSignature({Value::Type::GEOGRAPHY, Value::Type::GEOGRAPHY, Value::Type::FLOAT},
                        Value::Type::BOOL),
+         TypeSignature({Value::Type::GEOGRAPHY, Value::Type::GEOGRAPHY, Value::Type::INT},
+                       Value::Type::BOOL),
          TypeSignature({Value::Type::GEOGRAPHY,
                         Value::Type::GEOGRAPHY,
                         Value::Type::FLOAT,
                         Value::Type::BOOL},
                        Value::Type::BOOL),
+         TypeSignature(
+             {Value::Type::GEOGRAPHY, Value::Type::GEOGRAPHY, Value::Type::INT, Value::Type::BOOL},
+             Value::Type::BOOL),
      }},
     // geo measures
     {"st_distance",
@@ -2497,7 +2502,7 @@ FunctionManager::FunctionManager() {
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
       if (!args[0].get().isGeography() || !args[1].get().isGeography() ||
-          !args[2].get().isFloat()) {
+          !args[2].get().isNumeric()) {
         return Value::kNullBadType;
       }
       bool exclusive = false;
@@ -2507,10 +2512,11 @@ FunctionManager::FunctionManager() {
         }
         exclusive = args[3].get().getBool();
       }
-      return geo::GeoFunction::dWithin(args[0].get().getGeography(),
-                                       args[1].get().getGeography(),
-                                       args[2].get().getFloat(),
-                                       exclusive);
+      return geo::GeoFunction::dWithin(
+          args[0].get().getGeography(),
+          args[1].get().getGeography(),
+          args[2].get().isFloat() ? args[2].get().getFloat() : args[2].get().getInt(),
+          exclusive);
     };
   }
   // geo measures

src/common/geo/GeoFunction.cpp

@@ -436,6 +436,10 @@ std::vector<uint64_t> GeoFunction::s2CoveringCellIds(
   opts.set_max_cells(maxCells);
 
   if (bufferInMeters == 0.0) {
+    if (a.shape() == GeoShape::POINT) {
+      const S2Point& gPoint = static_cast<const S2PointRegion*>(aRegion.get())->point();
+      return {S2CellId(gPoint).id()};
+    }
     return coveringCellIds(*aRegion, opts);
   }
 

src/common/geo/test/GeoFunctionTest.cpp

@@ -1096,6 +1096,16 @@ TEST(isValid, lineString) {
     bool b = line.isValid();
     EXPECT_EQ(false, b);
   }
+  {
+    auto line = Geography::fromWKT("LINESTRING(1.0 1.0, 181.0 2.0)").value();
+    bool b = line.isValid();
+    EXPECT_EQ(false, b);
+  }
+  {
+    auto line = Geography::fromWKT("LINESTRING(1.0 1.0, 1.0 90.001)").value();
+    bool b = line.isValid();
+    EXPECT_EQ(false, b);
+  }
 }
 
 TEST(isValid, polygon) {
@@ -1153,6 +1163,17 @@ TEST(isValid, polygon) {
   //   bool b = polygon.isValid();
   //   EXPECT_EQ(false, b);  // Expect false, got true
   // }
+  {
+    auto polygon =
+        Geography::fromWKT("POLYGON((1.0 1.0, -180.0001 2.0, 0.0 2.0, 1.0 1.0))").value();
+    bool b = polygon.isValid();
+    EXPECT_EQ(false, b);
+  }
+  {
+    auto polygon = Geography::fromWKT("POLYGON((1.0 1.0, 2.0 2.0, 0.0 -90.001, 1.0 1.0))").value();
+    bool b = polygon.isValid();
+    EXPECT_EQ(false, b);
+  }
 }
 
 }  // namespace geo

src/common/utils/IndexKeyUtils.h

@@ -437,7 +437,7 @@ class IndexKeyUtils final {
           break;
         }
         case Value::Type::GEOGRAPHY: {
-          // LOG(FATAL) << "unable to get geography value from index key"
+          // NOTE: The data read from index key is S2CellId which type is uint64, not wkb
           len = sizeof(uint64_t);
           break;
         }

src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp

@@ -81,7 +81,7 @@ StatusOr<TransformResult> GeoPredicateIndexScanBaseRule::transform(
          first->kind() == Expression::Kind::kEdgeProperty);
   DCHECK(second->kind() == Expression::Kind::kConstant);
   const auto& secondVal = static_cast<const ConstantExpression*>(second)->value();
-  DCHECK(secondVal.type() == Value::Type::GEOGRAPHY);
+  DCHECK(secondVal.isGeography());
   const auto& geog = secondVal.getGeography();
 
   // TODO(jie): Get index params from meta to construct RegionCoverParams
@@ -100,8 +100,8 @@ StatusOr<TransformResult> GeoPredicateIndexScanBaseRule::transform(
     auto* third = geoPredicate->args()->args()[2];
     DCHECK_EQ(third->kind(), Expression::Kind::kConstant);
     const auto& thirdVal = static_cast<const ConstantExpression*>(third)->value();
-    DCHECK_EQ(thirdVal.type(), Value::Type::FLOAT);
-    double distanceInMeters = thirdVal.getFloat();
+    DCHECK(thirdVal.isNumeric());
+    double distanceInMeters = thirdVal.isFloat() ? thirdVal.getFloat() : thirdVal.getInt();
     scanRanges = geoIndex.dWithin(geog, distanceInMeters);
   }
   std::vector<IndexQueryContext> idxCtxs;

src/graph/validator/LookupValidator.cpp

@@ -224,6 +224,8 @@ Status LookupValidator::validateFilter() {
     auto ret = checkFilter(filter);
     NG_RETURN_IF_ERROR(ret);
     lookupCtx_->filter = std::move(ret).value();
+    // Make sure the type of the rewritted filter expr is right
+    NG_RETURN_IF_ERROR(deduceExprType(lookupCtx_->filter));
   }
   NG_RETURN_IF_ERROR(deduceProps(lookupCtx_->filter, exprProps_));
   return Status::OK();

tests/tck/features/geo/GeoBase.feature

@@ -2,6 +2,7 @@
 #
 # This source code is licensed under Apache 2.0 License,
 # attached with Common Clause Condition 1.0, found in the LICENSES directory.
+@jie
 Feature: Geo base
 
   Background:
@@ -420,6 +421,16 @@ Feature: Geo base
       LOOKUP ON any_shape WHERE ST_Intersects(any_shape.geo, any_shape.geo) YIELD ST_ASText(any_shape.geo);
       """
     Then a SemanticError should be raised at runtime: Expression ST_Intersects(any_shape.geo,any_shape.geo) not supported yet
+    When executing query:
+      """
+      LOOKUP ON any_shape WHERE ST_Distance(any_shape.geo, ST_Point(3, 8.4)) < true YIELD ST_ASText(any_shape.geo);
+      """
+    Then a SemanticError should be raised at runtime:
+    When executing query:
+      """
+      LOOKUP ON any_shape WHERE ST_DWithin(any_shape.geo, ST_Point(3, 8.4), true) YIELD ST_ASText(any_shape.geo);
+      """
+    Then a SemanticError should be raised at runtime:
     # Match with geo predicate
     When executing query:
       """
@@ -523,6 +534,14 @@ Feature: Geo base
       | VertexID | ST_ASText(any_shape.geo)     |
       | "101"    | "POINT(3 8)"                 |
       | "102"    | "LINESTRING(3 8, 4.7 73.23)" |
+    When executing query:
+      """
+      LOOKUP ON any_shape WHERE ST_DWithin(any_shape.geo, ST_Point(3, 8), 100) YIELD ST_ASText(any_shape.geo);
+      """
+    Then the result should be, in any order:
+      | VertexID | ST_ASText(any_shape.geo)     |
+      | "101"    | "POINT(3 8)"                 |
+      | "102"    | "LINESTRING(3 8, 4.7 73.23)" |
     # ST_Covers
     When executing query:
       """