update wkt reader/writer

vesoft-inc · Sep 27, 2021 · 38ddae7 · 38ddae7
1 parent dd95d1d
commit 38ddae7
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 15 deletions.
diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h
@@ -25,7 +25,8 @@ namespace nebula {
 // clang-format off
 /*
 static const std::unordered_map<ShapeType, S2Region> kShapeTypeToS2Region = {
-    {ShapeType::Point, S2PointRegion}, // S2PointRegion is a wrapper of S2Point, and it inherits from the S2Region class
+    // S2PointRegion is a wrapper of S2Point, and it inherits from the S2Region class while S2Point doesn't.
+    {ShapeType::Point, S2PointRegion},
     {ShapeType::LineString, S2Polyline},
     {ShapeType::Polygon, S2Polygon},
 };

diff --git a/src/common/function/FunctionManager.cpp b/src/common/function/FunctionManager.cpp
@@ -346,7 +346,7 @@ std::unordered_map<std::string, std::vector<TypeSignature>> FunctionManager::typ
      {
          TypeSignature({Value::Type::GEOGRAPHY}, Value::Type::STRING),
      }},
-    {"st_aswkb",
+    {"st_asbinary",
      {
          TypeSignature({Value::Type::GEOGRAPHY}, Value::Type::STRING),
      }},
@@ -2293,6 +2293,9 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 1;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isStr()) {
+        return Value::kNullBadType;
+      }
       const std::string &wkt = args[0].get().getStr();
       LOG(INFO) << "st_geogfromtext, wkt: " << wkt;
       auto geomRet = WKTReader().read(wkt);
@@ -2317,14 +2320,24 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 1;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
-      // const std::string &wkb = args[0].get().getStr();
-      // auto geom = WKBReader().read(wkb);
+      if (!args[0].get().isStr()) {  // wkb is byte sequence
+        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();
+        return Value::kNullBadData;
+      }
+      std::unique_ptr<Geometry> geom = std::move(geomRet).value();
+      DCHECK(!!geom);
       // if (!validateGeom(wkb)) {
       //   return Value::kNullBadData;
       // }
-      // return Geography(wkb);
-      UNUSED(args);
-      return Geography();
+      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);
     };
   }
   {
@@ -2333,6 +2346,9 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 2;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography() || !args[1].get().isGeography()) {
+        return Value::kNullBadType;
+      }
       return intersects(args[0].get().getGeography(), args[1].get().getGeography());
     };
   }
@@ -2342,6 +2358,9 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 2;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography() || !args[1].get().isGeography()) {
+        return Value::kNullBadType;
+      }
       return covers(args[0].get().getGeography(), args[1].get().getGeography());
     };
   }
@@ -2351,6 +2370,9 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 2;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography() || !args[1].get().isGeography()) {
+        return Value::kNullBadType;
+      }
       return coveredBy(args[0].get().getGeography(), args[1].get().getGeography());
     };
   }
@@ -2360,6 +2382,10 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 3;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography() || !args[1].get().isGeography() ||
+          !args[2].get().isFloat()) {
+        return Value::kNullBadType;
+      }
       return dWithin(args[0].get().getGeography(),
                      args[1].get().getGeography(),
                      args[2].get().getFloat(),
@@ -2372,6 +2398,9 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 2;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography() || !args[1].get().isGeography()) {
+        return Value::kNullBadType;
+      }
       return distance(args[0].get().getGeography(), args[1].get().getGeography());
     };
   }
@@ -2381,16 +2410,22 @@ FunctionManager::FunctionManager() {
     attr.maxArity_ = 1;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography()) {
+        return Value::kNullBadType;
+      }
       const Geography &g = args[0].get().getGeography();
       return g.asWKT();
     };
   }
   {
-    auto &attr = functions_["st_aswkb"];
-    attr.minArity_ = 2;
-    attr.maxArity_ = 2;
+    auto &attr = functions_["st_asbinary"];
+    attr.minArity_ = 1;
+    attr.maxArity_ = 1;
     attr.isPure_ = true;
     attr.body_ = [](const auto &args) -> Value {
+      if (!args[0].get().isGeography()) {
+        return Value::kNullBadType;
+      }
       const Geography &g = args[0].get().getGeography();
       return g.asWKB();
     };

diff --git a/src/common/geo/io/wkb/WKBReader.h b/src/common/geo/io/wkb/WKBReader.h
@@ -91,10 +91,10 @@ class WKBReader {
   StatusOr<Coordinate> readCoordinate(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const {
     auto xRet = readDouble(beg, end, byteOrder);
     NG_RETURN_IF_ERROR(xRet);
-    uint32_t x = xRet.value();
+    double x = xRet.value();
     auto yRet = readDouble(beg, end, byteOrder);
     NG_RETURN_IF_ERROR(yRet);
-    uint32_t y = yRet.value();
+    double y = yRet.value();
     return Coordinate(x, y);
   }
 
@@ -136,8 +136,9 @@ class WKBReader {
     if (end - beg < requiredSize) {
       return Status::Error("Unable to parse uint8_t");
     }
+    uint8_t v = *beg;
     beg += requiredSize;
-    return *beg;
+    return v;
   }
 
   StatusOr<uint32_t> readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const {
@@ -155,7 +156,7 @@ class WKBReader {
     if (end - beg < requiredSize) {
       return Status::Error("Unable to parse double");
     }
-    uint32_t v = ByteOrderData::getDouble(beg, byteOrder);
+    double v = ByteOrderData::getDouble(beg, byteOrder);
     beg += requiredSize;
     return v;
   }

diff --git a/src/common/geo/io/wkt/WKTWriter.h b/src/common/geo/io/wkt/WKTWriter.h
@@ -86,7 +86,11 @@ class WKTWriter {
     wkt.pop_back();
   }
 
-  void writeDouble(std::string& wkt, double v) const { wkt.append(std::to_string(v)); }
+  void writeDouble(std::string& wkt, double v) const {
+    wkt.append(folly::to<std::string>(v));
+    LOG(INFO) << "writeDouble(wkt, v): " << wkt << ", " << v
+              << ", folly::to: " << folly::to<std::string>(v);
+  }
 };
 
 }  // namespace nebula