fix some

vesoft-inc · Oct 1, 2021 · 69bda9a · 69bda9a
1 parent e1de238
commit 69bda9a
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 42 deletions.
diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
@@ -11,6 +11,7 @@
 #include "common/base/Base.h"
 #include "common/datatypes/DataSet.h"
 #include "common/datatypes/Edge.h"
+#include "common/datatypes/Geography.h"
 #include "common/datatypes/List.h"
 #include "common/datatypes/Map.h"
 #include "common/datatypes/Path.h"
@@ -2297,21 +2298,12 @@ FunctionManager::FunctionManager() {
         return Value::kNullBadType;
       }
       const std::string &wkt = args[0].get().getStr();
-      LOG(INFO) << "st_geogfromtext, wkt: " << wkt;
-      auto geomRet = WKTReader().read(wkt);
-      if (!geomRet.ok()) {
-        LOG(INFO) << "ST_GeogFromText: " << geomRet.status();
+      auto geogRet = Geography::fromWKT(wkt);
+      if (!geogRet.ok()) {
         return Value::kNullBadData;
       }
-      std::unique_ptr<Geometry> geom = std::move(geomRet).value();
-      DCHECK(!!geom);
-      // if (!isValidGeom(geom.get())) { // Check if the geometry is valid due to OGC and WGS84(SRID
-      // 4326)
-      //   return Value::kNullBadData;
-      // }
-      auto wkb = WKBWriter().write(geom.get());
-      LOG(INFO) << "st_geogfromtext, wkb:" << wkb << ", wkb.size():" << wkb.size();
-      return Geography(wkb);
+      auto geog = std::move(geogRet.value());
+      return geog;
     };
   }
   {
@@ -2324,20 +2316,12 @@ FunctionManager::FunctionManager() {
         return Value::kNullBadType;
       }
       const std::string &wkb = args[0].get().getStr();
-      auto geomRet = WKBReader().read(wkb);
-      if (!geomRet.ok()) {
-        LOG(INFO) << "ST_GeogFromWKB: " << geomRet.status();
+      auto geogRet = Geography::fromWKB(wkb);
+      if (!geogRet.ok()) {
         return Value::kNullBadData;
       }
-      std::unique_ptr<Geometry> geom = std::move(geomRet).value();
-      DCHECK(!!geom);
-      // if (!validateGeom(wkb)) {
-      //   return Value::kNullBadData;
-      // }
-      auto newWKB =
-          WKBWriter().write(geom.get());  // TODO(jie) maybe just use the argument wkb is also ok
-      LOG(INFO) << "st_geogfromwkb, wkb:" << newWKB << ", wkb.size():" << newWKB.size();
-      return Geography(newWKB);
+      auto geog = std::move(geogRet.value());
+      return geog;
     };
   }
   {
@@ -2414,7 +2398,11 @@ FunctionManager::FunctionManager() {
         return Value::kNullBadType;
       }
       const Geography &g = args[0].get().getGeography();
-      return g.asWKT();
+      auto wkt = g.asWKT();
+      if (!wkt) {
+        return Value::kNullBadData;
+      }
+      return *wkt;
     };
   }
   {
@@ -2427,7 +2415,11 @@ FunctionManager::FunctionManager() {
         return Value::kNullBadType;
       }
       const Geography &g = args[0].get().getGeography();
-      return g.asWKB();
+      auto wkb = g.asWKBHex();
+      if (!wkb) {
+        return Value::kNullBadData;
+      }
+      return *wkb;
     };
   }
 }  // NOLINT

diff --git a/src/common/geo/function/Covers.cpp b/src/common/geo/function/Covers.cpp
@@ -9,6 +9,8 @@
 #include <s2/s2cell.h>
 #include <s2/s2polygon.h>
 
+#include "common/geo/GeoShape.h"
+
 namespace nebula {
 
 // Covers returns whether geography b covers geography b.
@@ -33,10 +35,11 @@ bool covers(const Geography& a, const Geography& b) {
           return false;
         case GeoShape::POLYGON:
           return false;
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
-          return -1.0;
+          return false;
       }
     }
     case GeoShape::LINESTRING: {
@@ -49,10 +52,11 @@ bool covers(const Geography& a, const Geography& b) {
                                      S1Angle::Radians(1e-15));
         case GeoShape::POLYGON:
           return false;
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
-          return -1.0;
+          return false;
       }
     }
     case GeoShape::POLYGON: {
@@ -64,10 +68,11 @@ bool covers(const Geography& a, const Geography& b) {
           return aPolygon->Contains(*static_cast<S2Polyline*>(bRegion.get()));
         case GeoShape::POLYGON:
           return aPolygon->Contains(static_cast<S2Polygon*>(bRegion.get()));
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
-          return -1.0;
+          return false;
       }
     }
   }

diff --git a/src/common/geo/function/DWithin.cpp b/src/common/geo/function/DWithin.cpp
@@ -40,8 +40,9 @@ bool dWithin(const Geography& a, const Geography& b, double distance, bool inclu
           S2Polygon* bPolygon = static_cast<S2Polygon*>(bRegion.get());
           return s2PointAndS2PolygonAreWithinDistance(aPoint, bPolygon, distance, inclusive);
         }
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
       }
@@ -71,8 +72,9 @@ bool dWithin(const Geography& a, const Geography& b, double distance, bool inclu
           S2Polygon* bPolygon = static_cast<S2Polygon*>(bRegion.get());
           return s2PolylineAndS2PolygonAreWithinDistance(aLine, bPolygon, distance, inclusive);
         }
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
       }
@@ -99,8 +101,9 @@ bool dWithin(const Geography& a, const Geography& b, double distance, bool inclu
           return query.IsDistanceLess(&target,
                                       S2Earth::ToChordAngle(util::units::Meters(distance)));
         }
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
       }

diff --git a/src/common/geo/function/Distance.cpp b/src/common/geo/function/Distance.cpp
@@ -39,8 +39,9 @@ double distance(const Geography& a, const Geography& b) {
           S2Polygon* bPolygon = static_cast<S2Polygon*>(bRegion.get());
           return distanceOfS2PolygonWithS2Point(bPolygon, aPoint);
         }
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return -1.0;
       }
@@ -65,8 +66,9 @@ double distance(const Geography& a, const Geography& b) {
           S2Polygon* bPolygon = static_cast<S2Polygon*>(bRegion.get());
           return distanceOfS2PolygonWithS2Polyline(bPolygon, aLine);
         }
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return -1.0;
       }
@@ -88,8 +90,9 @@ double distance(const Geography& a, const Geography& b) {
           S2ClosestEdgeQuery::ShapeIndexTarget target(&bPolygon->index());
           return S2Earth::ToMeters(query.GetDistance(&target));
         }
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return -1.0;
       }

diff --git a/src/common/geo/function/Intersects.cpp b/src/common/geo/function/Intersects.cpp
@@ -35,8 +35,9 @@ bool intersects(const Geography& a, const Geography& b) {
         case GeoShape::POLYGON:
           return static_cast<S2Polygon*>(bRegion.get())
               ->MayIntersect(S2Cell(static_cast<S2PointRegion*>(aRegion.get())->point()));
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
       }
@@ -52,8 +53,9 @@ bool intersects(const Geography& a, const Geography& b) {
         case GeoShape::POLYGON:
           return static_cast<S2Polygon*>(bRegion.get())
               ->Intersects(*static_cast<S2Polyline*>(aRegion.get()));
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
       }
@@ -69,8 +71,9 @@ bool intersects(const Geography& a, const Geography& b) {
         case GeoShape::POLYGON:
           return static_cast<S2Polygon*>(aRegion.get())
               ->Intersects(static_cast<S2Polygon*>(bRegion.get()));
+        case GeoShape::UNKNOWN:
         default:
-          LOG(FATAL)
+          LOG(ERROR)
               << "Geography shapes other than Point/LineString/Polygon are not currently supported";
           return false;
       }