add isAcceptedGeometry

vesoft-inc · Oct 3, 2021 · 29081e0 · 29081e0
1 parent 69bda9a
commit 29081e0
Show file tree

Hide file tree

Showing 16 changed files with 188 additions and 129 deletions.
diff --git a/src/common/datatypes/Geography.cpp b/src/common/datatypes/Geography.cpp
@@ -23,8 +23,19 @@ StatusOr<Geography> Geography::fromWKT(const std::string& wkt) {
   auto geomRet = WKTReader().read(wkt);
   NG_RETURN_IF_ERROR(geomRet);
   auto geom = geomRet.value();
-  auto wkb = WKBWriter().write(geom);
-  return Geography(wkb);
+  if (!geom.isValid()) {
+    return Status::Error("It's not valid");
+  }
+  auto bytes = WKBWriter().write(geom);
+  return Geography(bytes);
+}
+
+StatusOr<Geography> Geography::fromGeometry(const Geometry& geom) {
+  if (!geom.isValid()) {
+    return Status::Error("It's not valid");
+  }
+  auto bytes = WKBWriter().write(geom);
+  return Geography(bytes);
 }
 
 GeoShape Geography::shape() const {

diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h
@@ -41,7 +41,9 @@ struct Geography {
 
   Geography() = default;
 
+  // Factory method
   static StatusOr<Geography> fromWKT(const std::string& wkt);
+  static StatusOr<Geography> fromGeometry(const Geometry& geom);
 
   GeoShape shape() const;
 

diff --git a/src/common/datatypes/test/ValueTest.cpp b/src/common/datatypes/test/ValueTest.cpp
@@ -18,6 +18,7 @@
 #include "common/datatypes/Set.h"
 #include "common/datatypes/Value.h"
 #include "common/datatypes/Vertex.h"
+#include "common/geo/io/Geometry.h"
 
 namespace nebula {
 
@@ -1156,12 +1157,19 @@ TEST(Value, Ctor) {
   Value vMap(Map({{"a", 9}, {"b", 10}}));
   EXPECT_TRUE(vMap.isMap());
   // TODO(jie) Add more geography value test
-  Value vGeoPoint(Geography::fromWKT("POINT(0 1)").value());
-  EXPECT_TRUE(vGeoPoint.isGeography());
-  Value vGeoLine(Geography::fromWKT("LINESTRING(0 1,2 7)").value());
-  EXPECT_TRUE(vGeoLine.isGeography());
-  Value vGeoPolygon(Geography::fromWKT("POLYGON((0 1,2 3,4 5,6 7,0 1),(2 4,5 6,3 8,2 4))").value());
-  EXPECT_TRUE(vGeoPolygon.isGeography());
+  Geometry geomPoint(Point(Coordinate(3, 7)));
+  Value vGeogPoint{Geography::fromGeometry(geomPoint).value()};
+  EXPECT_TRUE(vGeogPoint.isGeography());
+  Geometry geomLine(LineString(std::vector<Coordinate>{Coordinate(0, 1), Coordinate(2, 7)}));
+  Value vGeogLine{Geography::fromGeometry(geomLine).value()};
+  EXPECT_TRUE(vGeogLine.isGeography());
+  Geometry geomPolygon(Polygon(std::vector<std::vector<Coordinate>>{
+      std::vector<Coordinate>{
+          Coordinate(0, 1), Coordinate(2, 3), Coordinate(4, 5), Coordinate(6, 7), Coordinate(0, 1)},
+      std::vector<Coordinate>{
+          Coordinate(2, 4), Coordinate(5, 6), Coordinate(3, 8), Coordinate(2, 4)}}));
+  Value vGeogPolygon{Geography::fromGeometry(geomPolygon).value()};
+  EXPECT_TRUE(vGeogPolygon.isGeography());
   // Disabled
   // Lead to compile error
   // Value v(nullptr);

diff --git a/src/common/function/CMakeLists.txt b/src/common/function/CMakeLists.txt
@@ -6,6 +6,10 @@
 nebula_add_library(
     function_manager_obj OBJECT
     FunctionManager.cpp
+    ../geo/function/Covers.cpp
+    ../geo/function/Distance.cpp
+    ../geo/function/DWithin.cpp
+    ../geo/function/Intersects.cpp
 )
 
 nebula_add_library(

diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
@@ -2306,24 +2306,24 @@ FunctionManager::FunctionManager() {
       return geog;
     };
   }
-  {
-    auto &attr = functions_["st_geogfromwkb"];
-    attr.minArity_ = 1;
-    attr.maxArity_ = 1;
-    attr.isPure_ = true;
-    attr.body_ = [](const auto &args) -> Value {
-      if (!args[0].get().isStr()) {  // wkb is byte sequence
-        return Value::kNullBadType;
-      }
-      const std::string &wkb = args[0].get().getStr();
-      auto geogRet = Geography::fromWKB(wkb);
-      if (!geogRet.ok()) {
-        return Value::kNullBadData;
-      }
-      auto geog = std::move(geogRet.value());
-      return geog;
-    };
-  }
+  // {
+  //   auto &attr = functions_["st_geogfromwkb"];
+  //   attr.minArity_ = 1;
+  //   attr.maxArity_ = 1;
+  //   attr.isPure_ = true;
+  //   attr.body_ = [](const auto &args) -> Value {
+  //     if (!args[0].get().isStr()) {  // wkb is byte sequence
+  //       return Value::kNullBadType;
+  //     }
+  //     const std::string &wkb = args[0].get().getStr();
+  //     auto geogRet = Geography::fromWKB(wkb);
+  //     if (!geogRet.ok()) {
+  //       return Value::kNullBadData;
+  //     }
+  //     auto geog = std::move(geogRet.value());
+  //     return geog;
+  //   };
+  // }
   {
     auto &attr = functions_["st_intersects"];
     attr.minArity_ = 2;

diff --git a/src/common/geo/GeoUtils.h b/src/common/geo/GeoUtils.h
@@ -11,6 +11,7 @@
 
 #include "common/base/StatusOr.h"
 #include "common/datatypes/Geography.h"
+#include "common/geo/GeoShape.h"
 #include "common/geo/io/Geometry.h"
 
 namespace nebula {
@@ -30,7 +31,7 @@ class GeoUtils final {
       case GeoShape::LINESTRING: {
         const auto& lineString = geom.lineString();
         auto coordList = lineString.coordList;
-        if (UNLIKELY(coordList.size() < 2)) {
+        if (coordList.size() < 2) {
           LOG(ERROR) << "LineString must have at least 2 coordinates";
           return nullptr;
         }
@@ -51,16 +52,17 @@ class GeoUtils final {
       }
       case GeoShape::POLYGON: {
         const auto& polygon = geom.polygon();
-        uint32_t numRings = polygon.numRings();
+        uint32_t numCoordList = polygon.numCoordList();
         std::vector<std::unique_ptr<S2Loop>> s2Loops;
-        s2Loops.reserve(numRings);
-        for (size_t i = 0; i < numRings; ++i) {
+        s2Loops.reserve(numCoordList);
+        for (size_t i = 0; i < numCoordList; ++i) {
           auto coordList = polygon.coordListList[i];
-          if (UNLIKELY(coordList.size() < 4)) {
+          if (coordList.size() < 4) {
             LOG(ERROR) << "Polygon's LinearRing must have at least 4 coordinates";
             return nullptr;
           }
-          if (UNLIKELY(isLoopClosed(coordList))) {
+          if (!isLoopClosed(coordList)) {
+            LOG(ERROR) << "Polygon's LinearRing must be closed";
             return nullptr;
           }
           removeAdjacentDuplicateCoordinates(coordList);

diff --git a/src/common/geo/function/Covers.cpp b/src/common/geo/function/Covers.cpp
@@ -69,12 +69,19 @@ bool covers(const Geography& a, const Geography& b) {
         case GeoShape::POLYGON:
           return aPolygon->Contains(static_cast<S2Polygon*>(bRegion.get()));
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
+    case GeoShape::UNKNOWN:
+    default: {
+      LOG(ERROR)
+          << "Geography shapes other than Point/LineString/Polygon are not currently supported";
+      return false;
+    }
   }
 
   return false;

diff --git a/src/common/geo/function/DWithin.cpp b/src/common/geo/function/DWithin.cpp
@@ -41,10 +41,11 @@ bool dWithin(const Geography& a, const Geography& b, double distance, bool inclu
           return s2PointAndS2PolygonAreWithinDistance(aPoint, bPolygon, distance, inclusive);
         }
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
     case GeoShape::LINESTRING: {
@@ -73,10 +74,11 @@ bool dWithin(const Geography& a, const Geography& b, double distance, bool inclu
           return s2PolylineAndS2PolygonAreWithinDistance(aLine, bPolygon, distance, inclusive);
         }
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
     case GeoShape::POLYGON: {
@@ -102,12 +104,19 @@ bool dWithin(const Geography& a, const Geography& b, double distance, bool inclu
                                       S2Earth::ToChordAngle(util::units::Meters(distance)));
         }
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
+    case GeoShape::UNKNOWN:
+    default: {
+      LOG(ERROR)
+          << "Geography shapes other than Point/LineString/Polygon are not currently supported";
+      return false;
+    }
   }
 
   return false;

diff --git a/src/common/geo/function/Distance.cpp b/src/common/geo/function/Distance.cpp
@@ -40,10 +40,11 @@ double distance(const Geography& a, const Geography& b) {
           return distanceOfS2PolygonWithS2Point(bPolygon, aPoint);
         }
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
-          return -1.0;
+          return false;
+        }
       }
     }
     case GeoShape::LINESTRING: {
@@ -67,10 +68,11 @@ double distance(const Geography& a, const Geography& b) {
           return distanceOfS2PolygonWithS2Polyline(bPolygon, aLine);
         }
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
-          return -1.0;
+          return false;
+        }
       }
     }
     case GeoShape::POLYGON: {
@@ -91,12 +93,19 @@ double distance(const Geography& a, const Geography& b) {
           return S2Earth::ToMeters(query.GetDistance(&target));
         }
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
-          return -1.0;
+          return false;
+        }
       }
     }
+    case GeoShape::UNKNOWN:
+    default: {
+      LOG(ERROR)
+          << "Geography shapes other than Point/LineString/Polygon are not currently supported";
+      return false;
+    }
   }
 
   return false;

diff --git a/src/common/geo/function/Intersects.cpp b/src/common/geo/function/Intersects.cpp
@@ -36,10 +36,11 @@ bool intersects(const Geography& a, const Geography& b) {
           return static_cast<S2Polygon*>(bRegion.get())
               ->MayIntersect(S2Cell(static_cast<S2PointRegion*>(aRegion.get())->point()));
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
     case GeoShape::LINESTRING: {
@@ -54,10 +55,11 @@ bool intersects(const Geography& a, const Geography& b) {
           return static_cast<S2Polygon*>(bRegion.get())
               ->Intersects(*static_cast<S2Polyline*>(aRegion.get()));
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
     case GeoShape::POLYGON: {
@@ -72,12 +74,19 @@ bool intersects(const Geography& a, const Geography& b) {
           return static_cast<S2Polygon*>(aRegion.get())
               ->Intersects(static_cast<S2Polygon*>(bRegion.get()));
         case GeoShape::UNKNOWN:
-        default:
+        default: {
           LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
+        }
       }
     }
+    case GeoShape::UNKNOWN:
+    default: {
+      LOG(ERROR)
+          << "Geography shapes other than Point/LineString/Polygon are not currently supported";
+      return false;
+    }
   }
 
   return false;