From 78ce96ea34fd19a20d0e7de05eb24a4fbb8fb29f Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Mon, 27 Sep 2021 19:16:30 +0800 Subject: [PATCH 1/9] add geograpy value --- cmake/nebula/GeneralCompilerConfig.cmake | 1 + src/codec/RowReaderV2.cpp | 4 + src/codec/RowWriterV2.cpp | 10 + src/codec/test/CMakeLists.txt | 1 + src/codec/test/SchemaWriter.cpp | 3 + src/common/CMakeLists.txt | 1 + src/common/base/Base.h | 2 +- src/common/conf/Configuration.h | 25 +-- src/common/datatypes/CMakeLists.txt | 1 + src/common/datatypes/CommonCpp2Ops.h | 2 + src/common/datatypes/Geography.cpp | 71 +++++++ src/common/datatypes/Geography.h | 101 ++++++++++ src/common/datatypes/GeographyOps-inl.h | 132 +++++++++++++ src/common/datatypes/Value.cpp | 114 +++++++++++ src/common/datatypes/Value.h | 18 ++ src/common/datatypes/ValueOps-inl.h | 47 +++++ src/common/datatypes/test/CMakeLists.txt | 8 + src/common/datatypes/test/ValueTest.cpp | 1 + src/common/expression/test/CMakeLists.txt | 24 +++ src/common/function/test/CMakeLists.txt | 2 + src/common/geo/CMakeLists.txt | 7 + src/common/geo/GeoShape.h | 17 ++ src/common/geo/GeoUtils.h | 124 ++++++++++++ src/common/geo/io/CMakeLists.txt | 34 ++++ src/common/geo/io/Geometry.h | 70 +++++++ src/common/geo/io/wkb/ByteOrder.h | 57 ++++++ src/common/geo/io/wkb/WKBReader.cpp | 11 ++ src/common/geo/io/wkb/WKBReader.h | 166 ++++++++++++++++ src/common/geo/io/wkb/WKBWriter.cpp | 11 ++ src/common/geo/io/wkb/WKBWriter.h | 93 +++++++++ src/common/geo/io/wkt/CMakeLists.txt | 6 + src/common/geo/io/wkt/WKTReader.cpp | 11 ++ src/common/geo/io/wkt/WKTReader.h | 80 ++++++++ src/common/geo/io/wkt/WKTScanner.h | 67 +++++++ src/common/geo/io/wkt/WKTWriter.cpp | 11 ++ src/common/geo/io/wkt/WKTWriter.h | 96 +++++++++ src/common/geo/io/wkt/test/CMakeLists.txt | 54 +++++ src/common/geo/io/wkt/test/WKTParserTest.cpp | 137 +++++++++++++ src/common/geo/io/wkt/wkt_parser.yy | 198 +++++++++++++++++++ src/common/geo/io/wkt/wkt_scanner.lex | 63 ++++++ src/common/graph/tests/CMakeLists.txt | 1 + src/common/meta/NebulaSchemaProvider.cpp | 2 + src/common/thread/GenericWorker.h | 2 +- src/common/time/TimezoneInfo.h | 6 +- src/common/time/test/CMakeLists.txt | 1 + src/common/utils/IndexKeyUtils.h | 24 +++ src/common/utils/test/CMakeLists.txt | 3 + src/daemons/CMakeLists.txt | 2 + src/graph/context/test/CMakeLists.txt | 1 + src/graph/executor/test/CMakeLists.txt | 2 + src/graph/optimizer/OptimizerUtils.cpp | 5 + src/graph/optimizer/test/CMakeLists.txt | 1 + src/graph/planner/test/CMakeLists.txt | 1 + src/graph/service/Authenticator.h | 2 +- src/graph/service/GraphService.h | 4 +- src/graph/util/SchemaUtil.cpp | 2 + src/graph/util/test/CMakeLists.txt | 1 + src/graph/validator/Validator.h | 2 +- src/graph/validator/test/CMakeLists.txt | 2 + src/graph/visitor/test/CMakeLists.txt | 1 + src/interface/common.thrift | 7 +- src/interface/meta.thrift | 14 ++ src/kvstore/raftex/test/CMakeLists.txt | 1 + src/kvstore/test/CMakeLists.txt | 3 +- src/meta/CMakeLists.txt | 1 + src/meta/processors/schema/SchemaUtil.cpp | 7 + src/meta/test/CMakeLists.txt | 1 + src/parser/MaintainSentences.h | 18 +- src/parser/parser.yy | 34 +++- src/parser/scanner.lex | 4 + src/parser/test/CMakeLists.txt | 1 + src/storage/test/CMakeLists.txt | 1 + src/tools/db-dump/CMakeLists.txt | 1 + src/tools/db-upgrade/CMakeLists.txt | 1 + src/tools/meta-dump/CMakeLists.txt | 1 + src/tools/simple-kv-verify/CMakeLists.txt | 1 + src/tools/storage-perf/CMakeLists.txt | 1 + src/webservice/WebService.h | 4 +- 78 files changed, 2014 insertions(+), 33 deletions(-) create mode 100644 src/common/datatypes/Geography.cpp create mode 100644 src/common/datatypes/Geography.h create mode 100644 src/common/datatypes/GeographyOps-inl.h create mode 100644 src/common/geo/CMakeLists.txt create mode 100644 src/common/geo/GeoShape.h create mode 100644 src/common/geo/GeoUtils.h create mode 100644 src/common/geo/io/CMakeLists.txt create mode 100644 src/common/geo/io/Geometry.h create mode 100644 src/common/geo/io/wkb/ByteOrder.h create mode 100644 src/common/geo/io/wkb/WKBReader.cpp create mode 100644 src/common/geo/io/wkb/WKBReader.h create mode 100644 src/common/geo/io/wkb/WKBWriter.cpp create mode 100644 src/common/geo/io/wkb/WKBWriter.h create mode 100644 src/common/geo/io/wkt/CMakeLists.txt create mode 100644 src/common/geo/io/wkt/WKTReader.cpp create mode 100644 src/common/geo/io/wkt/WKTReader.h create mode 100644 src/common/geo/io/wkt/WKTScanner.h create mode 100644 src/common/geo/io/wkt/WKTWriter.cpp create mode 100644 src/common/geo/io/wkt/WKTWriter.h create mode 100644 src/common/geo/io/wkt/test/CMakeLists.txt create mode 100644 src/common/geo/io/wkt/test/WKTParserTest.cpp create mode 100644 src/common/geo/io/wkt/wkt_parser.yy create mode 100644 src/common/geo/io/wkt/wkt_scanner.lex diff --git a/cmake/nebula/GeneralCompilerConfig.cmake b/cmake/nebula/GeneralCompilerConfig.cmake index 2886423cce9..a914a49a63b 100644 --- a/cmake/nebula/GeneralCompilerConfig.cmake +++ b/cmake/nebula/GeneralCompilerConfig.cmake @@ -23,6 +23,7 @@ add_compile_options(-Wignored-qualifiers) # For s2 add_definitions(-DS2_USE_GLOG) +add_definitions(-DS2_USE_GFLAGS) # For breakpad add_definitions(-D__STDC_FORMAT_MACROS) diff --git a/src/codec/RowReaderV2.cpp b/src/codec/RowReaderV2.cpp index ad7c6d2866a..8d1a3324fea 100644 --- a/src/codec/RowReaderV2.cpp +++ b/src/codec/RowReaderV2.cpp @@ -175,6 +175,10 @@ Value RowReaderV2::getValueByIndex(const int64_t index) const noexcept { dt.microsec = microsec; return dt; } + case meta::cpp2::PropertyType::GEOGRAPHY: { + // TODO(jie) + return Geography(""); + } case meta::cpp2::PropertyType::UNKNOWN: break; } diff --git a/src/codec/RowWriterV2.cpp b/src/codec/RowWriterV2.cpp index 9b3723284fe..1953e63d29a 100644 --- a/src/codec/RowWriterV2.cpp +++ b/src/codec/RowWriterV2.cpp @@ -126,6 +126,9 @@ RowWriterV2::RowWriterV2(RowReader& reader) : RowWriterV2(reader.getSchema()) { case Value::Type::DATETIME: set(i, v.moveDateTime()); break; + case Value::Type::GEOGRAPHY: + // TODO(jie) + break; default: LOG(FATAL) << "Invalid data: " << v << ", type: " << v.typeName(); } @@ -203,6 +206,9 @@ WriteResult RowWriterV2::setValue(ssize_t index, const Value& val) noexcept { return write(index, val.getTime()); case Value::Type::DATETIME: return write(index, val.getDateTime()); + case Value::Type::GEOGRAPHY: + // TODO(jie) + return WriteResult::TYPE_MISMATCH; default: return WriteResult::TYPE_MISMATCH; } @@ -637,6 +643,7 @@ WriteResult RowWriterV2::write(ssize_t index, folly::StringPiece v) noexcept { auto field = schema_->field(index); auto offset = headerLen_ + numNullBytes_ + field->offset(); switch (field->type()) { + case meta::cpp2::PropertyType::GEOGRAPHY: // write wkb case meta::cpp2::PropertyType::STRING: { if (isSet_[index]) { // The string value has already been set, we need to turn it @@ -794,6 +801,9 @@ WriteResult RowWriterV2::checkUnsetFields() noexcept { case Value::Type::DATETIME: r = write(i, defVal.getDateTime()); break; + case Value::Type::GEOGRAPHY: + // TODO(jie) + break; default: LOG(FATAL) << "Unsupported default value type: " << defVal.typeName() << ", default value: " << defVal diff --git a/src/codec/test/CMakeLists.txt b/src/codec/test/CMakeLists.txt index 05581db1c94..1d761a12d88 100644 --- a/src/codec/test/CMakeLists.txt +++ b/src/codec/test/CMakeLists.txt @@ -28,6 +28,7 @@ set(CODEC_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/codec/test/SchemaWriter.cpp b/src/codec/test/SchemaWriter.cpp index 904d3db8bb3..d7c11ebe1eb 100644 --- a/src/codec/test/SchemaWriter.cpp +++ b/src/codec/test/SchemaWriter.cpp @@ -75,6 +75,9 @@ SchemaWriter& SchemaWriter::appendCol(folly::StringPiece name, case PropertyType::DATETIME: size = sizeof(int16_t) + 5 * sizeof(int8_t) + sizeof(int32_t); break; + case PropertyType::GEOGRAPHY: + size = 2 * sizeof(int32_t); // as same as STRING + break; default: LOG(FATAL) << "Unknown column type"; } diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index fe404277c93..469771ab431 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -26,3 +26,4 @@ nebula_add_subdirectory(graph) nebula_add_subdirectory(plugin) nebula_add_subdirectory(utils) nebula_add_subdirectory(ssl) +nebula_add_subdirectory(geo) diff --git a/src/common/base/Base.h b/src/common/base/Base.h index bae877f2883..aabff0e7b09 100644 --- a/src/common/base/Base.h +++ b/src/common/base/Base.h @@ -67,7 +67,7 @@ #include "common/base/Logging.h" -#define MUST_USE_RESULT __attribute__((warn_unused_result)) +#define NG_MUST_USE_RESULT __attribute__((warn_unused_result)) #define DONT_OPTIMIZE __attribute__((optimize("O0"))) #define ALWAYS_INLINE __attribute__((always_inline)) diff --git a/src/common/conf/Configuration.h b/src/common/conf/Configuration.h index 4f2a5c6e6f7..18aa9de8f0d 100644 --- a/src/common/conf/Configuration.h +++ b/src/common/conf/Configuration.h @@ -27,11 +27,11 @@ class Configuration final { /** * Parse from a file */ - Status MUST_USE_RESULT parseFromFile(const std::string &filename); + Status NG_MUST_USE_RESULT parseFromFile(const std::string &filename); /** * Parse from a string buffer */ - Status MUST_USE_RESULT parseFromString(const std::string &content); + Status NG_MUST_USE_RESULT parseFromString(const std::string &content); std::string dumpToString() const; @@ -42,19 +42,20 @@ class Configuration final { * @key item key * @val to hold the item value. */ - Status MUST_USE_RESULT fetchAsInt(const char *key, int64_t &val) const; - Status MUST_USE_RESULT fetchAsDouble(const char *key, double &val) const; - Status MUST_USE_RESULT fetchAsBool(const char *key, bool &val) const; - Status MUST_USE_RESULT fetchAsString(const char *key, std::string &val) const; + Status NG_MUST_USE_RESULT fetchAsInt(const char *key, int64_t &val) const; + Status NG_MUST_USE_RESULT fetchAsDouble(const char *key, double &val) const; + Status NG_MUST_USE_RESULT fetchAsBool(const char *key, bool &val) const; + Status NG_MUST_USE_RESULT fetchAsString(const char *key, std::string &val) const; - Status MUST_USE_RESULT fetchAsIntArray(const char *key, std::vector &val) const; - Status MUST_USE_RESULT fetchAsDoubleArray(const char *key, std::vector &val) const; - Status MUST_USE_RESULT fetchAsBoolArray(const char *key, std::vector &val) const; - Status MUST_USE_RESULT fetchAsStringArray(const char *key, std::vector &val) const; + Status NG_MUST_USE_RESULT fetchAsIntArray(const char *key, std::vector &val) const; + Status NG_MUST_USE_RESULT fetchAsDoubleArray(const char *key, std::vector &val) const; + Status NG_MUST_USE_RESULT fetchAsBoolArray(const char *key, std::vector &val) const; + Status NG_MUST_USE_RESULT fetchAsStringArray(const char *key, + std::vector &val) const; - Status MUST_USE_RESULT fetchAsSubConf(const char *key, Configuration &val) const; + Status NG_MUST_USE_RESULT fetchAsSubConf(const char *key, Configuration &val) const; - Status MUST_USE_RESULT upsertStringField(const char *key, const std::string &val); + Status NG_MUST_USE_RESULT upsertStringField(const char *key, const std::string &val); // Iterate through every key in the configuration Status forEachKey(std::function processor) const; diff --git a/src/common/datatypes/CMakeLists.txt b/src/common/datatypes/CMakeLists.txt index 53d0cae9a41..a6cbaf9fad5 100644 --- a/src/common/datatypes/CMakeLists.txt +++ b/src/common/datatypes/CMakeLists.txt @@ -14,6 +14,7 @@ nebula_add_library( Map.cpp List.cpp Set.cpp + Geography.cpp ) nebula_add_subdirectory(test) diff --git a/src/common/datatypes/CommonCpp2Ops.h b/src/common/datatypes/CommonCpp2Ops.h index 5d969c94d2d..8be8fdf89b4 100644 --- a/src/common/datatypes/CommonCpp2Ops.h +++ b/src/common/datatypes/CommonCpp2Ops.h @@ -24,6 +24,7 @@ struct Map; struct Set; struct List; struct DataSet; +struct Geography; } // namespace nebula namespace apache::thrift { @@ -43,6 +44,7 @@ SPECIALIZE_CPP2OPS(nebula::Map); SPECIALIZE_CPP2OPS(nebula::Set); SPECIALIZE_CPP2OPS(nebula::List); SPECIALIZE_CPP2OPS(nebula::DataSet); +SPECIALIZE_CPP2OPS(nebula::Geography); } // namespace apache::thrift diff --git a/src/common/datatypes/Geography.cpp b/src/common/datatypes/Geography.cpp new file mode 100644 index 00000000000..1d30974cf55 --- /dev/null +++ b/src/common/datatypes/Geography.cpp @@ -0,0 +1,71 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "common/datatypes/Geography.h" + +#include +#include + +#include + +#include "common/geo/GeoUtils.h" +#include "common/geo/io/wkb/WKBReader.h" +#include "common/geo/io/wkb/WKBWriter.h" +#include "common/geo/io/wkt/WKTReader.h" +#include "common/geo/io/wkt/WKTWriter.h" + +namespace nebula { + +StatusOr> Geography::fromWKT(const std::string& wkt) { + auto geomRet = WKTReader().read(wkt); + NG_RETURN_IF_ERROR(geomRet); + auto geom = std::move(geomRet).value(); + auto wkb = WKBWriter().write(geom.get()); + return std::make_unique(wkb); +} + +GeoShape Geography::shape() const { + // TODO(jie) May store the shapetype as the data member of Geography is ok. + std::string wkbCopy = wkb; + uint8_t* beg = reinterpret_cast(wkbCopy.data()); + uint8_t* end = beg + wkbCopy.size(); + WKBReader reader; + auto byteOrderRet = reader.readByteOrder(beg, end); + DCHECK(byteOrderRet.ok()); + ByteOrder byteOrder = byteOrderRet.value(); + auto shapeTypeRet = reader.readShapeType(beg, end, byteOrder); + DCHECK(shapeTypeRet.ok()); + return shapeTypeRet.value(); +} + +std::string Geography::asWKT() const { + auto geomRet = WKBReader().read(wkb); + DCHECK(geomRet.ok()); + std::unique_ptr geom = std::move(geomRet).value(); + std::string wkt = WKTWriter().write(geom.get()); + LOG(INFO) << "Geography::asWKT(): " << wkt; + return wkt; +} + +std::string Geography::asWKB() const { return folly::hexlify(wkb); } + +std::unique_ptr Geography::asS2() const { + auto geomRet = WKBReader().read(wkb); + DCHECK(geomRet.ok()); + std::unique_ptr geom = std::move(geomRet).value(); + return GeoUtils::s2RegionFromGeomtry(geom.get()); +} + +} // namespace nebula + +namespace std { + +// Inject a customized hash function +std::size_t hash::operator()(const nebula::Geography& h) const noexcept { + return hash{}(h.wkb); +} + +} // namespace std diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h new file mode 100644 index 00000000000..ed8aecd1a86 --- /dev/null +++ b/src/common/datatypes/Geography.h @@ -0,0 +1,101 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include +#include +#include +#include + +#include +#include +#include + +#include "common/base/StatusOr.h" +#include "common/datatypes/Value.h" +#include "common/geo/io/Geometry.h" + +// Do not include here, it will indirectly includes a header file which defines a +// enum `BEGIN`(not enum class). While Geography.h is indirectly included by parser.yy, which has a +// macro named `BEGIN`. So they will be conflicted. + +class S2Polygon; + +namespace nebula { + +// clang-format off +/* +static const std::unordered_map kShapeTypeToS2Region = { + // S2PointRegion is a wrapper of S2Point, and it inherits from the S2Region class while S2Point doesn't. + {GeoShape::POINT, S2PointRegion}, + {GeoShape::LINESTRING, S2Polyline}, + {GeoShape::POLYGON, S2Polygon}, +}; +*/ +// clang-format on + +// Do not construct a S2 data when constructing Geography. It's expensive. +// We just construct S2 when doing computation. +struct Geography { + std::string wkb; // TODO(jie) Is it better to store Geometry* or S2Region* here? + + Geography() = default; + explicit Geography(const std::string& bytes) { + // TODO(jie): Ensure the bytes is valid + wkb = bytes; + LOG(INFO) << "Geography.wkb: " << wkb << ", wkb.size(): " << wkb.size(); + } + + static StatusOr> fromWKT(const std::string& wkt); + + GeoShape shape() const; + + std::string asWKT() const; + + std::string asWKB() const; + + std::unique_ptr asS2() const; + + std::string toString() const { return asWKT(); } + + folly::dynamic toJson() const { return toString(); } + + void clear() { wkb.clear(); } + + void __clear() { clear(); } + + bool operator==(const Geography& rhs) const { return wkb == rhs.wkb; } + + bool operator!=(const Geography& rhs) const { return !(wkb == rhs.wkb); } + + bool operator<(const Geography& rhs) const { return wkb < rhs.wkb; } + + private: + std::unique_ptr s2RegionFromGeomtry(const Geometry* geom) const; + + S2Point s2PointFromCoordinate(const Coordinate& coord) const; + + std::vector s2PointsFromCoordinateList(const std::vector& coordList) const; + + bool isLoopClosed(const std::vector& coordList) const; + + void removeAdjacentDuplicateCoordinates(std::vector& coordList) const; +}; + +inline std::ostream& operator<<(std::ostream& os, const Geography& g) { return os << g.wkb; } + +} // namespace nebula + +namespace std { + +// Inject a customized hash function +template <> +struct hash { + std::size_t operator()(const nebula::Geography& h) const noexcept; +}; + +} // namespace std diff --git a/src/common/datatypes/GeographyOps-inl.h b/src/common/datatypes/GeographyOps-inl.h new file mode 100644 index 00000000000..8e5804ae593 --- /dev/null +++ b/src/common/datatypes/GeographyOps-inl.h @@ -0,0 +1,132 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#ifndef COMMON_DATATYPES_GEOGRAPHYOPS_H_ +#define COMMON_DATATYPES_GEOGRAPHYOPS_H_ + +#include +#include +#include + +#include "common/base/Base.h" +#include "common/datatypes/CommonCpp2Ops.h" +#include "common/datatypes/Geography.h" + +namespace apache { +namespace thrift { + +/************************************** + * + * Ops for class Geography + * + *************************************/ +namespace detail { + +template <> +struct TccStructTraits { + static void translateFieldName(MAYBE_UNUSED folly::StringPiece _fname, + MAYBE_UNUSED int16_t& fid, + MAYBE_UNUSED apache::thrift::protocol::TType& _ftype) { + if (_fname == "wkb") { + fid = 1; + _ftype = apache::thrift::protocol::T_STRING; + } + } +}; + +} // namespace detail + +inline constexpr protocol::TType Cpp2Ops::thriftType() { + return apache::thrift::protocol::T_STRUCT; +} + +template +uint32_t Cpp2Ops::write(Protocol* proto, nebula::Geography const* obj) { + uint32_t xfer = 0; + xfer += proto->writeStructBegin("Geography"); + xfer += proto->writeFieldBegin("wkb", apache::thrift::protocol::T_STRING, 1); + xfer += proto->writeString(obj->wkb); + xfer += proto->writeFieldEnd(); + xfer += proto->writeFieldStop(); + xfer += proto->writeStructEnd(); + return xfer; +} + +template +void Cpp2Ops::read(Protocol* proto, nebula::Geography* obj) { + apache::thrift::detail::ProtocolReaderStructReadState readState; + + readState.readStructBegin(proto); + + using apache::thrift::TProtocolException; + + if (UNLIKELY(!readState.advanceToNextField(proto, 0, 1, apache::thrift::protocol::T_STRING))) { + goto _loop; + } +_readField_wkb : { proto->readString(obj->wkb); } + + if (UNLIKELY(!readState.advanceToNextField(proto, 1, 0, apache::thrift::protocol::T_STOP))) { + goto _loop; + } + +_end: + readState.readStructEnd(proto); + + return; + +_loop: + if (readState.fieldType == apache::thrift::protocol::T_STOP) { + goto _end; + } + + if (proto->kUsesFieldNames()) { + detail::TccStructTraits::translateFieldName( + readState.fieldName(), readState.fieldId, readState.fieldType); + } + + switch (readState.fieldId) { + case 1: { + if (LIKELY(readState.fieldType == apache::thrift::protocol::T_STRING)) { + goto _readField_wkb; + } else { + goto _skip; + } + } + default: { +_skip: + proto->skip(readState.fieldType); + readState.readFieldEnd(proto); + readState.readFieldBeginNoInline(proto); + goto _loop; + } + } +} + +template +uint32_t Cpp2Ops::serializedSize(Protocol const* proto, + nebula::Geography const* obj) { + uint32_t xfer = 0; + xfer += proto->serializedStructSize("Geography"); + xfer += proto->serializedFieldSize("wkb", apache::thrift::protocol::T_STRING, 1); + xfer += proto->serializedSizeString(obj->wkb); + xfer += proto->serializedSizeStop(); + return xfer; +} + +template +uint32_t Cpp2Ops::serializedSizeZC(Protocol const* proto, + nebula::Geography const* obj) { + uint32_t xfer = 0; + xfer += proto->serializedStructSize("Geography"); + xfer += proto->serializedFieldSize("wkb", apache::thrift::protocol::T_STRING, 1); + xfer += proto->serializedSizeString(obj->wkb); + xfer += proto->serializedSizeStop(); + return xfer; +} + +} // namespace thrift +} // namespace apache +#endif // COMMON_DATATYPES_GEOGRAPHYOPS_H_ diff --git a/src/common/datatypes/Value.cpp b/src/common/datatypes/Value.cpp index 19545de8135..3704ae4ff09 100644 --- a/src/common/datatypes/Value.cpp +++ b/src/common/datatypes/Value.cpp @@ -15,6 +15,7 @@ #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" @@ -64,6 +65,9 @@ std::size_t hash::operator()(const nebula::Value& v) const noexce case nebula::Value::Type::LIST: { return hash()(v.getList()); } + case nebula::Value::Type::GEOGRAPHY: { + return hash()(v.getGeography()); + } case nebula::Value::Type::MAP: { LOG(FATAL) << "Hash for MAP has not been implemented"; } @@ -164,6 +168,10 @@ Value::Value(Value&& rhs) noexcept : type_(Value::Type::__EMPTY__) { setG(std::move(rhs.value_.gVal)); break; } + case Type::GEOGRAPHY: { + setGG(std::move(rhs.value_.ggVal)); + break; + } default: { assert(false); break; @@ -240,6 +248,10 @@ Value::Value(const Value& rhs) : type_(Value::Type::__EMPTY__) { setG(rhs.value_.gVal); break; } + case Type::GEOGRAPHY: { + setGG(rhs.value_.ggVal); + break; + } default: { assert(false); break; @@ -333,6 +345,13 @@ Value::Value(const DataSet& v) { Value::Value(DataSet&& v) { setG(std::make_unique(std::move(v))); } +Value::Value(const Geography& v) { + auto c = std::make_unique(v); + setGG(std::move(c)); +} + +Value::Value(Geography&& v) { setGG(std::make_unique(std::move(v))); } + const std::string& Value::typeName() const { static const std::unordered_map typeNames = { {Type::__EMPTY__, "__EMPTY__"}, @@ -351,6 +370,7 @@ const std::string& Value::typeName() const { {Type::MAP, "map"}, {Type::SET, "set"}, {Type::DATASET, "dataset"}, + {Type::GEOGRAPHY, "geography"}, }; static const std::unordered_map nullTypes = { @@ -599,6 +619,21 @@ void Value::setDataSet(std::unique_ptr&& v) { setG(std::move(v)); } +void Value::setGeography(const Geography& v) { + clear(); + setGG(v); +} + +void Value::setGeography(Geography&& v) { + clear(); + setGG(std::move(v)); +} + +void Value::setGeography(std::unique_ptr&& v) { + clear(); + setGG(std::move(v)); +} + const NullType& Value::getNull() const { CHECK_EQ(type_, Type::NULLVALUE); return value_.nVal; @@ -709,6 +744,16 @@ const DataSet* Value::getDataSetPtr() const { return value_.gVal.get(); } +const Geography& Value::getGeography() const { + CHECK_EQ(type_, Type::GEOGRAPHY); + return *(value_.ggVal); +} + +const Geography* Value::getGeographyPtr() const { + CHECK_EQ(type_, Type::GEOGRAPHY); + return value_.ggVal.get(); +} + NullType& Value::mutableNull() { CHECK_EQ(type_, Type::NULLVALUE); return value_.nVal; @@ -784,6 +829,11 @@ DataSet& Value::mutableDataSet() { return *(value_.gVal); } +Geography& Value::mutableGeography() { + CHECK_EQ(type_, Type::GEOGRAPHY); + return *(value_.ggVal); +} + NullType Value::moveNull() { CHECK_EQ(type_, Type::NULLVALUE); NullType v = std::move(value_.nVal); @@ -889,6 +939,13 @@ DataSet Value::moveDataSet() { return ds; } +Geography Value::moveGeography() { + CHECK_EQ(type_, Type::GEOGRAPHY); + Geography v = std::move(*(value_.ggVal)); + clear(); + return v; +} + void Value::clear() { switch (type_) { case Type::__EMPTY__: { @@ -954,6 +1011,10 @@ void Value::clear() { destruct(value_.gVal); break; } + case Type::GEOGRAPHY: { + destruct(value_.ggVal); + break; + } } type_ = Type::__EMPTY__; } @@ -1027,6 +1088,10 @@ Value& Value::operator=(Value&& rhs) noexcept { setG(std::move(rhs.value_.gVal)); break; } + case Type::GEOGRAPHY: { + setGG(std::move(rhs.value_.ggVal)); + break; + } default: { assert(false); break; @@ -1105,6 +1170,10 @@ Value& Value::operator=(const Value& rhs) { setG(rhs.value_.gVal); break; } + case Type::GEOGRAPHY: { + setGG(rhs.value_.ggVal); + break; + } default: { assert(false); break; @@ -1344,6 +1413,26 @@ void Value::setG(DataSet&& v) { new (std::addressof(value_.gVal)) std::unique_ptr(new DataSet(std::move(v))); } +void Value::setGG(const std::unique_ptr& v) { + type_ = Type::GEOGRAPHY; + new (std::addressof(value_.ggVal)) std::unique_ptr(new Geography(*v)); +} + +void Value::setGG(std::unique_ptr&& v) { + type_ = Type::GEOGRAPHY; + new (std::addressof(value_.ggVal)) std::unique_ptr(std::move(v)); +} + +void Value::setGG(const Geography& v) { + type_ = Type::GEOGRAPHY; + new (std::addressof(value_.ggVal)) std::unique_ptr(new Geography(v)); +} + +void Value::setGG(Geography&& v) { + type_ = Type::GEOGRAPHY; + new (std::addressof(value_.ggVal)) std::unique_ptr(new Geography(std::move(v))); +} + // Convert Nebula::Value to a value compatible with Json standard // DATE, TIME, DATETIME will be converted to strings in UTC // VERTEX, EDGES, PATH will be converted to objects @@ -1403,6 +1492,9 @@ folly::dynamic Value::toJson() const { } case Value::Type::DATASET: { return getDataSet().toJson(); + } + case Value::Type::GEOGRAPHY: { + return getGeography().toJson(); } // no default so the compiler will warning when lack } @@ -1521,6 +1613,9 @@ std::string Value::toString() const { } case Value::Type::DATASET: { return getDataSet().toString(); + } + case Value::Type::GEOGRAPHY: { + return getGeography().toString(); } // no default so the compiler will warning when lack } @@ -1713,6 +1808,9 @@ Value Value::lessThan(const Value& v) const { case Value::Type::DATASET: { return getDataSet() < v.getDataSet(); } + case Value::Type::GEOGRAPHY: { + return getGeography() < v.getGeography(); + } case Value::Type::NULLVALUE: case Value::Type::__EMPTY__: { return kNullBadType; @@ -1800,6 +1898,9 @@ Value Value::equal(const Value& v) const { case Value::Type::DATASET: { return getDataSet() == v.getDataSet(); } + case Value::Type::GEOGRAPHY: { + return getGeography() == v.getGeography(); + } case Value::Type::NULLVALUE: case Value::Type::__EMPTY__: { return false; @@ -1881,6 +1982,10 @@ std::ostream& operator<<(std::ostream& os, const Value::Type& type) { os << "DATASET"; break; } + case Value::Type::GEOGRAPHY: { + os << "GEOGRAPHY"; + break; + } default: { os << "__UNKNOWN__"; break; @@ -2075,6 +2180,9 @@ Value operator+(const Value& lhs, const Value& rhs) { case Value::Type::DATASET: { return Value::kNullBadType; } + case Value::Type::GEOGRAPHY: { + return Value::kNullBadType; + } case Value::Type::__EMPTY__: { return Value::kEmpty; } @@ -2494,6 +2602,9 @@ bool operator<(const Value& lhs, const Value& rhs) { // TODO: return false; } + case Value::Type::GEOGRAPHY: { + return lhs.getGeography() < rhs.getGeography(); + } case Value::Type::NULLVALUE: case Value::Type::__EMPTY__: { return false; @@ -2578,6 +2689,9 @@ bool operator==(const Value& lhs, const Value& rhs) { case Value::Type::DATASET: { return lhs.getDataSet() == rhs.getDataSet(); } + case Value::Type::GEOGRAPHY: { + return lhs.getGeography() == rhs.getGeography(); + } case Value::Type::NULLVALUE: case Value::Type::__EMPTY__: { return false; diff --git a/src/common/datatypes/Value.h b/src/common/datatypes/Value.h index 57dc60df4e4..abd2f21173f 100644 --- a/src/common/datatypes/Value.h +++ b/src/common/datatypes/Value.h @@ -32,6 +32,7 @@ struct Map; struct List; struct Set; struct DataSet; +struct Geography; enum class NullType { __NULL__ = 0, @@ -76,6 +77,7 @@ struct Value { MAP = 1UL << 12, SET = 1UL << 13, DATASET = 1UL << 14, + GEOGRAPHY = 1UL << 15, NULLVALUE = 1UL << 63, }; @@ -127,6 +129,8 @@ struct Value { Value(Set&& v); // NOLINT Value(const DataSet& v); // NOLINT Value(DataSet&& v); // NOLINT + Value(const Geography& v); // NOLINT + Value(Geography&& v); // NOLINT ~Value() { clear(); } Type type() const noexcept { return type_; } @@ -159,6 +163,7 @@ struct Value { bool isMap() const { return type_ == Type::MAP; } bool isSet() const { return type_ == Type::SET; } bool isDataSet() const { return type_ == Type::DATASET; } + bool isGeography() const { return type_ == Type::GEOGRAPHY; } void clear(); @@ -211,6 +216,9 @@ struct Value { void setDataSet(const DataSet& v); void setDataSet(DataSet&& v); void setDataSet(std::unique_ptr&& v); + void setGeography(const Geography& v); + void setGeography(Geography&& v); + void setGeography(std::unique_ptr&& v); const NullType& getNull() const; const bool& getBool() const; @@ -234,6 +242,8 @@ struct Value { const Set* getSetPtr() const; const DataSet& getDataSet() const; const DataSet* getDataSetPtr() const; + const Geography& getGeography() const; + const Geography* getGeographyPtr() const; NullType moveNull(); bool moveBool(); @@ -250,6 +260,7 @@ struct Value { Map moveMap(); Set moveSet(); DataSet moveDataSet(); + Geography moveGeography(); NullType& mutableNull(); bool& mutableBool(); @@ -266,6 +277,7 @@ struct Value { Map& mutableMap(); Set& mutableSet(); DataSet& mutableDataSet(); + Geography& mutableGeography(); static const Value& null() noexcept { return kNullValue; } @@ -301,6 +313,7 @@ struct Value { std::unique_ptr mVal; std::unique_ptr uVal; std::unique_ptr gVal; + std::unique_ptr ggVal; Storage() {} ~Storage() {} @@ -372,6 +385,11 @@ struct Value { void setG(std::unique_ptr&& v); void setG(const DataSet& v); void setG(DataSet&& v); + // Geography value + void setGG(const std::unique_ptr& v); + void setGG(std::unique_ptr&& v); + void setGG(const Geography& v); + void setGG(Geography&& v); }; static_assert(sizeof(Value) == 16UL, "The size of Value should be 16UL"); diff --git a/src/common/datatypes/ValueOps-inl.h b/src/common/datatypes/ValueOps-inl.h index 124151ee8b4..32982081d2d 100644 --- a/src/common/datatypes/ValueOps-inl.h +++ b/src/common/datatypes/ValueOps-inl.h @@ -17,6 +17,7 @@ #include "common/datatypes/DataSetOps-inl.h" #include "common/datatypes/DateOps-inl.h" #include "common/datatypes/EdgeOps-inl.h" +#include "common/datatypes/GeographyOps-inl.h" #include "common/datatypes/ListOps-inl.h" #include "common/datatypes/MapOps-inl.h" #include "common/datatypes/PathOps-inl.h" @@ -79,6 +80,9 @@ struct TccStructTraits { } else if (_fname == "gVal") { fid = 15; _ftype = apache::thrift::protocol::T_STRUCT; + } else if (_fname == "ggVal") { + fid = 16; + _ftype = apache::thrift::protocol::T_STRUCT; } } }; @@ -229,6 +233,18 @@ uint32_t Cpp2Ops::write(Protocol* proto, nebula::Value const* obj xfer += proto->writeFieldEnd(); break; } + case nebula::Value::Type::GEOGRAPHY: { + xfer += proto->writeFieldBegin("ggVal", protocol::T_STRUCT, 16); + if (obj->getGeographyPtr()) { + xfer += Cpp2Ops::write(proto, obj->getGeographyPtr()); + } else { + xfer += proto->writeStructBegin("Geography"); + xfer += proto->writeStructEnd(); + xfer += proto->writeFieldStop(); + } + xfer += proto->writeFieldEnd(); + break; + } case nebula::Value::Type::__EMPTY__: { break; } @@ -409,6 +425,17 @@ void Cpp2Ops::read(Protocol* proto, nebula::Value* obj) { } break; } + case 16: { + if (readState.fieldType == apache::thrift::protocol::T_STRUCT) { + obj->setGeography(nebula::Geography()); + auto ptr = std::make_unique(); + Cpp2Ops::read(proto, ptr.get()); + obj->setGeography(std::move(ptr)); + } else { + proto->skip(readState.fieldType); + } + break; + } default: { proto->skip(readState.fieldType); break; @@ -543,6 +570,16 @@ uint32_t Cpp2Ops::serializedSize(Protocol const* proto, nebula::V } break; } + case nebula::Value::Type::GEOGRAPHY: { + xfer += proto->serializedFieldSize("ggVal", protocol::T_STRUCT, 16); + if (obj->getGeographyPtr()) { + xfer += Cpp2Ops::serializedSize(proto, obj->getGeographyPtr()); + } else { + xfer += proto->serializedStructSize("Geography"); + xfer += proto->serializedSizeStop(); + } + break; + } case nebula::Value::Type::__EMPTY__: { break; } @@ -671,6 +708,16 @@ uint32_t Cpp2Ops::serializedSizeZC(Protocol const* proto, nebula: } break; } + case nebula::Value::Type::GEOGRAPHY: { + xfer += proto->serializedFieldSize("ggVal", protocol::T_STRUCT, 16); + if (obj->getGeographyPtr()) { + xfer += Cpp2Ops::serializedSizeZC(proto, obj->getGeographyPtr()); + } else { + xfer += proto->serializedStructSize("Geography"); + xfer += proto->serializedSizeStop(); + } + break; + } case nebula::Value::Type::__EMPTY__: { break; } diff --git a/src/common/datatypes/test/CMakeLists.txt b/src/common/datatypes/test/CMakeLists.txt index 54517fda196..4c5bcce7e7c 100644 --- a/src/common/datatypes/test/CMakeLists.txt +++ b/src/common/datatypes/test/CMakeLists.txt @@ -11,6 +11,7 @@ nebula_add_test( OBJECTS $ $ + $ LIBRARIES gtest ) @@ -24,6 +25,7 @@ nebula_add_test( OBJECTS $ $ + $ LIBRARIES gtest ) @@ -39,6 +41,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -59,6 +62,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -75,6 +79,7 @@ nebula_add_test( OBJECTS $ $ + $ LIBRARIES gtest ) @@ -88,6 +93,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -106,6 +112,7 @@ nebula_add_executable( OBJECTS $ $ + $ LIBRARIES follybenchmark boost_regex @@ -120,6 +127,7 @@ nebula_add_executable( OBJECTS $ $ + $ LIBRARIES follybenchmark boost_regex diff --git a/src/common/datatypes/test/ValueTest.cpp b/src/common/datatypes/test/ValueTest.cpp index e29d3430c4e..67175a22bfc 100644 --- a/src/common/datatypes/test/ValueTest.cpp +++ b/src/common/datatypes/test/ValueTest.cpp @@ -11,6 +11,7 @@ #include "common/datatypes/DataSet.h" #include "common/datatypes/Date.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" diff --git a/src/common/expression/test/CMakeLists.txt b/src/common/expression/test/CMakeLists.txt index 6ea325a9648..20ba9d99f2a 100644 --- a/src/common/expression/test/CMakeLists.txt +++ b/src/common/expression/test/CMakeLists.txt @@ -19,6 +19,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -39,6 +40,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ @@ -60,6 +62,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ @@ -81,6 +84,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ @@ -101,6 +105,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -119,6 +124,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -137,6 +143,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -155,6 +162,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -173,6 +181,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -191,6 +200,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -209,6 +219,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -227,6 +238,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -245,6 +257,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -263,6 +276,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -281,6 +295,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -299,6 +314,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -317,6 +333,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -335,6 +352,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -353,6 +371,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -371,6 +390,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -389,6 +409,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -407,6 +428,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -425,6 +447,7 @@ nebula_add_test( $ $ $ + $ $ $ $ @@ -443,6 +466,7 @@ nebula_add_test( $ $ $ + $ $ $ $ diff --git a/src/common/function/test/CMakeLists.txt b/src/common/function/test/CMakeLists.txt index 9a2748c9dd5..65c794ca94c 100644 --- a/src/common/function/test/CMakeLists.txt +++ b/src/common/function/test/CMakeLists.txt @@ -10,6 +10,7 @@ nebula_add_test( FunctionManagerTest.cpp OBJECTS $ + $ $ $ $ @@ -29,6 +30,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES gtest gtest_main diff --git a/src/common/geo/CMakeLists.txt b/src/common/geo/CMakeLists.txt new file mode 100644 index 00000000000..64129d9bc83 --- /dev/null +++ b/src/common/geo/CMakeLists.txt @@ -0,0 +1,7 @@ +# Copyright (c) 2020 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License, +# attached with Common Clause Condition 1.0, found in the LICENSES directory. + +nebula_add_subdirectory(io) +# nebula_add_subdirectory(test) diff --git a/src/common/geo/GeoShape.h b/src/common/geo/GeoShape.h new file mode 100644 index 00000000000..2cd28700ff5 --- /dev/null +++ b/src/common/geo/GeoShape.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +namespace nebula { + +enum class GeoShape : uint32_t { + POINT = 1, + LINESTRING = 2, + POLYGON = 3, +}; + +} // namespace nebula diff --git a/src/common/geo/GeoUtils.h b/src/common/geo/GeoUtils.h new file mode 100644 index 00000000000..8d76513057d --- /dev/null +++ b/src/common/geo/GeoUtils.h @@ -0,0 +1,124 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include +#include + +#include "common/base/StatusOr.h" +#include "common/datatypes/Geography.h" +#include "common/geo/io/Geometry.h" + +namespace nebula { + +class GeoUtils final { + public: + static std::unique_ptr s2RegionFromGeomtry(const Geometry* geom) { + switch (geom->shape()) { + case GeoShape::POINT: { + const auto* point = static_cast(geom); + auto s2Point = s2PointFromCoordinate(point->coord); + return std::make_unique(s2Point); + } + case GeoShape::LINESTRING: { + const auto* lineString = static_cast(geom); + auto coordList = lineString->coordList; + DCHECK_GE(coordList.size(), 2); + removeAdjacentDuplicateCoordinates(coordList); + if (coordList.size() < 2) { + LOG(INFO) + << "Invalid LineString, adjacent coordinates must not be identical or antipodal."; + return nullptr; + } + + auto s2Points = s2PointsFromCoordinateList(coordList); + auto s2Polyline = std::make_unique(s2Points, S2Debug::DISABLE); + if (!s2Polyline->IsValid()) { + return nullptr; + } + return s2Polyline; + } + case GeoShape::POLYGON: { + const auto* polygon = static_cast(geom); + uint32_t numRings = polygon->numRings(); + std::vector> s2Loops; + s2Loops.reserve(numRings); + for (size_t i = 0; i < numRings; ++i) { + auto coordList = polygon->coordListList[i]; + DCHECK_GE(coordList.size(), 4); + DCHECK(isLoopClosed(coordList)); + removeAdjacentDuplicateCoordinates(coordList); + if (coordList.size() < 4) { + LOG(INFO) + << "Invalid linearRing in polygon, must have at least 4 distinct coordinates."; + return nullptr; + } + coordList.pop_back(); // Remove redundant last coordinate + auto s2Points = s2PointsFromCoordinateList(coordList); + auto* s2Loop = new S2Loop(std::move(s2Points), S2Debug::DISABLE); + if (!s2Loop->IsValid()) { + return nullptr; + } + s2Loop->Normalize(); // All loops must be oriented CCW(counterclockwise) for S2 + s2Loops.emplace_back(s2Loop); + } + auto s2Polygon = std::make_unique(std::move(s2Loops), S2Debug::DISABLE); + if (!s2Polygon->IsValid()) { // Exterior loop must contain other interior loops + return nullptr; + } + return s2Polygon; + } + default: + LOG(FATAL) + << "Geography shapes other than Point/LineString/Polygon are not currently supported"; + return nullptr; + } + } + + static S2Point s2PointFromCoordinate(const Coordinate& coord) { + auto latlng = S2LatLng::FromDegrees( + coord.y, coord.x); // Note: S2Point requires latitude to be first, and longitude to be last + DCHECK(latlng.is_valid()); + return latlng.ToPoint(); + } + + static std::vector s2PointsFromCoordinateList(const std::vector& coordList) { + std::vector s2Points; + uint32_t numCoords = coordList.size(); + s2Points.reserve(numCoords); + for (size_t i = 0; i < numCoords; ++i) { + auto coord = coordList[i]; + auto s2Point = s2PointFromCoordinate(coord); + s2Points.emplace_back(s2Point); + } + + return s2Points; + } + + static bool isLoopClosed(const std::vector& coordList) { + DCHECK_GE(coordList.size(), 2); + return coordList.front() == coordList.back(); + } + + static void removeAdjacentDuplicateCoordinates(std::vector& coordList) { + if (coordList.size() < 2) { + return; + } + + size_t i = 0, j = 1; + for (; j < coordList.size(); ++j) { + if (coordList[i] != coordList[j]) { + ++i; + if (i != j) { // i is always <= j + coordList[i] = coordList[j]; + } + } + } + } +}; + +} // namespace nebula diff --git a/src/common/geo/io/CMakeLists.txt b/src/common/geo/io/CMakeLists.txt new file mode 100644 index 00000000000..c2df1e27d35 --- /dev/null +++ b/src/common/geo/io/CMakeLists.txt @@ -0,0 +1,34 @@ +# Copyright (c) 2020 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License, +# attached with Common Clause Condition 1.0, found in the LICENSES directory. + +include_directories(${CMAKE_CURRENT_SOURCE_DIR}/wkt) +include_directories(${CMAKE_CURRENT_BINARY_DIR}/wkt) + +if(ENABLE_VERBOSE_BISON) + set(bison_flags "-Werror -v") +else() + set(bison_flags "-Werror") +endif() +bison_target(Parser wkt/wkt_parser.yy ${CMAKE_CURRENT_BINARY_DIR}/wkt/WKTParser.cpp COMPILE_FLAGS ${bison_flags}) +flex_target(Scanner wkt/wkt_scanner.lex ${CMAKE_CURRENT_BINARY_DIR}/wkt/WKTScanner.cpp) + +add_custom_target(wkt_parser_target DEPENDS ${FLEX_Scanner_OUTPUTS} ${BISON_Parser_OUTPUTS}) + +add_flex_bison_dependency(Scanner Parser) + +add_compile_options(-Wno-sign-compare -Wno-conversion-null -Wno-pedantic -Wno-extra) + +nebula_add_library( + wkt_wkb_io_obj OBJECT + ${FLEX_Scanner_OUTPUTS} + ${BISON_Parser_OUTPUTS} + wkt/WKTReader.cpp + wkt/WKTWriter.cpp + wkb/WKBReader.cpp + wkb/WKBWriter.cpp +) + +nebula_add_subdirectory(wkt) +# nebula_add_subdirectory(wkb) diff --git a/src/common/geo/io/Geometry.h b/src/common/geo/io/Geometry.h new file mode 100644 index 00000000000..51772bdba7b --- /dev/null +++ b/src/common/geo/io/Geometry.h @@ -0,0 +1,70 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include +#include +#include + +#include "common/geo/GeoShape.h" + +namespace nebula { + +// These Geometry structs are just for parsing wkt/wkb +struct Coordinate { + double x, y; + + Coordinate(double lng, double lat) : x(lng), y(lat) {} + + // TODO(jie) compare double correctly + bool operator==(const Coordinate &rhs) const { return x == rhs.x && y == rhs.y; } + bool operator!=(const Coordinate &rhs) const { return !(*this == rhs); } + + static bool isValidLng(double lng) { return std::abs(lng) <= 180; } + + static bool isValidLat(double lat) { return std::abs(lat) <= 90; } +}; + +struct Geometry { + virtual ~Geometry() {} + virtual GeoShape shape() const = 0; +}; + +struct Point : public Geometry { + Coordinate coord; + + explicit Point(const Coordinate &v) : coord(v) {} + explicit Point(Coordinate &&v) : coord(std::move(v)) {} + ~Point() override = default; + + GeoShape shape() const override { return GeoShape::POINT; } +}; + +struct LineString : public Geometry { + std::vector coordList; + + explicit LineString(const std::vector &v) : coordList(v) {} + explicit LineString(std::vector &&v) : coordList(std::move(v)) {} + ~LineString() override = default; + + GeoShape shape() const override { return GeoShape::LINESTRING; } + uint32_t numCoords() const { return coordList.size(); } +}; + +struct Polygon : public Geometry { + std::vector> coordListList; + + explicit Polygon(const std::vector> &v) : coordListList(v) {} + explicit Polygon(std::vector> &&v) : coordListList(std::move(v)) {} + ~Polygon() override = default; + + GeoShape shape() const override { return GeoShape::POLYGON; } + uint32_t numRings() const { return coordListList.size(); } + uint32_t numInteriorRing() const { return numRings() - 1; } +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkb/ByteOrder.h b/src/common/geo/io/wkb/ByteOrder.h new file mode 100644 index 00000000000..128e7dca829 --- /dev/null +++ b/src/common/geo/io/wkb/ByteOrder.h @@ -0,0 +1,57 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +namespace nebula { + +enum class ByteOrder : uint8_t { + BigEndian = 0, + LittleEndian = 1, +}; + +struct ByteOrderData { + static ByteOrder getMachineByteOrder() { + static int endianCheck = 1; + return static_cast( + *(reinterpret_cast(&endianCheck))); // 0 for BigEndian, 1 for LittleEndian + } + + static uint32_t getUint32(const uint8_t *buf, ByteOrder byteOrder) { + if (byteOrder == ByteOrder::BigEndian) { + return ((uint32_t)(buf[0] & 0xff) << 24) | ((uint32_t)(buf[1] & 0xff) << 16) | + ((uint32_t)(buf[2] & 0xff) << 8) | ((uint32_t)(buf[3] & 0xff)); + } else { + DCHECK(byteOrder == ByteOrder::LittleEndian); + return ((uint32_t)(buf[3] & 0xff) << 24) | ((uint32_t)(buf[2] & 0xff) << 16) | + ((uint32_t)(buf[1] & 0xff) << 8) | ((uint32_t)(buf[0] & 0xff)); + } + } + + static uint64_t getUint64(const uint8_t *buf, ByteOrder byteOrder) { + if (byteOrder == ByteOrder::BigEndian) { + return (uint64_t)(buf[0]) << 56 | (uint64_t)(buf[1] & 0xff) << 48 | + (uint64_t)(buf[2] & 0xff) << 40 | (uint64_t)(buf[3] & 0xff) << 32 | + (uint64_t)(buf[4] & 0xff) << 24 | (uint64_t)(buf[5] & 0xff) << 16 | + (uint64_t)(buf[6] & 0xff) << 8 | (uint64_t)(buf[7] & 0xff); + } else { + DCHECK(byteOrder == ByteOrder::LittleEndian); + return (uint64_t)(buf[7]) << 56 | (uint64_t)(buf[6] & 0xff) << 48 | + (uint64_t)(buf[5] & 0xff) << 40 | (uint64_t)(buf[4] & 0xff) << 32 | + (uint64_t)(buf[3] & 0xff) << 24 | (uint64_t)(buf[2] & 0xff) << 16 | + (uint64_t)(buf[1] & 0xff) << 8 | (uint64_t)(buf[0] & 0xff); + } + } + + static double getDouble(const uint8_t *buf, ByteOrder byteOrder) { + uint64_t v = getUint64(buf, byteOrder); + double ret; + std::memcpy(&ret, &v, sizeof(double)); + return ret; + } +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkb/WKBReader.cpp b/src/common/geo/io/wkb/WKBReader.cpp new file mode 100644 index 00000000000..b86d3147613 --- /dev/null +++ b/src/common/geo/io/wkb/WKBReader.cpp @@ -0,0 +1,11 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "common/geo/io/wkb/WKBReader.h" + +#include "common/base/Base.h" + +namespace nebula {} // namespace nebula diff --git a/src/common/geo/io/wkb/WKBReader.h b/src/common/geo/io/wkb/WKBReader.h new file mode 100644 index 00000000000..4bcb252efd2 --- /dev/null +++ b/src/common/geo/io/wkb/WKBReader.h @@ -0,0 +1,166 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include "common/base/StatusOr.h" +#include "common/geo/io/Geometry.h" +#include "common/geo/io/wkb/ByteOrder.h" + +namespace nebula { + +class WKBReader { + public: + WKBReader() {} + + ~WKBReader() {} + + // TODO(jie) Check the validity of geometry when reading the wkb + StatusOr> read(std::string wkb) const { + uint8_t *beg = reinterpret_cast(wkb.data()); + uint8_t *end = beg + wkb.size(); + return read(beg, end); + } + + StatusOr> read(uint8_t *&beg, uint8_t *end) const { + auto byteOrderRet = readByteOrder(beg, end); + NG_RETURN_IF_ERROR(byteOrderRet); + ByteOrder byteOrder = byteOrderRet.value(); + + auto shapeTypeRet = readShapeType(beg, end, byteOrder); + NG_RETURN_IF_ERROR(shapeTypeRet); + GeoShape shapeType = shapeTypeRet.value(); + + switch (shapeType) { + case GeoShape::POINT: { + auto coordRet = readCoordinate(beg, end, byteOrder); + NG_RETURN_IF_ERROR(coordRet); + Coordinate coord = coordRet.value(); + return std::make_unique(coord); + } + case GeoShape::LINESTRING: { + auto numPointsRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(numPointsRet); + uint32_t numPoints = numPointsRet.value(); + auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); + NG_RETURN_IF_ERROR(coordListRet); + std::vector coordList = coordListRet.value(); + return std::make_unique(coordList); + } + case GeoShape::POLYGON: { + auto numRingsRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(numRingsRet); + uint32_t numRings = numRingsRet.value(); + auto coordListListRet = readCoordinateListList(beg, end, byteOrder, numRings); + NG_RETURN_IF_ERROR(coordListListRet); + std::vector> coordListList = coordListListRet.value(); + return std::make_unique(coordListList); + } + default: + LOG(FATAL) + << "Geography shapes other than Point/LineString/Polygon are not currently supported"; + return Status::Error( + "Geography shapes other than Point/LineString/Polygon are not currently supported"); + } + } + + StatusOr readByteOrder(uint8_t *&beg, uint8_t *end) const { + auto vRet = readUint8(beg, end); + NG_RETURN_IF_ERROR(vRet); + uint8_t v = vRet.value(); + if (v != 0 && v != 1) { + return Status::Error("Unknown byte order"); + } + ByteOrder byteOrder = static_cast(v); + return byteOrder; + } + + StatusOr readShapeType(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { + auto vRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(vRet); + uint32_t v = vRet.value(); + if (v != 1 && v != 2 && v != 3) { + return Status::Error("Unknown shape type"); + } + GeoShape shapeType = static_cast(v); + return shapeType; + } + + StatusOr readCoordinate(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { + auto xRet = readDouble(beg, end, byteOrder); + NG_RETURN_IF_ERROR(xRet); + double x = xRet.value(); + auto yRet = readDouble(beg, end, byteOrder); + NG_RETURN_IF_ERROR(yRet); + double y = yRet.value(); + return Coordinate(x, y); + } + + StatusOr> readCoordinateList(uint8_t *&beg, + uint8_t *end, + ByteOrder byteOrder, + uint32_t num) const { + std::vector coordList; + coordList.reserve(num); + for (size_t i = 0; i < num; ++i) { + auto coordRet = readCoordinate(beg, end, byteOrder); + NG_RETURN_IF_ERROR(coordRet); + Coordinate coord = coordRet.value(); + coordList.emplace_back(coord); + } + return coordList; + } + + StatusOr>> readCoordinateListList(uint8_t *&beg, + uint8_t *end, + ByteOrder byteOrder, + uint32_t num) const { + std::vector> coordListList; + coordListList.reserve(num); + for (size_t i = 0; i < num; ++i) { + auto numPointsRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(numPointsRet); + uint32_t numPoints = numPointsRet.value(); + auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); + NG_RETURN_IF_ERROR(coordListRet); + std::vector coordList = coordListRet.value(); + coordListList.emplace_back(coordList); + } + return coordListList; + } + + StatusOr readUint8(uint8_t *&beg, uint8_t *end) const { + auto requiredSize = static_cast(sizeof(uint8_t)); + if (end - beg < requiredSize) { + return Status::Error("Unable to parse uint8_t"); + } + uint8_t v = *beg; + beg += requiredSize; + return v; + } + + StatusOr readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { + auto requiredSize = static_cast(sizeof(uint32_t)); + if (end - beg < requiredSize) { + return Status::Error("Unable to parse uint32_t"); + } + uint32_t v = ByteOrderData::getUint32(beg, byteOrder); + beg += requiredSize; + return v; + } + + StatusOr readDouble(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { + auto requiredSize = static_cast(sizeof(double)); + if (end - beg < requiredSize) { + return Status::Error("Unable to parse double"); + } + double v = ByteOrderData::getDouble(beg, byteOrder); + beg += requiredSize; + return v; + } +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkb/WKBWriter.cpp b/src/common/geo/io/wkb/WKBWriter.cpp new file mode 100644 index 00000000000..172a2679091 --- /dev/null +++ b/src/common/geo/io/wkb/WKBWriter.cpp @@ -0,0 +1,11 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "common/geo/io/wkb/WKBWriter.h" + +#include "common/base/Base.h" + +namespace nebula {} // namespace nebula diff --git a/src/common/geo/io/wkb/WKBWriter.h b/src/common/geo/io/wkb/WKBWriter.h new file mode 100644 index 00000000000..7077453b6f7 --- /dev/null +++ b/src/common/geo/io/wkb/WKBWriter.h @@ -0,0 +1,93 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include "common/geo/io/Geometry.h" +#include "common/geo/io/wkb/ByteOrder.h" + +namespace nebula { + +class WKBWriter { + public: + WKBWriter() {} + + ~WKBWriter() {} + + std::string write(const Geometry* geom) const { + std::string wkb = ""; + + uint8_t byteOrder = + static_cast>(ByteOrderData::getMachineByteOrder()); + writeUint8(wkb, byteOrder); + + auto shape = geom->shape(); + uint32_t shapeType = folly::to(shape); + writeUint32(wkb, shapeType); + switch (shape) { + case GeoShape::POINT: { + const Point* point = static_cast(geom); + writeCoordinate(wkb, point->coord); + return wkb; + } + case GeoShape::LINESTRING: { + const LineString* line = static_cast(geom); + auto coordList = line->coordList; + uint32_t numPoints = coordList.size(); + writeUint32(wkb, numPoints); + writeCoordinateList(wkb, coordList); + return wkb; + } + case GeoShape::POLYGON: { + const Polygon* polygon = static_cast(geom); + auto coordListList = polygon->coordListList; + uint32_t numRings = coordListList.size(); + writeUint32(wkb, numRings); + writeCoordinateListList(wkb, coordListList); + return wkb; + } + default: + LOG(FATAL) + << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; + return ""; + } + } + + void writeCoordinate(std::string& wkb, const Coordinate& coord) const { + writeDouble(wkb, coord.x); + writeDouble(wkb, coord.y); + } + + void writeCoordinateList(std::string& wkb, const std::vector& coordList) const { + for (size_t i = 0; i < coordList.size(); ++i) { + writeCoordinate(wkb, coordList[i]); + } + } + + void writeCoordinateListList(std::string& wkb, + const std::vector>& coordListList) const { + for (size_t i = 0; i < coordListList.size(); ++i) { + const auto& coordList = coordListList[i]; + uint32_t numPoints = coordList.size(); + writeUint32(wkb, numPoints); + writeCoordinateList(wkb, coordList); + } + } + + void writeUint8(std::string& wkb, uint8_t v) const { + wkb.append(reinterpret_cast(&v), sizeof(v)); + } + + void writeUint32(std::string& wkb, uint32_t v) const { + wkb.append(reinterpret_cast(&v), sizeof(v)); + } + + void writeDouble(std::string& wkb, double v) const { + wkb.append(reinterpret_cast(&v), sizeof(v)); + } +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkt/CMakeLists.txt b/src/common/geo/io/wkt/CMakeLists.txt new file mode 100644 index 00000000000..f359a95626a --- /dev/null +++ b/src/common/geo/io/wkt/CMakeLists.txt @@ -0,0 +1,6 @@ +# Copyright (c) 2020 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License, +# attached with Common Clause Condition 1.0, found in the LICENSES directory. + +nebula_add_subdirectory(test) diff --git a/src/common/geo/io/wkt/WKTReader.cpp b/src/common/geo/io/wkt/WKTReader.cpp new file mode 100644 index 00000000000..82e360219ef --- /dev/null +++ b/src/common/geo/io/wkt/WKTReader.cpp @@ -0,0 +1,11 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "common/geo/io/wkt/WKTReader.h" + +#include "common/base/Base.h" + +namespace nebula {} // namespace nebula diff --git a/src/common/geo/io/wkt/WKTReader.h b/src/common/geo/io/wkt/WKTReader.h new file mode 100644 index 00000000000..cf7164b1b94 --- /dev/null +++ b/src/common/geo/io/wkt/WKTReader.h @@ -0,0 +1,80 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include "common/base/Base.h" +#include "common/base/StatusOr.h" +#include "common/geo/io/Geometry.h" +#include "common/geo/io/wkt/WKTParser.hpp" +#include "common/geo/io/wkt/WKTScanner.h" + +namespace nebula { + +class WKTReader { + public: + WKTReader() : parser_(scanner_, error_, &geom_) { + // Callback invoked by WKTScanner + auto readBuffer = [this](char *buf, int maxSize) -> int { + // Reach the end + if (pos_ >= end_) { + pos_ = nullptr; + end_ = nullptr; + return 0; + } + int left = end_ - pos_; + auto n = maxSize > left ? left : maxSize; + ::memcpy(buf, pos_, n); + pos_ += n; + return n; // Number of bytes we actually filled in `buf' + }; + scanner_.setReadBuffer(std::move(readBuffer)); + } + + ~WKTReader() { + if (geom_ != nullptr) delete geom_; + } + + StatusOr> read(std::string wkt) { + // Since WKTScanner needs a writable buffer, we have to copy the query string + buffer_ = std::move(wkt); + pos_ = &buffer_[0]; + end_ = pos_ + buffer_.size(); + + scanner_.setWKT(&buffer_); + if (parser_.parse() != 0) { + pos_ = nullptr; + end_ = nullptr; + // To flush the internal buffer to recover from a failure + scanner_.flushBuffer(); + if (geom_ != nullptr) { + delete geom_; + geom_ = nullptr; + } + scanner_.setWKT(nullptr); + return Status::SyntaxError(error_); + } + + if (geom_ == nullptr) { + return Status::StatementEmpty(); // WKTEmpty() + } + auto *geom = geom_; + geom_ = nullptr; + scanner_.setWKT(nullptr); + return std::unique_ptr(geom); + } + + private: + std::string buffer_; + const char *pos_{nullptr}; + const char *end_{nullptr}; + nebula::WKTScanner scanner_; + nebula::WKTParser parser_; + std::string error_; + Geometry *geom_{nullptr}; +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkt/WKTScanner.h b/src/common/geo/io/wkt/WKTScanner.h new file mode 100644 index 00000000000..7e9c88a9cd8 --- /dev/null +++ b/src/common/geo/io/wkt/WKTScanner.h @@ -0,0 +1,67 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include "common/base/Base.h" + +// This macro must be defined before #include !!! +#define yyFlexLexer wktFlexLexer + +// Only include FlexLexer.h if it hasn't been already included +#if !defined(yyFlexLexerOnce) +#include +#endif + +// Override the interface for yylex since we namespaced it +#undef YY_DECL +#define YY_DECL int nebula::WKTScanner::yylex() + +#include "common/geo/io/wkt/WKTParser.hpp" + +namespace nebula { + +// TODO(jie) Try to reuse the class GraphScanner +class WKTScanner : public yyFlexLexer { + public: + int yylex(nebula::WKTParser::semantic_type *lval, nebula::WKTParser::location_type *loc) { + yylval = lval; + yylloc = loc; + return yylex(); + } + + public: + // Called by WKTReader to set the `readBuffer' callback, which would be + // invoked by LexerInput to fill the stream buffer. + void setReadBuffer(std::function readBuffer) { readBuffer_ = readBuffer; } + + // Manually invoked by WKTReader to recover from a failure state. + // This makes the scanner reentrant. + void flushBuffer() { + yy_flush_buffer(yy_buffer_stack ? yy_buffer_stack[yy_buffer_stack_top] : nullptr); + } + + void setWKT(std::string *wkt) { wkt_ = wkt; } + + std::string *wkt() { return wkt_; } + + protected: + // Called when YY_INPUT is invoked + int LexerInput(char *buf, int maxSize) override { return readBuffer_(buf, maxSize); } + + using TokenType = nebula::WKTParser::token; + + private: + // friend class Scanner_Basic_Test; TODO(jie) add it + int yylex() override; + + nebula::WKTParser::semantic_type *yylval{nullptr}; + nebula::WKTParser::location_type *yylloc{nullptr}; + std::function readBuffer_; + std::string *wkt_{nullptr}; +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkt/WKTWriter.cpp b/src/common/geo/io/wkt/WKTWriter.cpp new file mode 100644 index 00000000000..e1462561482 --- /dev/null +++ b/src/common/geo/io/wkt/WKTWriter.cpp @@ -0,0 +1,11 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "common/geo/io/wkt/WKTWriter.h" + +#include "common/base/Base.h" + +namespace nebula {} // namespace nebula diff --git a/src/common/geo/io/wkt/WKTWriter.h b/src/common/geo/io/wkt/WKTWriter.h new file mode 100644 index 00000000000..4e7764acbbb --- /dev/null +++ b/src/common/geo/io/wkt/WKTWriter.h @@ -0,0 +1,96 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#pragma once + +#include "common/geo/io/Geometry.h" + +namespace nebula { + +class WKTWriter { + public: + WKTWriter() {} + + ~WKTWriter() {} + + std::string write(const Geometry* geom) const { + std::string wkt = ""; + + auto shape = geom->shape(); + switch (shape) { + case GeoShape::POINT: { + wkt.append("POINT"); + const Point* point = static_cast(geom); + wkt.append("("); + writeCoordinate(wkt, point->coord); + wkt.append(")"); + return wkt; + } + case GeoShape::LINESTRING: { + wkt.append("LINESTRING"); + const LineString* line = static_cast(geom); + auto coordList = line->coordList; + uint32_t numPoints = coordList.size(); + UNUSED(numPoints); + wkt.append("("); + writeCoordinateList(wkt, coordList); + wkt.append(")"); + return wkt; + } + case GeoShape::POLYGON: { + wkt.append("POLYGON"); + const Polygon* polygon = static_cast(geom); + auto coordListList = polygon->coordListList; + uint32_t numRings = coordListList.size(); + UNUSED(numRings); + wkt.append("("); + writeCoordinateListList(wkt, coordListList); + wkt.append(")"); + return wkt; + } + default: + LOG(FATAL) + << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; + return ""; + } + } + + void writeCoordinate(std::string& wkt, const Coordinate& coord) const { + writeDouble(wkt, coord.x); + wkt.append(" "); + writeDouble(wkt, coord.y); + } + + void writeCoordinateList(std::string& wkt, const std::vector& coordList) const { + for (size_t i = 0; i < coordList.size(); ++i) { + writeCoordinate(wkt, coordList[i]); + wkt.append(","); + } + wkt.pop_back(); + } + + void writeCoordinateListList(std::string& wkt, + const std::vector>& coordListList) const { + for (size_t i = 0; i < coordListList.size(); ++i) { + const auto& coordList = coordListList[i]; + uint32_t numPoints = coordList.size(); + UNUSED(numPoints); + wkt.append("("); + writeCoordinateList(wkt, coordList); + wkt.append(")"); + wkt.append(","); + } + wkt.pop_back(); + } + + void writeDouble(std::string& wkt, double v) const { + wkt.append(folly::to(v)); + LOG(INFO) << "writeDouble(wkt, v): " << wkt << ", " << v + << ", folly::to: " << folly::to(v); + } +}; + +} // namespace nebula diff --git a/src/common/geo/io/wkt/test/CMakeLists.txt b/src/common/geo/io/wkt/test/CMakeLists.txt new file mode 100644 index 00000000000..3223d53f98a --- /dev/null +++ b/src/common/geo/io/wkt/test/CMakeLists.txt @@ -0,0 +1,54 @@ +# Copyright (c) 2020 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License, +# attached with Common Clause Condition 1.0, found in the LICENSES directory. + +set(WKT_PARSER_TEST_LIBS + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ + $ +) + +nebula_add_test( + NAME wkt_parser_test + SOURCES WKTParserTest.cpp + OBJECTS ${WKT_PARSER_TEST_LIBS} + LIBRARIES gtest gtest_main ${THRIFT_LIBRARIES} ${PROXYGEN_LIBRARIES} +) diff --git a/src/common/geo/io/wkt/test/WKTParserTest.cpp b/src/common/geo/io/wkt/test/WKTParserTest.cpp new file mode 100644 index 00000000000..e9cd4774a96 --- /dev/null +++ b/src/common/geo/io/wkt/test/WKTParserTest.cpp @@ -0,0 +1,137 @@ +/* Copyright (c) 2018 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include + +#include "common/base/Base.h" +#include "common/geo/io/wkt/WKTReader.h" +#include "common/geo/io/wkt/WKTWriter.h" + +namespace nebula { + +class WKTParserTest : public ::testing::Test { + public: + void SetUp() override {} + void TearDown() override {} + + protected: + StatusOr> parse(const std::string& wkt) { + auto geomRet = WKTReader().read(wkt); + NG_RETURN_IF_ERROR(geomRet); + NG_RETURN_IF_ERROR(check(geomRet.value().get())); + return geomRet; + } + + Status check(const Geometry* geom) { + auto wkt = WKTWriter().write(geom); + auto geomCopyRet = WKTReader().read(wkt); + auto geomCopy = std::move(geomCopyRet).value(); + auto wktCopy = WKTWriter().write(geomCopy.get()); + + if (wkt != wktCopy) { + return Status::Error("The reparsed geometry `%s' is different from origin `%s'.", + wktCopy.c_str(), + wkt.c_str()); + } + return Status::OK(); + } +}; + +TEST_F(WKTParserTest, TestWKTParser) { + // Point + { + std::string wkt = "POINT(24.7 36.842)"; + auto result = parse(wkt); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string wkt = "POINT(-179 36.842)"; + auto result = parse(wkt); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string wkt = "POINT(24.7, 36.842)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), "SyntaxError: syntax error near `, 36.842'"); + } + { + std::string wkt = "POINT(179,"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), "SyntaxError: syntax error near `,'"); + } + { + std::string wkt = "POINT(-190 36.842)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), + "SyntaxError: Longitude must be between -180 and 180 degrees near `-190'"); + } + { + std::string wkt = "POINT(179 91)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), + "SyntaxError: Latitude must be between -90 and 90 degrees near `91'"); + } + // LineString + { + std::string wkt = "LINESTRING(0 1, 1 2, 2 3, 3 4)"; + auto result = parse(wkt); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string wkt = "LINESTRING(26.4 78.9, 138.725 52)"; + auto result = parse(wkt); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string wkt = "LINESTRING(0 1, 2 3,)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), "SyntaxError: syntax error near `)'"); + } + { + std::string wkt = "LINESTRING(0 1)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), + "SyntaxError: LineString must have at least 2 coordinates near `0 1'"); + } + // Polygon + { + std::string wkt = "POLYGON((0 1, 1 2, 2 3, 0 1))"; + auto result = parse(wkt); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string wkt = "POLYGON((0 1, 1 2, 2 3, 0 1), (4 5, 5 6, 6 7, 9 9, 4 5))"; + auto result = parse(wkt); + ASSERT_TRUE(result.ok()) << result.status(); + } + { + std::string wkt = "POLYGON(0 1, 1 2, 2 3, 0 1)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), "SyntaxError: syntax error near `0 1, 1 2'"); + } + { + std::string wkt = "POLYGON((0 1, 1 2, 0 1)"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), "SyntaxError: syntax error near `)'"); + } + { + std::string wkt = "POLYGON((0 1, 1 2, 2 3, 3 4))"; + auto result = parse(wkt); + ASSERT_FALSE(result.ok()); + EXPECT_EQ(result.status().toString(), + "SyntaxError: Polygon's LinearRing must be closed near `(0 1, 1 2, 2 3, 3 4)'"); + } +} + +} // namespace nebula diff --git a/src/common/geo/io/wkt/wkt_parser.yy b/src/common/geo/io/wkt/wkt_parser.yy new file mode 100644 index 00000000000..4af77374ac9 --- /dev/null +++ b/src/common/geo/io/wkt/wkt_parser.yy @@ -0,0 +1,198 @@ +%language "C++" +%skeleton "lalr1.cc" +%no-lines +%locations +%define api.namespace { nebula } +%define parser_class_name { WKTParser } +%lex-param { nebula::WKTScanner& scanner } +%parse-param { nebula::WKTScanner& scanner } +%parse-param { std::string &errmsg } +%parse-param { nebula::Geometry** geom } + +%code requires { +#include +#include +#include +#include +#include "common/geo/io/Geometry.h" + +namespace nebula { + +class WKTScanner; + +} + +} + +%code { + #include "WKTScanner.h" + static int yylex(nebula::WKTParser::semantic_type* yylval, + nebula::WKTParser::location_type *yylloc, + nebula::WKTScanner& scanner); +} + +%union { + double doubleVal; + Geometry* geomVal; + Point* pointVal; + LineString* lineVal; + Polygon* polygonVal; + Coordinate* coordVal; + std::vector* coordListVal; + std::vector>* coordListListVal; +} + +/* destructors */ +%destructor {} +%destructor {} +%destructor {} + +/* wkt shape type prefix */ +%token KW_POINT KW_LINESTRING KW_POLYGON + +/* symbols */ +%token L_PAREN R_PAREN COMMA + +/* token type specification */ +%token DOUBLE + +%type geometry +%type point +%type linestring +%type polygon +%type coordinate +%type coordinate_list +%type coordinate_list_list + +%define api.prefix {wkt} + +%start geometry + +%% + +geometry + : point { + $$ = $1; + LOG(INFO) << "jie test" << static_cast($$)->coord.x << static_cast($$)->coord.y; + *geom = $$; + } + | linestring { + $$ = $1; + *geom = $$; + } + | polygon { + $$ = $1; + *geom = $$; + } +; + +point + : KW_POINT L_PAREN coordinate R_PAREN { + $$ = new Point(*$3); + } + ; + +linestring + : KW_LINESTRING L_PAREN coordinate_list R_PAREN { + if ($3->size() < 2) { + throw nebula::WKTParser::syntax_error(@3, "LineString must have at least 2 coordinates"); + } + $$ = new LineString(*$3); + } + ; + +polygon + : KW_POLYGON L_PAREN coordinate_list_list R_PAREN { + for (size_t i = 0; i < $3->size(); ++i) { + auto coordList = (*$3)[i]; + if (coordList.size() < 4) { + throw nebula::WKTParser::syntax_error(@3, "Polygon's LinearRing must have at least 4 coordinates"); + } + if (coordList.front() != coordList.back()) { + throw nebula::WKTParser::syntax_error(@3, "Polygon's LinearRing must be closed"); + } + } + $$ = new Polygon(*$3); + } + ; + +coordinate + : DOUBLE DOUBLE { + if (!Coordinate::isValidLng($1)) { + throw nebula::WKTParser::syntax_error(@1, "Longitude must be between -180 and 180 degrees"); + } + if (!Coordinate::isValidLat($2)) { + throw nebula::WKTParser::syntax_error(@2, "Latitude must be between -90 and 90 degrees"); + } + $$ = new Coordinate($1, $2); + } + ; + +coordinate_list + : coordinate { + $$ = new std::vector(); + $$->emplace_back(*$1); + } + | coordinate_list COMMA coordinate { + $$ = $1; + $$->emplace_back(*$3); + } + ; + +coordinate_list_list + : L_PAREN coordinate_list R_PAREN { + $$ = new std::vector>(); + $$->emplace_back(*$2); + + } + | coordinate_list_list COMMA L_PAREN coordinate_list R_PAREN { + $$ = $1; + $$->emplace_back(*$4); + } + ; + +%% + +void nebula::WKTParser::error(const nebula::WKTParser::location_type& loc, + const std::string &msg) { + std::ostringstream os; + if (msg.empty()) { + os << "syntax error"; + } else { + os << msg; + } + + auto *wkt = scanner.wkt(); + if (wkt == nullptr) { + os << " at " << loc; + errmsg = os.str(); + return; + } + + auto begin = loc.begin.column > 0 ? loc.begin.column - 1 : 0; + if ((loc.end.filename + && (!loc.begin.filename + || *loc.begin.filename != *loc.end.filename)) + || loc.begin.line < loc.end.line + || begin >= wkt->size()) { + os << " at " << loc; + } else if (loc.begin.column < (loc.end.column ? loc.end.column - 1 : 0)) { + uint32_t len = loc.end.column - loc.begin.column; + if (len > 80) { + len = 80; + } + os << " near `" << wkt->substr(begin, len) << "'"; + } else { + os << " near `" << wkt->substr(begin, 8) << "'"; + } + + errmsg = os.str(); +} + +static int yylex(nebula::WKTParser::semantic_type* yylval, + nebula::WKTParser::location_type *yylloc, + nebula::WKTScanner& scanner) { + auto token = scanner.yylex(yylval, yylloc); + return token; +} + diff --git a/src/common/geo/io/wkt/wkt_scanner.lex b/src/common/geo/io/wkt/wkt_scanner.lex new file mode 100644 index 00000000000..8ac5a27122f --- /dev/null +++ b/src/common/geo/io/wkt/wkt_scanner.lex @@ -0,0 +1,63 @@ +%option c++ +%option yyclass="WKTScanner" +%option nodefault noyywrap +%option never-interactive +%option yylineno +%option case-insensitive +%option prefix="wkt" + +%{ +#include "common/geo/io/wkt/WKTScanner.h" +#include "WKTParser.hpp" +#include + +#define YY_USER_ACTION \ + yylloc->step(); \ + yylloc->columns(yyleng); + +%} + +%% + + /* WKT shape type prefix */ +"POINT" { return TokenType::KW_POINT; } +"LINESTRING" { return TokenType::KW_LINESTRING; } +"POLYGON" { return TokenType::KW_POLYGON; } + +"," { return TokenType::COMMA; } +"(" { return TokenType::L_PAREN; } +")" { return TokenType::R_PAREN; } + +-?(([0-9]+\.?)|([0-9]*\.?[0-9]+)([eE][-+]?[0-9]+)?) { + yylval->doubleVal = atof(yytext); + return TokenType::DOUBLE; +} + +[ \t\n\r] {} + +. { + /** + * Any other unmatched byte sequences will get us here, + * including the non-ascii ones, which are negative + * in terms of type of `signed char'. At the same time, because + * Bison translates all negative tokens to EOF(i.e. YY_NULL), + * so we have to cast illegal characters to type of `unsinged char' + * This will make Bison receive an unknown token, which leads to + * a syntax error. + * + * Please note that it is not Flex but Bison to regard illegal + * characters as errors, in such case. + */ + return static_cast(yytext[0]); + + /** + * Alternatively, we could report illegal characters by + * throwing a `syntax_error' exception. + * In such a way, we could distinguish illegal characters + * from normal syntax errors, but at cost of poor performance + * incurred by the expensive exception handling. + */ + // throw WKTParser::syntax_error(*yylloc, "char illegal"); + } + +%% diff --git a/src/common/graph/tests/CMakeLists.txt b/src/common/graph/tests/CMakeLists.txt index 72795420487..275b6eb1545 100644 --- a/src/common/graph/tests/CMakeLists.txt +++ b/src/common/graph/tests/CMakeLists.txt @@ -14,6 +14,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES gtest gtest_main diff --git a/src/common/meta/NebulaSchemaProvider.cpp b/src/common/meta/NebulaSchemaProvider.cpp index 72a72402a2b..dc5aab4a2c5 100644 --- a/src/common/meta/NebulaSchemaProvider.cpp +++ b/src/common/meta/NebulaSchemaProvider.cpp @@ -160,6 +160,8 @@ std::size_t NebulaSchemaProvider::fieldSize(cpp2::PropertyType type, std::size_t sizeof(int8_t) + // minute sizeof(int8_t) + // sec sizeof(int32_t); // microsec + case cpp2::PropertyType::GEOGRAPHY: + return 8; // as same as STRING case cpp2::PropertyType::UNKNOWN: break; } diff --git a/src/common/thread/GenericWorker.h b/src/common/thread/GenericWorker.h index 5d10ba92cad..83e3de063a8 100644 --- a/src/common/thread/GenericWorker.h +++ b/src/common/thread/GenericWorker.h @@ -52,7 +52,7 @@ class GenericWorker final : public nebula::cpp::NonCopyable, public nebula::cpp: * A GenericWorker MUST be `start'ed successfully before invoking * any other interfaces. */ - bool MUST_USE_RESULT start(std::string name = ""); + bool NG_MUST_USE_RESULT start(std::string name = ""); /** * Asynchronouly to notify the worker to stop handling further new tasks. diff --git a/src/common/time/TimezoneInfo.h b/src/common/time/TimezoneInfo.h index 34795c5f5c4..221dba3c3fa 100644 --- a/src/common/time/TimezoneInfo.h +++ b/src/common/time/TimezoneInfo.h @@ -24,7 +24,7 @@ class Timezone { public: Timezone() = default; - static MUST_USE_RESULT Status init() { + static NG_MUST_USE_RESULT Status init() { try { tzdb.load_from_file(FLAGS_timezone_file); } catch (const std::exception &e) { @@ -34,7 +34,7 @@ class Timezone { return Status::OK(); } - MUST_USE_RESULT Status loadFromDb(const std::string ®ion) { + NG_MUST_USE_RESULT Status loadFromDb(const std::string ®ion) { zoneInfo_ = tzdb.time_zone_from_region(region); if (zoneInfo_ == nullptr) { return Status::Error("Not supported timezone `%s'.", region.c_str()); @@ -44,7 +44,7 @@ class Timezone { // see the posix timezone literal format in // https://man7.org/linux/man-pages/man3/tzset.3.html - MUST_USE_RESULT Status parsePosixTimezone(const std::string &posixTimezone) { + NG_MUST_USE_RESULT Status parsePosixTimezone(const std::string &posixTimezone) { try { zoneInfo_.reset(new ::boost::local_time::posix_time_zone(posixTimezone)); } catch (const std::exception &e) { diff --git a/src/common/time/test/CMakeLists.txt b/src/common/time/test/CMakeLists.txt index 95010c73898..36cf1ed429d 100644 --- a/src/common/time/test/CMakeLists.txt +++ b/src/common/time/test/CMakeLists.txt @@ -50,6 +50,7 @@ nebula_add_test( $ $ $ + $ LIBRARIES gtest ) diff --git a/src/common/utils/IndexKeyUtils.h b/src/common/utils/IndexKeyUtils.h index 71d0b619ccf..666f126c982 100644 --- a/src/common/utils/IndexKeyUtils.h +++ b/src/common/utils/IndexKeyUtils.h @@ -48,6 +48,8 @@ class IndexKeyUtils final { return Value::Type::TIME; case PropertyType::DATETIME: return Value::Type::DATETIME; + case PropertyType::GEOGRAPHY: + return Value::Type::GEOGRAPHY; case PropertyType::UNKNOWN: return Value::Type::__EMPTY__; } @@ -85,6 +87,10 @@ class IndexKeyUtils final { len = sizeof(int32_t) + sizeof(int16_t) + sizeof(int8_t) * 5; break; } + case Value::Type::GEOGRAPHY: { + len = sizeof(uint64_t); // S2CellId + break; + } default: LOG(ERROR) << "Unsupported default value type"; } @@ -131,6 +137,10 @@ class IndexKeyUtils final { case Value::Type::DATETIME: { return encodeDateTime(v.getDateTime()); } + case Value::Type::GEOGRAPHY: { + // TODO(jie) + return ""; + } default: LOG(ERROR) << "Unsupported default value type"; } @@ -164,6 +174,11 @@ class IndexKeyUtils final { return val; } + static std::string encodeUint64(uint64_t v) { + auto val = folly::Endian::big(v); + return {reinterpret_cast(&val), sizeof(uint64_t)}; + } + static std::string encodeRank(EdgeRanking rank) { return IndexKeyUtils::encodeInt64(rank); } static EdgeRanking decodeRank(const folly::StringPiece& raw) { @@ -332,6 +347,10 @@ class IndexKeyUtils final { v.setDateTime(decodeDateTime(raw)); break; } + case Value::Type::GEOGRAPHY: { + // unable to get geography value from index key + return Value::kNullBadData; + } default: return Value(NullType::BAD_DATA); } @@ -396,6 +415,11 @@ class IndexKeyUtils final { len = sizeof(int32_t) + sizeof(int16_t) + sizeof(int8_t) * 5; break; } + case Value::Type::GEOGRAPHY: { + // LOG(FATAL) << "unable to get geography value from index key" + len = sizeof(uint64_t); + break; + } default: len = 0; } diff --git a/src/common/utils/test/CMakeLists.txt b/src/common/utils/test/CMakeLists.txt index 0f58aa14fd8..7e9d59739ff 100644 --- a/src/common/utils/test/CMakeLists.txt +++ b/src/common/utils/test/CMakeLists.txt @@ -8,6 +8,7 @@ nebula_add_test( $ $ $ + $ $ $ LIBRARIES @@ -25,6 +26,7 @@ nebula_add_test( $ $ $ + $ $ $ LIBRARIES @@ -42,6 +44,7 @@ nebula_add_test( $ $ $ + $ $ $ LIBRARIES diff --git a/src/daemons/CMakeLists.txt b/src/daemons/CMakeLists.txt index b1ecaa7fbea..b9c9a9e1d82 100644 --- a/src/daemons/CMakeLists.txt +++ b/src/daemons/CMakeLists.txt @@ -23,9 +23,11 @@ set(common_deps $ $ $ + $ $ $ $ + $ $ $ $ diff --git a/src/graph/context/test/CMakeLists.txt b/src/graph/context/test/CMakeLists.txt index 02d05be1d42..a4ac8309df2 100644 --- a/src/graph/context/test/CMakeLists.txt +++ b/src/graph/context/test/CMakeLists.txt @@ -9,6 +9,7 @@ SET(CONTEXT_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/graph/executor/test/CMakeLists.txt b/src/graph/executor/test/CMakeLists.txt index 484240809bd..b97672192c0 100644 --- a/src/graph/executor/test/CMakeLists.txt +++ b/src/graph/executor/test/CMakeLists.txt @@ -23,10 +23,12 @@ SET(EXEC_QUERY_TEST_OBJS $ $ $ + $ $ $ $ $ + $ $ $ $ diff --git a/src/graph/optimizer/OptimizerUtils.cpp b/src/graph/optimizer/OptimizerUtils.cpp index 53f7e89c240..2fb82011b0c 100644 --- a/src/graph/optimizer/OptimizerUtils.cpp +++ b/src/graph/optimizer/OptimizerUtils.cpp @@ -190,6 +190,7 @@ Value OptimizerUtils::boundValueWithGT(const meta::cpp2::ColumnDef& col, const V case Value::Type::SET: case Value::Type::MAP: case Value::Type::DATASET: + case Value::Type::GEOGRAPHY: // TODO(jie) case Value::Type::PATH: { DLOG(FATAL) << "Not supported value type " << type << "for index."; return Value::kNullBadType; @@ -336,6 +337,7 @@ Value OptimizerUtils::boundValueWithLT(const meta::cpp2::ColumnDef& col, const V case Value::Type::SET: case Value::Type::MAP: case Value::Type::DATASET: + case Value::Type::GEOGRAPHY: // TODO(jie) case Value::Type::PATH: { DLOG(FATAL) << "Not supported value type " << type << "for index."; return Value::kNullBadType; @@ -395,6 +397,7 @@ Value OptimizerUtils::boundValueWithMax(const meta::cpp2::ColumnDef& col) { case Value::Type::SET: case Value::Type::MAP: case Value::Type::DATASET: + case Value::Type::GEOGRAPHY: // TODO(jie) case Value::Type::PATH: { DLOG(FATAL) << "Not supported value type " << type << "for index."; return Value::kNullBadType; @@ -437,6 +440,7 @@ Value OptimizerUtils::boundValueWithMin(const meta::cpp2::ColumnDef& col) { case Value::Type::SET: case Value::Type::MAP: case Value::Type::DATASET: + case Value::Type::GEOGRAPHY: // TODO(jie) case Value::Type::PATH: { DLOG(FATAL) << "Not supported value type " << type << "for index."; return Value::kNullBadType; @@ -482,6 +486,7 @@ Value OptimizerUtils::normalizeValue(const meta::cpp2::ColumnDef& col, const Val case Value::Type::SET: case Value::Type::MAP: case Value::Type::DATASET: + case Value::Type::GEOGRAPHY: // TODO(jie) case Value::Type::PATH: { DLOG(FATAL) << "Not supported value type " << type << "for index."; return Value::kNullBadType; diff --git a/src/graph/optimizer/test/CMakeLists.txt b/src/graph/optimizer/test/CMakeLists.txt index a1fa426ac72..222c8835e28 100644 --- a/src/graph/optimizer/test/CMakeLists.txt +++ b/src/graph/optimizer/test/CMakeLists.txt @@ -10,6 +10,7 @@ set(OPTIMIZER_TEST_LIB $ $ $ + $ $ $ $ diff --git a/src/graph/planner/test/CMakeLists.txt b/src/graph/planner/test/CMakeLists.txt index b4de1974764..997010c4511 100644 --- a/src/graph/planner/test/CMakeLists.txt +++ b/src/graph/planner/test/CMakeLists.txt @@ -32,6 +32,7 @@ nebula_add_test( $ $ $ + $ $ $ $ diff --git a/src/graph/service/Authenticator.h b/src/graph/service/Authenticator.h index 7c4d90439bc..ea390b8a148 100644 --- a/src/graph/service/Authenticator.h +++ b/src/graph/service/Authenticator.h @@ -16,7 +16,7 @@ class Authenticator { public: virtual ~Authenticator() {} - virtual bool MUST_USE_RESULT auth(const std::string &user, const std::string &password) = 0; + virtual bool NG_MUST_USE_RESULT auth(const std::string &user, const std::string &password) = 0; }; } // namespace graph diff --git a/src/graph/service/GraphService.h b/src/graph/service/GraphService.h index f72c36bb39e..76dc0f811b1 100644 --- a/src/graph/service/GraphService.h +++ b/src/graph/service/GraphService.h @@ -25,8 +25,8 @@ class GraphService final : public cpp2::GraphServiceSvIf { GraphService() = default; ~GraphService() = default; - Status MUST_USE_RESULT init(std::shared_ptr ioExecutor, - const HostAddr& hostAddr); + Status NG_MUST_USE_RESULT init(std::shared_ptr ioExecutor, + const HostAddr& hostAddr); folly::Future future_authenticate(const std::string& username, const std::string& password) override; diff --git a/src/graph/util/SchemaUtil.cpp b/src/graph/util/SchemaUtil.cpp index d24c604229e..ccbcb9ca028 100644 --- a/src/graph/util/SchemaUtil.cpp +++ b/src/graph/util/SchemaUtil.cpp @@ -294,6 +294,8 @@ Value::Type SchemaUtil::propTypeToValueType(meta::cpp2::PropertyType propType) { return Value::Type::DATE; case meta::cpp2::PropertyType::DATETIME: return Value::Type::DATETIME; + case meta::cpp2::PropertyType::GEOGRAPHY: + return Value::Type::GEOGRAPHY; case meta::cpp2::PropertyType::UNKNOWN: return Value::Type::__EMPTY__; } diff --git a/src/graph/util/test/CMakeLists.txt b/src/graph/util/test/CMakeLists.txt index 745ab224efe..2076f613254 100644 --- a/src/graph/util/test/CMakeLists.txt +++ b/src/graph/util/test/CMakeLists.txt @@ -10,6 +10,7 @@ nebula_add_test( $ $ $ + $ $ $ $ diff --git a/src/graph/validator/Validator.h b/src/graph/validator/Validator.h index ab504d89d1f..4d1320ff14b 100644 --- a/src/graph/validator/Validator.h +++ b/src/graph/validator/Validator.h @@ -32,7 +32,7 @@ class Validator { Status validate(); - MUST_USE_RESULT Status appendPlan(PlanNode* tail); + NG_MUST_USE_RESULT Status appendPlan(PlanNode* tail); void setInputVarName(std::string name) { inputVarName_ = std::move(name); } diff --git a/src/graph/validator/test/CMakeLists.txt b/src/graph/validator/test/CMakeLists.txt index 487e7791f4a..60e004bfe03 100644 --- a/src/graph/validator/test/CMakeLists.txt +++ b/src/graph/validator/test/CMakeLists.txt @@ -33,6 +33,7 @@ set(VALIDATOR_TEST_LIBS $ $ $ + $ $ $ $ @@ -41,6 +42,7 @@ set(VALIDATOR_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/graph/visitor/test/CMakeLists.txt b/src/graph/visitor/test/CMakeLists.txt index 159e50b613a..ec7c18f8775 100644 --- a/src/graph/visitor/test/CMakeLists.txt +++ b/src/graph/visitor/test/CMakeLists.txt @@ -46,6 +46,7 @@ nebula_add_test( $ $ $ + $ $ $ $ diff --git a/src/interface/common.thrift b/src/interface/common.thrift index 45dcedea3d4..e571729514b 100644 --- a/src/interface/common.thrift +++ b/src/interface/common.thrift @@ -25,6 +25,7 @@ cpp_include "common/datatypes/SetOps-inl.h" cpp_include "common/datatypes/DataSetOps-inl.h" cpp_include "common/datatypes/KeyValueOps-inl.h" cpp_include "common/datatypes/HostAddrOps-inl.h" +cpp_include "common/datatypes/GeographyOps-inl.h" /* * @@ -85,7 +86,6 @@ struct DateTime { 7: i32 microsec; // Micro-second: 0 - 999,999 } (cpp.type = "nebula::DateTime") - enum NullType { __NULL__ = 0, NaN = 1, @@ -115,6 +115,7 @@ union Value { 13: NMap (cpp.type = "nebula::Map") mVal (cpp.ref_type = "unique"); 14: NSet (cpp.type = "nebula::Set") uVal (cpp.ref_type = "unique"); 15: DataSet (cpp.type = "nebula::DataSet") gVal (cpp.ref_type = "unique"); + 16: Geography (cpp.type = "nebula::Geography") ggVal (cpp.ref_type = "unique"); } (cpp.type = "nebula::Value") @@ -146,6 +147,10 @@ struct DataSet { 2: list rows; } (cpp.type = "nebula::DataSet") +struct Geography { + 1: string wkb; +} (cpp.type = "nebula::Geography") + struct Tag { 1: binary name, diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index d4fe48298a6..c84ef0193ec 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -60,6 +60,15 @@ union ID { } +// Geo shape type +enum GeoShape { + ANY = 0, + POINT = 1, + LINESTRING = 2, + POLYGON = 3, +} (cpp.enum_strict) + + // These are all data types supported in the graph properties enum PropertyType { UNKNOWN = 0, @@ -83,12 +92,17 @@ enum PropertyType { DATE = 24, DATETIME = 25, TIME = 26, + + // Geo spatial + GEOGRAPHY = 31, } (cpp.enum_strict) struct ColumnTypeDef { 1: required PropertyType type, // type_length is valid for fixed_string type 2: optional i16 type_length = 0, + // geo_shape is valid for geography type + 3: optional GeoShape geo_shape, } struct ColumnDef { diff --git a/src/kvstore/raftex/test/CMakeLists.txt b/src/kvstore/raftex/test/CMakeLists.txt index 3a0abe49b43..c40b29720c3 100644 --- a/src/kvstore/raftex/test/CMakeLists.txt +++ b/src/kvstore/raftex/test/CMakeLists.txt @@ -5,6 +5,7 @@ set(RAFTEX_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/kvstore/test/CMakeLists.txt b/src/kvstore/test/CMakeLists.txt index a4d9c040d58..6f3db67d2cd 100644 --- a/src/kvstore/test/CMakeLists.txt +++ b/src/kvstore/test/CMakeLists.txt @@ -25,6 +25,7 @@ set(KVSTORE_TEST_LIBS $ $ $ + $ $ $ $ @@ -200,4 +201,4 @@ nebula_add_test( ${PROXYGEN_LIBRARIES} wangle gtest -) \ No newline at end of file +) diff --git a/src/meta/CMakeLists.txt b/src/meta/CMakeLists.txt index 5c69e0336eb..6fb07b41669 100644 --- a/src/meta/CMakeLists.txt +++ b/src/meta/CMakeLists.txt @@ -154,6 +154,7 @@ set(meta_test_deps $ $ $ + $ $ $ $ diff --git a/src/meta/processors/schema/SchemaUtil.cpp b/src/meta/processors/schema/SchemaUtil.cpp index 5d1b45f80fd..a135ad4df27 100644 --- a/src/meta/processors/schema/SchemaUtil.cpp +++ b/src/meta/processors/schema/SchemaUtil.cpp @@ -158,6 +158,13 @@ bool SchemaUtil::checkType(std::vector& columns) { return false; } break; + case cpp2::PropertyType::GEOGRAPHY: + if (!value.isGeography()) { // TODO(jie) + LOG(ERROR) << "Invalid default value for ` " << name << "', value type is " + << value.type(); + return false; + } + break; default: LOG(ERROR) << "Unsupported type"; return false; diff --git a/src/meta/test/CMakeLists.txt b/src/meta/test/CMakeLists.txt index 355d06cd468..bcedc7f01d2 100644 --- a/src/meta/test/CMakeLists.txt +++ b/src/meta/test/CMakeLists.txt @@ -18,6 +18,7 @@ nebula_add_test( $ $ $ + $ $ $ $ diff --git a/src/parser/MaintainSentences.h b/src/parser/MaintainSentences.h index ee2e0393b1c..603e52cba30 100644 --- a/src/parser/MaintainSentences.h +++ b/src/parser/MaintainSentences.h @@ -83,9 +83,14 @@ class ColumnSpecification final { public: ColumnSpecification(std::string *name, meta::cpp2::PropertyType type, - ColumnProperties *properties, - int16_t typeLen = 0) - : name_(name), type_(type), properties_(DCHECK_NOTNULL(properties)), typeLen_(typeLen) {} + ColumnProperties *properties = nullptr, + int16_t typeLen = 0, + meta::cpp2::GeoShape geoShape = meta::cpp2::GeoShape::ANY) + : name_(name), + type_(type), + properties_(DCHECK_NOTNULL(properties)), + typeLen_(typeLen), + geoShape_(geoShape) {} meta::cpp2::PropertyType type() const { return type_; } @@ -93,6 +98,8 @@ class ColumnSpecification final { int16_t typeLen() const { return typeLen_; } + meta::cpp2::GeoShape geoShape() const { return geoShape_; } + auto &properties() const { return properties_; } std::string toString() const; @@ -100,8 +107,9 @@ class ColumnSpecification final { private: std::unique_ptr name_; meta::cpp2::PropertyType type_; - std::unique_ptr properties_{nullptr}; - int16_t typeLen_{0}; + std::unique_ptr properties_; + int16_t typeLen_; + meta::cpp2::GeoShape geoShape_; }; class ColumnSpecificationList final { diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 282de9d9b8f..a02699a293c 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -60,6 +60,7 @@ static constexpr size_t kCommentLengthLimit = 256; int64_t intval; double doubleval; std::string *strval; + nebula::meta::cpp2::GeoShape geo_shape; nebula::meta::cpp2::ColumnTypeDef *type; nebula::Expression *expr; nebula::Sentence *sentence; @@ -155,7 +156,7 @@ static constexpr size_t kCommentLengthLimit = 256; // Expression related memory will be managed by object pool %destructor {} %destructor {} -%destructor {} +%destructor {} %destructor { delete $$; } <*> /* keywords */ @@ -200,6 +201,7 @@ static constexpr size_t kCommentLengthLimit = 256; %token KW_REDUCE %token KW_SESSIONS KW_SESSION %token KW_KILL KW_QUERY KW_QUERIES KW_TOP +%token KW_GEOGRAPHY KW_POINT KW_LINESTRING KW_POLYGON /* symbols */ %token L_PAREN R_PAREN L_BRACKET R_BRACKET L_BRACE R_BRACE COMMA @@ -242,6 +244,7 @@ static constexpr size_t kCommentLengthLimit = 256; %type constant_expression %type query_unique_identifier_value %type argument_list opt_argument_list +%type geo_shape_type %type type_spec %type step_clause %type from_clause @@ -527,6 +530,9 @@ unreserved_keyword | KW_QUERY { $$ = new std::string("query"); } | KW_KILL { $$ = new std::string("kill"); } | KW_TOP { $$ = new std::string("top"); } + | KW_POINT { $$ = new std::string("point"); } + | KW_LINESTRING { $$ = new std::string("linestring"); } + | KW_POLYGON { $$ = new std::string("polygon"); } ; expression @@ -1075,6 +1081,18 @@ argument_list } ; +geo_shape_type + : KW_POINT { + $$ = meta::cpp2::GeoShape::POINT; + } + | KW_LINESTRING { + $$ = meta::cpp2::GeoShape::LINESTRING; + } + | KW_POLYGON { + $$ = meta::cpp2::GeoShape::POLYGON; + } + ; + type_spec : KW_BOOL { $$ = new meta::cpp2::ColumnTypeDef(); @@ -1136,6 +1154,16 @@ type_spec $$ = new meta::cpp2::ColumnTypeDef(); $$->set_type(meta::cpp2::PropertyType::DATETIME); } + | KW_GEOGRAPHY { + $$ = new meta::cpp2::ColumnTypeDef(); + $$->set_type(meta::cpp2::PropertyType::GEOGRAPHY); + $$->set_geo_shape(meta::cpp2::GeoShape::ANY); + } + | KW_GEOGRAPHY L_PAREN geo_shape_type R_PAREN { + $$ = new meta::cpp2::ColumnTypeDef(); + $$->set_type(meta::cpp2::PropertyType::GEOGRAPHY); + $$->set_geo_shape($3); + } ; @@ -2286,11 +2314,11 @@ column_spec_list column_spec : name_label type_spec { - $$ = new ColumnSpecification($1, $2->type, new ColumnProperties(), $2->type_length_ref().value_or(0)); + $$ = new ColumnSpecification($1, $2->type, new ColumnProperties(), $2->type_length_ref().value_or(0), $2->geo_shape_ref().value_or(meta::cpp2::GeoShape::ANY)); delete $2; } | name_label type_spec column_properties { - $$ = new ColumnSpecification($1, $2->type, $3, $2->type_length_ref().value_or(0)); + $$ = new ColumnSpecification($1, $2->type, $3, $2->type_length_ref().value_or(0), $2->geo_shape_ref().value_or(meta::cpp2::GeoShape::ANY)); delete $2; } ; diff --git a/src/parser/scanner.lex b/src/parser/scanner.lex index 5fc535a20cb..6c316e4b2bf 100644 --- a/src/parser/scanner.lex +++ b/src/parser/scanner.lex @@ -255,6 +255,10 @@ IP_OCTET ([0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5]) "QUERY" { return TokenType::KW_QUERY; } "KILL" { return TokenType::KW_KILL; } "TOP" { return TokenType::KW_TOP; } +"GEOGRAPHY" { return TokenType::KW_GEOGRAPHY; } +"POINT" { return TokenType::KW_POINT; } +"LINESTRING" { return TokenType::KW_LINESTRING; } +"POLYGON" { return TokenType::KW_POLYGON; } "TRUE" { yylval->boolval = true; return TokenType::BOOL; } "FALSE" { yylval->boolval = false; return TokenType::BOOL; } diff --git a/src/parser/test/CMakeLists.txt b/src/parser/test/CMakeLists.txt index 260722922d4..c90ee40a187 100644 --- a/src/parser/test/CMakeLists.txt +++ b/src/parser/test/CMakeLists.txt @@ -17,6 +17,7 @@ set(PARSER_TEST_LIBS $ $ $ + $ $ $ $ diff --git a/src/storage/test/CMakeLists.txt b/src/storage/test/CMakeLists.txt index 86cc7076896..33dee520e63 100644 --- a/src/storage/test/CMakeLists.txt +++ b/src/storage/test/CMakeLists.txt @@ -41,6 +41,7 @@ set(storage_test_deps $ $ $ + $ $ $ $ diff --git a/src/tools/db-dump/CMakeLists.txt b/src/tools/db-dump/CMakeLists.txt index fa4587a52f3..d48e0f73575 100644 --- a/src/tools/db-dump/CMakeLists.txt +++ b/src/tools/db-dump/CMakeLists.txt @@ -38,6 +38,7 @@ set(tools_test_deps $ $ $ + $ $ $ $ diff --git a/src/tools/db-upgrade/CMakeLists.txt b/src/tools/db-upgrade/CMakeLists.txt index ed036caea97..7b3670d20ee 100644 --- a/src/tools/db-upgrade/CMakeLists.txt +++ b/src/tools/db-upgrade/CMakeLists.txt @@ -46,6 +46,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ diff --git a/src/tools/meta-dump/CMakeLists.txt b/src/tools/meta-dump/CMakeLists.txt index d6aad52c963..663e4686366 100644 --- a/src/tools/meta-dump/CMakeLists.txt +++ b/src/tools/meta-dump/CMakeLists.txt @@ -43,6 +43,7 @@ nebula_add_executable( $ $ $ + $ $ $ $ diff --git a/src/tools/simple-kv-verify/CMakeLists.txt b/src/tools/simple-kv-verify/CMakeLists.txt index 9d988f9cd18..75c8b9cfb88 100644 --- a/src/tools/simple-kv-verify/CMakeLists.txt +++ b/src/tools/simple-kv-verify/CMakeLists.txt @@ -41,6 +41,7 @@ nebula_add_executable( $ $ $ + $ $ $ LIBRARIES diff --git a/src/tools/storage-perf/CMakeLists.txt b/src/tools/storage-perf/CMakeLists.txt index 94e888f12bd..5a4400210d2 100644 --- a/src/tools/storage-perf/CMakeLists.txt +++ b/src/tools/storage-perf/CMakeLists.txt @@ -38,6 +38,7 @@ set(perf_test_deps $ $ $ + $ $ $ $ diff --git a/src/webservice/WebService.h b/src/webservice/WebService.h index 518ac61a0f7..e4a373689f6 100644 --- a/src/webservice/WebService.h +++ b/src/webservice/WebService.h @@ -33,7 +33,7 @@ class WebService final { explicit WebService(const std::string& name = ""); ~WebService(); - MUST_USE_RESULT web::Router& router() { + NG_MUST_USE_RESULT web::Router& router() { CHECK(!started_) << "Don't add routes after starting web server!"; return *router_; } @@ -42,7 +42,7 @@ class WebService final { // Two ports would be bound, one for HTTP, another one for HTTP2. // If FLAGS_ws_http_port or FLAGS_ws_h2_port is zero, an ephemeral port // would be assigned and set back to the gflag, respectively. - MUST_USE_RESULT Status start(); + NG_MUST_USE_RESULT Status start(); // Check whether web service is started bool started() const { return started_; } From 028eb905c1231009c7f75cae998f298a5222803f Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 28 Sep 2021 10:14:23 +0800 Subject: [PATCH 2/9] add test --- src/common/datatypes/test/CMakeLists.txt | 13 ++ src/common/datatypes/test/GeographyTest.cpp | 71 ++++++++++ src/common/geo/io/wkb/ByteOrder.h | 2 + src/common/geo/io/wkb/WKBReader.cpp | 149 +++++++++++++++++++- src/common/geo/io/wkb/WKBReader.h | 139 ++---------------- src/common/geo/io/wkb/WKBWriter.cpp | 77 +++++++++- src/common/geo/io/wkb/WKBWriter.h | 72 ++-------- src/common/geo/io/wkt/WKTReader.cpp | 2 - src/common/geo/io/wkt/WKTWriter.cpp | 81 ++++++++++- src/common/geo/io/wkt/WKTWriter.h | 76 +--------- 10 files changed, 412 insertions(+), 270 deletions(-) create mode 100644 src/common/datatypes/test/GeographyTest.cpp diff --git a/src/common/datatypes/test/CMakeLists.txt b/src/common/datatypes/test/CMakeLists.txt index 4c5bcce7e7c..00c482a57f8 100644 --- a/src/common/datatypes/test/CMakeLists.txt +++ b/src/common/datatypes/test/CMakeLists.txt @@ -84,6 +84,19 @@ nebula_add_test( gtest ) +nebula_add_test( + NAME + geography_test + SOURCES + GeographyTest.cpp + OBJECTS + $ + $ + $ + LIBRARIES + gtest +) + nebula_add_test( NAME value_to_json_test diff --git a/src/common/datatypes/test/GeographyTest.cpp b/src/common/datatypes/test/GeographyTest.cpp new file mode 100644 index 00000000000..ee180015014 --- /dev/null +++ b/src/common/datatypes/test/GeographyTest.cpp @@ -0,0 +1,71 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include +#include + +#include "common/base/Base.h" +#include "common/datatypes/Geography.h" + +namespace nebula { + +TEST(Geography, shape) { + { + std::string wkt = "POINT(3 7)"; + auto gRet = Geography::fromWKT(wkt); + ASSERT_TRUE(gRet.ok()); + auto g = std::move(gRet.value()); + EXPECT_EQ(GeoShape::POINT, g->shape()); + } + { + std::string wkt = "LINESTRING(28.4 79.20,134.25 -28.34)"; + auto gRet = Geography::fromWKT(wkt); + ASSERT_TRUE(gRet.ok()); + auto g = std::move(gRet.value()); + EXPECT_EQ(GeoShape::LINESTRING, g->shape()); + } + { + std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; + auto gRet = Geography::fromWKT(wkt); + ASSERT_TRUE(gRet.ok()); + auto g = std::move(gRet.value()); + EXPECT_EQ(GeoShape::POLYGON, g->shape()); + } +} + +TEST(Geography, asWKT) { + { + std::string wkt = "POINT(3 7)"; + auto gRet = Geography::fromWKT(wkt); + ASSERT_TRUE(gRet.ok()); + auto g = std::move(gRet.value()); + EXPECT_EQ(wkt, g->asWKT()); + } + { + std::string wkt = "LINESTRING(28.4 79.2,134.25 -28.34)"; + auto gRet = Geography::fromWKT(wkt); + ASSERT_TRUE(gRet.ok()); + auto g = std::move(gRet.value()); + EXPECT_EQ(wkt, g->asWKT()); + } + { + std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; + auto gRet = Geography::fromWKT(wkt); + ASSERT_TRUE(gRet.ok()); + auto g = std::move(gRet.value()); + EXPECT_EQ(wkt, g->asWKT()); + } +} + +} // namespace nebula + +int main(int argc, char** argv) { + testing::InitGoogleTest(&argc, argv); + folly::init(&argc, &argv, true); + google::SetStderrLogging(google::INFO); + + return RUN_ALL_TESTS(); +} diff --git a/src/common/geo/io/wkb/ByteOrder.h b/src/common/geo/io/wkb/ByteOrder.h index 128e7dca829..668461d72bf 100644 --- a/src/common/geo/io/wkb/ByteOrder.h +++ b/src/common/geo/io/wkb/ByteOrder.h @@ -6,6 +6,8 @@ #pragma once +#include "common/base/Base.h" + namespace nebula { enum class ByteOrder : uint8_t { diff --git a/src/common/geo/io/wkb/WKBReader.cpp b/src/common/geo/io/wkb/WKBReader.cpp index b86d3147613..c4afbc55473 100644 --- a/src/common/geo/io/wkb/WKBReader.cpp +++ b/src/common/geo/io/wkb/WKBReader.cpp @@ -6,6 +6,151 @@ #include "common/geo/io/wkb/WKBReader.h" -#include "common/base/Base.h" +namespace nebula { -namespace nebula {} // namespace nebula +StatusOr> WKBReader::read(std::string wkb) const { + uint8_t *beg = reinterpret_cast(wkb.data()); + uint8_t *end = beg + wkb.size(); + return read(beg, end); +} + +StatusOr> WKBReader::read(uint8_t *&beg, uint8_t *end) const { + auto byteOrderRet = readByteOrder(beg, end); + NG_RETURN_IF_ERROR(byteOrderRet); + ByteOrder byteOrder = byteOrderRet.value(); + + auto shapeTypeRet = readShapeType(beg, end, byteOrder); + NG_RETURN_IF_ERROR(shapeTypeRet); + GeoShape shapeType = shapeTypeRet.value(); + + switch (shapeType) { + case GeoShape::POINT: { + auto coordRet = readCoordinate(beg, end, byteOrder); + NG_RETURN_IF_ERROR(coordRet); + Coordinate coord = coordRet.value(); + return std::make_unique(coord); + } + case GeoShape::LINESTRING: { + auto numPointsRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(numPointsRet); + uint32_t numPoints = numPointsRet.value(); + auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); + NG_RETURN_IF_ERROR(coordListRet); + std::vector coordList = coordListRet.value(); + return std::make_unique(coordList); + } + case GeoShape::POLYGON: { + auto numRingsRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(numRingsRet); + uint32_t numRings = numRingsRet.value(); + auto coordListListRet = readCoordinateListList(beg, end, byteOrder, numRings); + NG_RETURN_IF_ERROR(coordListListRet); + std::vector> coordListList = coordListListRet.value(); + return std::make_unique(coordListList); + } + default: + LOG(FATAL) + << "Geography shapes other than Point/LineString/Polygon are not currently supported"; + return Status::Error( + "Geography shapes other than Point/LineString/Polygon are not currently supported"); + } +} + +StatusOr WKBReader::readByteOrder(uint8_t *&beg, uint8_t *end) const { + auto vRet = readUint8(beg, end); + NG_RETURN_IF_ERROR(vRet); + uint8_t v = vRet.value(); + if (v != 0 && v != 1) { + return Status::Error("Unknown byte order"); + } + ByteOrder byteOrder = static_cast(v); + return byteOrder; +} + +StatusOr WKBReader::readShapeType(uint8_t *&beg, + uint8_t *end, + ByteOrder byteOrder) const { + auto vRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(vRet); + uint32_t v = vRet.value(); + if (v != 1 && v != 2 && v != 3) { + return Status::Error("Unknown shape type"); + } + GeoShape shapeType = static_cast(v); + return shapeType; +} + +StatusOr WKBReader::readCoordinate(uint8_t *&beg, + uint8_t *end, + ByteOrder byteOrder) const { + auto xRet = readDouble(beg, end, byteOrder); + NG_RETURN_IF_ERROR(xRet); + double x = xRet.value(); + auto yRet = readDouble(beg, end, byteOrder); + NG_RETURN_IF_ERROR(yRet); + double y = yRet.value(); + return Coordinate(x, y); +} + +StatusOr> WKBReader::readCoordinateList(uint8_t *&beg, + uint8_t *end, + ByteOrder byteOrder, + uint32_t num) const { + std::vector coordList; + coordList.reserve(num); + for (size_t i = 0; i < num; ++i) { + auto coordRet = readCoordinate(beg, end, byteOrder); + NG_RETURN_IF_ERROR(coordRet); + Coordinate coord = coordRet.value(); + coordList.emplace_back(coord); + } + return coordList; +} + +StatusOr>> WKBReader::readCoordinateListList( + uint8_t *&beg, uint8_t *end, ByteOrder byteOrder, uint32_t num) const { + std::vector> coordListList; + coordListList.reserve(num); + for (size_t i = 0; i < num; ++i) { + auto numPointsRet = readUint32(beg, end, byteOrder); + NG_RETURN_IF_ERROR(numPointsRet); + uint32_t numPoints = numPointsRet.value(); + auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); + NG_RETURN_IF_ERROR(coordListRet); + std::vector coordList = coordListRet.value(); + coordListList.emplace_back(coordList); + } + return coordListList; +} + +StatusOr WKBReader::readUint8(uint8_t *&beg, uint8_t *end) const { + auto requiredSize = static_cast(sizeof(uint8_t)); + if (end - beg < requiredSize) { + return Status::Error("Unable to parse uint8_t"); + } + uint8_t v = *beg; + beg += requiredSize; + return v; +} + +StatusOr WKBReader::readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { + auto requiredSize = static_cast(sizeof(uint32_t)); + if (end - beg < requiredSize) { + return Status::Error("Unable to parse uint32_t"); + } + uint32_t v = ByteOrderData::getUint32(beg, byteOrder); + beg += requiredSize; + return v; +} + +StatusOr WKBReader::readDouble(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { + auto requiredSize = static_cast(sizeof(double)); + if (end - beg < requiredSize) { + return Status::Error("Unable to parse double"); + } + double v = ByteOrderData::getDouble(beg, byteOrder); + beg += requiredSize; + return v; +} + +} // namespace nebula diff --git a/src/common/geo/io/wkb/WKBReader.h b/src/common/geo/io/wkb/WKBReader.h index 4bcb252efd2..a6012ff02fe 100644 --- a/src/common/geo/io/wkb/WKBReader.h +++ b/src/common/geo/io/wkb/WKBReader.h @@ -6,6 +6,7 @@ #pragma once +#include "common/base/Base.h" #include "common/base/StatusOr.h" #include "common/geo/io/Geometry.h" #include "common/geo/io/wkb/ByteOrder.h" @@ -19,148 +20,30 @@ class WKBReader { ~WKBReader() {} // TODO(jie) Check the validity of geometry when reading the wkb - StatusOr> read(std::string wkb) const { - uint8_t *beg = reinterpret_cast(wkb.data()); - uint8_t *end = beg + wkb.size(); - return read(beg, end); - } + StatusOr> read(std::string wkb) const; - StatusOr> read(uint8_t *&beg, uint8_t *end) const { - auto byteOrderRet = readByteOrder(beg, end); - NG_RETURN_IF_ERROR(byteOrderRet); - ByteOrder byteOrder = byteOrderRet.value(); + StatusOr> read(uint8_t *&beg, uint8_t *end) const; - auto shapeTypeRet = readShapeType(beg, end, byteOrder); - NG_RETURN_IF_ERROR(shapeTypeRet); - GeoShape shapeType = shapeTypeRet.value(); + StatusOr readByteOrder(uint8_t *&beg, uint8_t *end) const; - switch (shapeType) { - case GeoShape::POINT: { - auto coordRet = readCoordinate(beg, end, byteOrder); - NG_RETURN_IF_ERROR(coordRet); - Coordinate coord = coordRet.value(); - return std::make_unique(coord); - } - case GeoShape::LINESTRING: { - auto numPointsRet = readUint32(beg, end, byteOrder); - NG_RETURN_IF_ERROR(numPointsRet); - uint32_t numPoints = numPointsRet.value(); - auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); - NG_RETURN_IF_ERROR(coordListRet); - std::vector coordList = coordListRet.value(); - return std::make_unique(coordList); - } - case GeoShape::POLYGON: { - auto numRingsRet = readUint32(beg, end, byteOrder); - NG_RETURN_IF_ERROR(numRingsRet); - uint32_t numRings = numRingsRet.value(); - auto coordListListRet = readCoordinateListList(beg, end, byteOrder, numRings); - NG_RETURN_IF_ERROR(coordListListRet); - std::vector> coordListList = coordListListRet.value(); - return std::make_unique(coordListList); - } - default: - LOG(FATAL) - << "Geography shapes other than Point/LineString/Polygon are not currently supported"; - return Status::Error( - "Geography shapes other than Point/LineString/Polygon are not currently supported"); - } - } + StatusOr readShapeType(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; - StatusOr readByteOrder(uint8_t *&beg, uint8_t *end) const { - auto vRet = readUint8(beg, end); - NG_RETURN_IF_ERROR(vRet); - uint8_t v = vRet.value(); - if (v != 0 && v != 1) { - return Status::Error("Unknown byte order"); - } - ByteOrder byteOrder = static_cast(v); - return byteOrder; - } - - StatusOr readShapeType(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { - auto vRet = readUint32(beg, end, byteOrder); - NG_RETURN_IF_ERROR(vRet); - uint32_t v = vRet.value(); - if (v != 1 && v != 2 && v != 3) { - return Status::Error("Unknown shape type"); - } - GeoShape shapeType = static_cast(v); - return shapeType; - } - - StatusOr readCoordinate(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { - auto xRet = readDouble(beg, end, byteOrder); - NG_RETURN_IF_ERROR(xRet); - double x = xRet.value(); - auto yRet = readDouble(beg, end, byteOrder); - NG_RETURN_IF_ERROR(yRet); - double y = yRet.value(); - return Coordinate(x, y); - } + StatusOr readCoordinate(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; StatusOr> readCoordinateList(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder, - uint32_t num) const { - std::vector coordList; - coordList.reserve(num); - for (size_t i = 0; i < num; ++i) { - auto coordRet = readCoordinate(beg, end, byteOrder); - NG_RETURN_IF_ERROR(coordRet); - Coordinate coord = coordRet.value(); - coordList.emplace_back(coord); - } - return coordList; - } + uint32_t num) const; StatusOr>> readCoordinateListList(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder, - uint32_t num) const { - std::vector> coordListList; - coordListList.reserve(num); - for (size_t i = 0; i < num; ++i) { - auto numPointsRet = readUint32(beg, end, byteOrder); - NG_RETURN_IF_ERROR(numPointsRet); - uint32_t numPoints = numPointsRet.value(); - auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); - NG_RETURN_IF_ERROR(coordListRet); - std::vector coordList = coordListRet.value(); - coordListList.emplace_back(coordList); - } - return coordListList; - } - - StatusOr readUint8(uint8_t *&beg, uint8_t *end) const { - auto requiredSize = static_cast(sizeof(uint8_t)); - if (end - beg < requiredSize) { - return Status::Error("Unable to parse uint8_t"); - } - uint8_t v = *beg; - beg += requiredSize; - return v; - } + uint32_t num) const; + StatusOr readUint8(uint8_t *&beg, uint8_t *end) const; - StatusOr readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { - auto requiredSize = static_cast(sizeof(uint32_t)); - if (end - beg < requiredSize) { - return Status::Error("Unable to parse uint32_t"); - } - uint32_t v = ByteOrderData::getUint32(beg, byteOrder); - beg += requiredSize; - return v; - } + StatusOr readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; - StatusOr readDouble(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { - auto requiredSize = static_cast(sizeof(double)); - if (end - beg < requiredSize) { - return Status::Error("Unable to parse double"); - } - double v = ByteOrderData::getDouble(beg, byteOrder); - beg += requiredSize; - return v; - } + StatusOr readDouble(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; }; } // namespace nebula diff --git a/src/common/geo/io/wkb/WKBWriter.cpp b/src/common/geo/io/wkb/WKBWriter.cpp index 172a2679091..f5f9878e383 100644 --- a/src/common/geo/io/wkb/WKBWriter.cpp +++ b/src/common/geo/io/wkb/WKBWriter.cpp @@ -6,6 +6,79 @@ #include "common/geo/io/wkb/WKBWriter.h" -#include "common/base/Base.h" +namespace nebula { -namespace nebula {} // namespace nebula +std::string WKBWriter::write(const Geometry* geom) const { + std::string wkb = ""; + + uint8_t byteOrder = + static_cast>(ByteOrderData::getMachineByteOrder()); + writeUint8(wkb, byteOrder); + + auto shape = geom->shape(); + uint32_t shapeType = folly::to(shape); + writeUint32(wkb, shapeType); + switch (shape) { + case GeoShape::POINT: { + const Point* point = static_cast(geom); + writeCoordinate(wkb, point->coord); + return wkb; + } + case GeoShape::LINESTRING: { + const LineString* line = static_cast(geom); + auto coordList = line->coordList; + uint32_t numPoints = coordList.size(); + writeUint32(wkb, numPoints); + writeCoordinateList(wkb, coordList); + return wkb; + } + case GeoShape::POLYGON: { + const Polygon* polygon = static_cast(geom); + auto coordListList = polygon->coordListList; + uint32_t numRings = coordListList.size(); + writeUint32(wkb, numRings); + writeCoordinateListList(wkb, coordListList); + return wkb; + } + default: + LOG(FATAL) + << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; + return ""; + } +} + +void WKBWriter::writeCoordinate(std::string& wkb, const Coordinate& coord) const { + writeDouble(wkb, coord.x); + writeDouble(wkb, coord.y); +} + +void WKBWriter::writeCoordinateList(std::string& wkb, + const std::vector& coordList) const { + for (size_t i = 0; i < coordList.size(); ++i) { + writeCoordinate(wkb, coordList[i]); + } +} + +void WKBWriter::writeCoordinateListList( + std::string& wkb, const std::vector>& coordListList) const { + for (size_t i = 0; i < coordListList.size(); ++i) { + const auto& coordList = coordListList[i]; + uint32_t numPoints = coordList.size(); + writeUint32(wkb, numPoints); + writeCoordinateList(wkb, coordList); + } +} + +void WKBWriter::writeUint8(std::string& wkb, uint8_t v) const { + wkb.append(reinterpret_cast(&v), sizeof(v)); +} + +void WKBWriter::writeUint32(std::string& wkb, uint32_t v) const { + wkb.append(reinterpret_cast(&v), sizeof(v)); +} + +void WKBWriter::writeDouble(std::string& wkb, double v) const { + wkb.append(reinterpret_cast(&v), sizeof(v)); +} + +} // namespace nebula diff --git a/src/common/geo/io/wkb/WKBWriter.h b/src/common/geo/io/wkb/WKBWriter.h index 7077453b6f7..01d1c3558b4 100644 --- a/src/common/geo/io/wkb/WKBWriter.h +++ b/src/common/geo/io/wkb/WKBWriter.h @@ -6,6 +6,7 @@ #pragma once +#include "common/base/Base.h" #include "common/geo/io/Geometry.h" #include "common/geo/io/wkb/ByteOrder.h" @@ -17,77 +18,20 @@ class WKBWriter { ~WKBWriter() {} - std::string write(const Geometry* geom) const { - std::string wkb = ""; + std::string write(const Geometry* geom) const; - uint8_t byteOrder = - static_cast>(ByteOrderData::getMachineByteOrder()); - writeUint8(wkb, byteOrder); + void writeCoordinate(std::string& wkb, const Coordinate& coord) const; - auto shape = geom->shape(); - uint32_t shapeType = folly::to(shape); - writeUint32(wkb, shapeType); - switch (shape) { - case GeoShape::POINT: { - const Point* point = static_cast(geom); - writeCoordinate(wkb, point->coord); - return wkb; - } - case GeoShape::LINESTRING: { - const LineString* line = static_cast(geom); - auto coordList = line->coordList; - uint32_t numPoints = coordList.size(); - writeUint32(wkb, numPoints); - writeCoordinateList(wkb, coordList); - return wkb; - } - case GeoShape::POLYGON: { - const Polygon* polygon = static_cast(geom); - auto coordListList = polygon->coordListList; - uint32_t numRings = coordListList.size(); - writeUint32(wkb, numRings); - writeCoordinateListList(wkb, coordListList); - return wkb; - } - default: - LOG(FATAL) - << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; - return ""; - } - } - - void writeCoordinate(std::string& wkb, const Coordinate& coord) const { - writeDouble(wkb, coord.x); - writeDouble(wkb, coord.y); - } - - void writeCoordinateList(std::string& wkb, const std::vector& coordList) const { - for (size_t i = 0; i < coordList.size(); ++i) { - writeCoordinate(wkb, coordList[i]); - } - } + void writeCoordinateList(std::string& wkb, const std::vector& coordList) const; void writeCoordinateListList(std::string& wkb, - const std::vector>& coordListList) const { - for (size_t i = 0; i < coordListList.size(); ++i) { - const auto& coordList = coordListList[i]; - uint32_t numPoints = coordList.size(); - writeUint32(wkb, numPoints); - writeCoordinateList(wkb, coordList); - } - } + const std::vector>& coordListList) const; - void writeUint8(std::string& wkb, uint8_t v) const { - wkb.append(reinterpret_cast(&v), sizeof(v)); - } + void writeUint8(std::string& wkb, uint8_t v) const; - void writeUint32(std::string& wkb, uint32_t v) const { - wkb.append(reinterpret_cast(&v), sizeof(v)); - } + void writeUint32(std::string& wkb, uint32_t v) const; - void writeDouble(std::string& wkb, double v) const { - wkb.append(reinterpret_cast(&v), sizeof(v)); - } + void writeDouble(std::string& wkb, double v) const; }; } // namespace nebula diff --git a/src/common/geo/io/wkt/WKTReader.cpp b/src/common/geo/io/wkt/WKTReader.cpp index 82e360219ef..6869d86a408 100644 --- a/src/common/geo/io/wkt/WKTReader.cpp +++ b/src/common/geo/io/wkt/WKTReader.cpp @@ -6,6 +6,4 @@ #include "common/geo/io/wkt/WKTReader.h" -#include "common/base/Base.h" - namespace nebula {} // namespace nebula diff --git a/src/common/geo/io/wkt/WKTWriter.cpp b/src/common/geo/io/wkt/WKTWriter.cpp index e1462561482..52eed139870 100644 --- a/src/common/geo/io/wkt/WKTWriter.cpp +++ b/src/common/geo/io/wkt/WKTWriter.cpp @@ -6,6 +6,83 @@ #include "common/geo/io/wkt/WKTWriter.h" -#include "common/base/Base.h" +namespace nebula { -namespace nebula {} // namespace nebula +std::string WKTWriter::write(const Geometry* geom) const { + std::string wkt = ""; + + auto shape = geom->shape(); + switch (shape) { + case GeoShape::POINT: { + wkt.append("POINT"); + const Point* point = static_cast(geom); + wkt.append("("); + writeCoordinate(wkt, point->coord); + wkt.append(")"); + return wkt; + } + case GeoShape::LINESTRING: { + wkt.append("LINESTRING"); + const LineString* line = static_cast(geom); + auto coordList = line->coordList; + uint32_t numPoints = coordList.size(); + UNUSED(numPoints); + wkt.append("("); + writeCoordinateList(wkt, coordList); + wkt.append(")"); + return wkt; + } + case GeoShape::POLYGON: { + wkt.append("POLYGON"); + const Polygon* polygon = static_cast(geom); + auto coordListList = polygon->coordListList; + uint32_t numRings = coordListList.size(); + UNUSED(numRings); + wkt.append("("); + writeCoordinateListList(wkt, coordListList); + wkt.append(")"); + return wkt; + } + default: + LOG(FATAL) + << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; + return ""; + } +} + +void WKTWriter::writeCoordinate(std::string& wkt, const Coordinate& coord) const { + writeDouble(wkt, coord.x); + wkt.append(" "); + writeDouble(wkt, coord.y); +} + +void WKTWriter::writeCoordinateList(std::string& wkt, + const std::vector& coordList) const { + for (size_t i = 0; i < coordList.size(); ++i) { + writeCoordinate(wkt, coordList[i]); + wkt.append(","); + } + wkt.pop_back(); +} + +void WKTWriter::WKTWriter::writeCoordinateListList( + std::string& wkt, const std::vector>& coordListList) const { + for (size_t i = 0; i < coordListList.size(); ++i) { + const auto& coordList = coordListList[i]; + uint32_t numPoints = coordList.size(); + UNUSED(numPoints); + wkt.append("("); + writeCoordinateList(wkt, coordList); + wkt.append(")"); + wkt.append(","); + } + wkt.pop_back(); +} + +void WKTWriter::writeDouble(std::string& wkt, double v) const { + wkt.append(folly::to(v)); + LOG(INFO) << "writeDouble(wkt, v): " << wkt << ", " << v + << ", folly::to: " << folly::to(v); +} + +} // namespace nebula diff --git a/src/common/geo/io/wkt/WKTWriter.h b/src/common/geo/io/wkt/WKTWriter.h index 4e7764acbbb..3c2fe8dc258 100644 --- a/src/common/geo/io/wkt/WKTWriter.h +++ b/src/common/geo/io/wkt/WKTWriter.h @@ -6,6 +6,7 @@ #pragma once +#include "common/base/Base.h" #include "common/geo/io/Geometry.h" namespace nebula { @@ -16,81 +17,16 @@ class WKTWriter { ~WKTWriter() {} - std::string write(const Geometry* geom) const { - std::string wkt = ""; + std::string write(const Geometry* geom) const; - auto shape = geom->shape(); - switch (shape) { - case GeoShape::POINT: { - wkt.append("POINT"); - const Point* point = static_cast(geom); - wkt.append("("); - writeCoordinate(wkt, point->coord); - wkt.append(")"); - return wkt; - } - case GeoShape::LINESTRING: { - wkt.append("LINESTRING"); - const LineString* line = static_cast(geom); - auto coordList = line->coordList; - uint32_t numPoints = coordList.size(); - UNUSED(numPoints); - wkt.append("("); - writeCoordinateList(wkt, coordList); - wkt.append(")"); - return wkt; - } - case GeoShape::POLYGON: { - wkt.append("POLYGON"); - const Polygon* polygon = static_cast(geom); - auto coordListList = polygon->coordListList; - uint32_t numRings = coordListList.size(); - UNUSED(numRings); - wkt.append("("); - writeCoordinateListList(wkt, coordListList); - wkt.append(")"); - return wkt; - } - default: - LOG(FATAL) - << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; - return ""; - } - } + void writeCoordinate(std::string& wkt, const Coordinate& coord) const; - void writeCoordinate(std::string& wkt, const Coordinate& coord) const { - writeDouble(wkt, coord.x); - wkt.append(" "); - writeDouble(wkt, coord.y); - } - - void writeCoordinateList(std::string& wkt, const std::vector& coordList) const { - for (size_t i = 0; i < coordList.size(); ++i) { - writeCoordinate(wkt, coordList[i]); - wkt.append(","); - } - wkt.pop_back(); - } + void writeCoordinateList(std::string& wkt, const std::vector& coordList) const; void writeCoordinateListList(std::string& wkt, - const std::vector>& coordListList) const { - for (size_t i = 0; i < coordListList.size(); ++i) { - const auto& coordList = coordListList[i]; - uint32_t numPoints = coordList.size(); - UNUSED(numPoints); - wkt.append("("); - writeCoordinateList(wkt, coordList); - wkt.append(")"); - wkt.append(","); - } - wkt.pop_back(); - } + const std::vector>& coordListList) const; - void writeDouble(std::string& wkt, double v) const { - wkt.append(folly::to(v)); - LOG(INFO) << "writeDouble(wkt, v): " << wkt << ", " << v - << ", folly::to: " << folly::to(v); - } + void writeDouble(std::string& wkt, double v) const; }; } // namespace nebula From b5c988f201a9a4a36904d7ec01a4f0ad2e34b532 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 28 Sep 2021 19:44:25 +0800 Subject: [PATCH 3/9] address the comment --- src/codec/RowReaderV2.cpp | 2 +- src/common/datatypes/Geography.cpp | 47 ++++++++---- src/common/datatypes/Geography.h | 12 +-- src/common/datatypes/Value.cpp | 5 +- src/common/datatypes/test/GeographyTest.cpp | 12 ++- src/common/geo/GeoShape.h | 1 + src/common/geo/GeoUtils.h | 65 ++++++++++------- src/common/geo/io/CMakeLists.txt | 3 - src/common/geo/io/Geometry.h | 77 ++++++++++++++------ src/common/geo/io/wkb/WKBReader.cpp | 40 +++++----- src/common/geo/io/wkb/WKBReader.h | 28 ++++--- src/common/geo/io/wkb/WKBWriter.cpp | 17 +++-- src/common/geo/io/wkb/WKBWriter.h | 2 +- src/common/geo/io/wkt/WKTWriter.cpp | 20 +++-- src/common/geo/io/wkt/WKTWriter.h | 2 +- src/common/geo/io/wkt/test/WKTParserTest.cpp | 6 +- src/common/geo/io/wkt/wkt_parser.yy | 9 +-- 17 files changed, 205 insertions(+), 143 deletions(-) diff --git a/src/codec/RowReaderV2.cpp b/src/codec/RowReaderV2.cpp index 8d1a3324fea..0e8bb247c75 100644 --- a/src/codec/RowReaderV2.cpp +++ b/src/codec/RowReaderV2.cpp @@ -177,7 +177,7 @@ Value RowReaderV2::getValueByIndex(const int64_t index) const noexcept { } case meta::cpp2::PropertyType::GEOGRAPHY: { // TODO(jie) - return Geography(""); + return Geography(); } case meta::cpp2::PropertyType::UNKNOWN: break; diff --git a/src/common/datatypes/Geography.cpp b/src/common/datatypes/Geography.cpp index 1d30974cf55..661199a4519 100644 --- a/src/common/datatypes/Geography.cpp +++ b/src/common/datatypes/Geography.cpp @@ -23,40 +23,55 @@ StatusOr> Geography::fromWKT(const std::string& wkt) auto geomRet = WKTReader().read(wkt); NG_RETURN_IF_ERROR(geomRet); auto geom = std::move(geomRet).value(); - auto wkb = WKBWriter().write(geom.get()); + auto wkb = WKBWriter().write(*geom); return std::make_unique(wkb); } GeoShape Geography::shape() const { // TODO(jie) May store the shapetype as the data member of Geography is ok. - std::string wkbCopy = wkb; - uint8_t* beg = reinterpret_cast(wkbCopy.data()); - uint8_t* end = beg + wkbCopy.size(); + const uint8_t* beg = reinterpret_cast(wkb.data()); + const uint8_t* end = beg + wkb.size(); WKBReader reader; auto byteOrderRet = reader.readByteOrder(beg, end); - DCHECK(byteOrderRet.ok()); + if (!byteOrderRet.ok()) { + return GeoShape::UNKNOWN; + } ByteOrder byteOrder = byteOrderRet.value(); auto shapeTypeRet = reader.readShapeType(beg, end, byteOrder); - DCHECK(shapeTypeRet.ok()); + if (!shapeTypeRet.ok()) { + return GeoShape::UNKNOWN; + } return shapeTypeRet.value(); } -std::string Geography::asWKT() const { +std::unique_ptr Geography::asWKT() const { auto geomRet = WKBReader().read(wkb); - DCHECK(geomRet.ok()); - std::unique_ptr geom = std::move(geomRet).value(); - std::string wkt = WKTWriter().write(geom.get()); - LOG(INFO) << "Geography::asWKT(): " << wkt; - return wkt; + if (!geomRet.ok()) { + LOG(ERROR) << geomRet.status(); + return nullptr; + } + auto geom = geomRet.value(); + return std::make_unique(WKTWriter().write(geom)); } -std::string Geography::asWKB() const { return folly::hexlify(wkb); } +std::unique_ptr Geography::asWKBHex() const { + auto geomRet = WKBReader().read(wkb); + if (!geomRet.ok()) { + LOG(ERROR) << geomRet.status(); + return nullptr; + } + auto geom = geomRet.value(); + return std::make_unique(folly::hexlify(WKBWriter().write(geom))); +} std::unique_ptr Geography::asS2() const { auto geomRet = WKBReader().read(wkb); - DCHECK(geomRet.ok()); - std::unique_ptr geom = std::move(geomRet).value(); - return GeoUtils::s2RegionFromGeomtry(geom.get()); + if (!geomRet.ok()) { + LOG(ERROR) << geomRet.status(); + return nullptr; + } + auto geom = geomRet.value(); + return GeoUtils::s2RegionFromGeomtry(geom); } } // namespace nebula diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h index ed8aecd1a86..74a99641ad1 100644 --- a/src/common/datatypes/Geography.h +++ b/src/common/datatypes/Geography.h @@ -38,29 +38,29 @@ static const std::unordered_map kShapeTypeToS2Region = { */ // clang-format on -// Do not construct a S2 data when constructing Geography. It's expensive. +// Do not construct a S2 object when constructing Geography. It's expensive. // We just construct S2 when doing computation. struct Geography { std::string wkb; // TODO(jie) Is it better to store Geometry* or S2Region* here? Geography() = default; + explicit Geography(const std::string& bytes) { - // TODO(jie): Ensure the bytes is valid + // TODO(jie): Must ensure the bytes is valid wkb = bytes; - LOG(INFO) << "Geography.wkb: " << wkb << ", wkb.size(): " << wkb.size(); } static StatusOr> fromWKT(const std::string& wkt); GeoShape shape() const; - std::string asWKT() const; + std::unique_ptr asWKT() const; - std::string asWKB() const; + std::unique_ptr asWKBHex() const; std::unique_ptr asS2() const; - std::string toString() const { return asWKT(); } + std::string toString() const { return wkb; } folly::dynamic toJson() const { return toString(); } diff --git a/src/common/datatypes/Value.cpp b/src/common/datatypes/Value.cpp index 3704ae4ff09..e4294a551ab 100644 --- a/src/common/datatypes/Value.cpp +++ b/src/common/datatypes/Value.cpp @@ -345,10 +345,7 @@ Value::Value(const DataSet& v) { Value::Value(DataSet&& v) { setG(std::make_unique(std::move(v))); } -Value::Value(const Geography& v) { - auto c = std::make_unique(v); - setGG(std::move(c)); -} +Value::Value(const Geography& v) { setGG(std::make_unique(v)); } Value::Value(Geography&& v) { setGG(std::make_unique(std::move(v))); } diff --git a/src/common/datatypes/test/GeographyTest.cpp b/src/common/datatypes/test/GeographyTest.cpp index ee180015014..faa29f9e383 100644 --- a/src/common/datatypes/test/GeographyTest.cpp +++ b/src/common/datatypes/test/GeographyTest.cpp @@ -42,21 +42,27 @@ TEST(Geography, asWKT) { auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = std::move(gRet.value()); - EXPECT_EQ(wkt, g->asWKT()); + auto got = g->asWKT(); + ASSERT_TRUE(!!got); + EXPECT_EQ(wkt, *got); } { std::string wkt = "LINESTRING(28.4 79.2,134.25 -28.34)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = std::move(gRet.value()); - EXPECT_EQ(wkt, g->asWKT()); + auto got = g->asWKT(); + ASSERT_TRUE(!!got); + EXPECT_EQ(wkt, *got); } { std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = std::move(gRet.value()); - EXPECT_EQ(wkt, g->asWKT()); + auto got = g->asWKT(); + ASSERT_TRUE(!!got); + EXPECT_EQ(wkt, *got); } } diff --git a/src/common/geo/GeoShape.h b/src/common/geo/GeoShape.h index 2cd28700ff5..6622433c408 100644 --- a/src/common/geo/GeoShape.h +++ b/src/common/geo/GeoShape.h @@ -9,6 +9,7 @@ namespace nebula { enum class GeoShape : uint32_t { + UNKNOWN = 0, // illegal POINT = 1, LINESTRING = 2, POLYGON = 3, diff --git a/src/common/geo/GeoUtils.h b/src/common/geo/GeoUtils.h index 8d76513057d..b3d0c083786 100644 --- a/src/common/geo/GeoUtils.h +++ b/src/common/geo/GeoUtils.h @@ -17,20 +17,26 @@ namespace nebula { class GeoUtils final { public: - static std::unique_ptr s2RegionFromGeomtry(const Geometry* geom) { - switch (geom->shape()) { + static std::unique_ptr s2RegionFromGeomtry(const Geometry& geom) { + switch (geom.shape()) { case GeoShape::POINT: { - const auto* point = static_cast(geom); - auto s2Point = s2PointFromCoordinate(point->coord); + const auto& point = geom.point(); + auto s2Point = s2PointFromCoordinate(point.coord); + if (!S2::IsUnitLength(s2Point)) { // S2LatLng::IsValid() + return nullptr; + } return std::make_unique(s2Point); } case GeoShape::LINESTRING: { - const auto* lineString = static_cast(geom); - auto coordList = lineString->coordList; - DCHECK_GE(coordList.size(), 2); + const auto& lineString = geom.lineString(); + auto coordList = lineString.coordList; + if (UNLIKELY(coordList.size() < 2)) { + LOG(ERROR) << "LineString must have at least 2 coordinates"; + return nullptr; + } removeAdjacentDuplicateCoordinates(coordList); if (coordList.size() < 2) { - LOG(INFO) + LOG(ERROR) << "Invalid LineString, adjacent coordinates must not be identical or antipodal."; return nullptr; } @@ -38,36 +44,45 @@ class GeoUtils final { auto s2Points = s2PointsFromCoordinateList(coordList); auto s2Polyline = std::make_unique(s2Points, S2Debug::DISABLE); if (!s2Polyline->IsValid()) { + LOG(ERROR) << "Invalid S2Polyline"; return nullptr; } return s2Polyline; } case GeoShape::POLYGON: { - const auto* polygon = static_cast(geom); - uint32_t numRings = polygon->numRings(); + const auto& polygon = geom.polygon(); + uint32_t numRings = polygon.numRings(); std::vector> s2Loops; s2Loops.reserve(numRings); for (size_t i = 0; i < numRings; ++i) { - auto coordList = polygon->coordListList[i]; - DCHECK_GE(coordList.size(), 4); - DCHECK(isLoopClosed(coordList)); + auto coordList = polygon.coordListList[i]; + if (UNLIKELY(coordList.size() < 4)) { + LOG(ERROR) << "Polygon's LinearRing must have at least 4 coordinates"; + return nullptr; + } + if (UNLIKELY(isLoopClosed(coordList))) { + return nullptr; + } removeAdjacentDuplicateCoordinates(coordList); if (coordList.size() < 4) { - LOG(INFO) + LOG(ERROR) << "Invalid linearRing in polygon, must have at least 4 distinct coordinates."; return nullptr; } coordList.pop_back(); // Remove redundant last coordinate auto s2Points = s2PointsFromCoordinateList(coordList); - auto* s2Loop = new S2Loop(std::move(s2Points), S2Debug::DISABLE); - if (!s2Loop->IsValid()) { + auto s2Loop = std::make_unique(std::move(s2Points), S2Debug::DISABLE); + if (!s2Loop->IsValid()) { // TODO(jie) May not need to check S2loop here, S2Polygon's + // IsValid() will check its loops + LOG(ERROR) << "Invalid S2Loop"; return nullptr; } s2Loop->Normalize(); // All loops must be oriented CCW(counterclockwise) for S2 - s2Loops.emplace_back(s2Loop); + s2Loops.emplace_back(std::move(s2Loop)); } auto s2Polygon = std::make_unique(std::move(s2Loops), S2Debug::DISABLE); if (!s2Polygon->IsValid()) { // Exterior loop must contain other interior loops + LOG(ERROR) << "Invalid S2Polygon"; return nullptr; } return s2Polygon; @@ -82,7 +97,6 @@ class GeoUtils final { static S2Point s2PointFromCoordinate(const Coordinate& coord) { auto latlng = S2LatLng::FromDegrees( coord.y, coord.x); // Note: S2Point requires latitude to be first, and longitude to be last - DCHECK(latlng.is_valid()); return latlng.ToPoint(); } @@ -100,7 +114,9 @@ class GeoUtils final { } static bool isLoopClosed(const std::vector& coordList) { - DCHECK_GE(coordList.size(), 2); + if (coordList.size() < 2) { + return false; + } return coordList.front() == coordList.back(); } @@ -109,15 +125,8 @@ class GeoUtils final { return; } - size_t i = 0, j = 1; - for (; j < coordList.size(); ++j) { - if (coordList[i] != coordList[j]) { - ++i; - if (i != j) { // i is always <= j - coordList[i] = coordList[j]; - } - } - } + auto last = std::unique(coordList.begin(), coordList.end()); + coordList.erase(last, coordList.end()); } }; diff --git a/src/common/geo/io/CMakeLists.txt b/src/common/geo/io/CMakeLists.txt index c2df1e27d35..180b9b77034 100644 --- a/src/common/geo/io/CMakeLists.txt +++ b/src/common/geo/io/CMakeLists.txt @@ -3,9 +3,6 @@ # This source code is licensed under Apache 2.0 License, # attached with Common Clause Condition 1.0, found in the LICENSES directory. -include_directories(${CMAKE_CURRENT_SOURCE_DIR}/wkt) -include_directories(${CMAKE_CURRENT_BINARY_DIR}/wkt) - if(ENABLE_VERBOSE_BISON) set(bison_flags "-Werror -v") else() diff --git a/src/common/geo/io/Geometry.h b/src/common/geo/io/Geometry.h index 51772bdba7b..c0ad6d8eb66 100644 --- a/src/common/geo/io/Geometry.h +++ b/src/common/geo/io/Geometry.h @@ -8,8 +8,10 @@ #include #include +#include #include +#include "common/base/Base.h" #include "common/geo/GeoShape.h" namespace nebula { @@ -21,50 +23,79 @@ struct Coordinate { Coordinate(double lng, double lat) : x(lng), y(lat) {} // TODO(jie) compare double correctly - bool operator==(const Coordinate &rhs) const { return x == rhs.x && y == rhs.y; } - bool operator!=(const Coordinate &rhs) const { return !(*this == rhs); } + bool operator==(const Coordinate& rhs) const { return x == rhs.x && y == rhs.y; } + bool operator!=(const Coordinate& rhs) const { return !(*this == rhs); } static bool isValidLng(double lng) { return std::abs(lng) <= 180; } static bool isValidLat(double lat) { return std::abs(lat) <= 90; } }; -struct Geometry { - virtual ~Geometry() {} - virtual GeoShape shape() const = 0; -}; - -struct Point : public Geometry { +struct Point { Coordinate coord; - explicit Point(const Coordinate &v) : coord(v) {} - explicit Point(Coordinate &&v) : coord(std::move(v)) {} - ~Point() override = default; - - GeoShape shape() const override { return GeoShape::POINT; } + explicit Point(const Coordinate& v) : coord(v) {} + explicit Point(Coordinate&& v) : coord(std::move(v)) {} + ~Point() {} }; -struct LineString : public Geometry { +struct LineString { std::vector coordList; - explicit LineString(const std::vector &v) : coordList(v) {} - explicit LineString(std::vector &&v) : coordList(std::move(v)) {} - ~LineString() override = default; + explicit LineString(const std::vector& v) : coordList(v) {} + explicit LineString(std::vector&& v) : coordList(std::move(v)) {} + ~LineString() {} - GeoShape shape() const override { return GeoShape::LINESTRING; } uint32_t numCoords() const { return coordList.size(); } }; -struct Polygon : public Geometry { +struct Polygon { std::vector> coordListList; - explicit Polygon(const std::vector> &v) : coordListList(v) {} - explicit Polygon(std::vector> &&v) : coordListList(std::move(v)) {} - ~Polygon() override = default; + explicit Polygon(const std::vector>& v) : coordListList(v) {} + explicit Polygon(std::vector>&& v) : coordListList(std::move(v)) {} + ~Polygon() {} - GeoShape shape() const override { return GeoShape::POLYGON; } uint32_t numRings() const { return coordListList.size(); } uint32_t numInteriorRing() const { return numRings() - 1; } }; +struct Geometry { + std::variant geom; + + Geometry(const Point& v) : geom(v) {} // NOLINT + Geometry(Point&& v) : geom(std::move(v)) {} // NOLINT + Geometry(const LineString& v) : geom(v) {} // NOLINT + Geometry(LineString&& v) : geom(std::move(v)) {} // NOLINT + Geometry(const Polygon& v) : geom(v) {} // NOLINT + Geometry(Polygon&& v) : geom(std::move(v)) {} // NOLINT + + GeoShape shape() const { + switch (geom.index()) { + case 0: + return GeoShape::POINT; + case 1: + return GeoShape::LINESTRING; + case 2: + return GeoShape::POLYGON; + default: + return GeoShape::UNKNOWN; + } + } + const Point& point() const { + CHECK(std::holds_alternative(geom)); + return std::get(geom); + } + + const LineString& lineString() const { + CHECK(std::holds_alternative(geom)); + return std::get(geom); + } + + const Polygon& polygon() const { + CHECK(std::holds_alternative(geom)); + return std::get(geom); + } +}; + } // namespace nebula diff --git a/src/common/geo/io/wkb/WKBReader.cpp b/src/common/geo/io/wkb/WKBReader.cpp index c4afbc55473..8c3223b34bb 100644 --- a/src/common/geo/io/wkb/WKBReader.cpp +++ b/src/common/geo/io/wkb/WKBReader.cpp @@ -8,13 +8,13 @@ namespace nebula { -StatusOr> WKBReader::read(std::string wkb) const { - uint8_t *beg = reinterpret_cast(wkb.data()); - uint8_t *end = beg + wkb.size(); +StatusOr WKBReader::read(const std::string &wkb) const { + const uint8_t *beg = reinterpret_cast(wkb.data()); + const uint8_t *end = beg + wkb.size(); return read(beg, end); } -StatusOr> WKBReader::read(uint8_t *&beg, uint8_t *end) const { +StatusOr WKBReader::read(const uint8_t *&beg, const uint8_t *end) const { auto byteOrderRet = readByteOrder(beg, end); NG_RETURN_IF_ERROR(byteOrderRet); ByteOrder byteOrder = byteOrderRet.value(); @@ -28,7 +28,7 @@ StatusOr> WKBReader::read(uint8_t *&beg, uint8_t *end) auto coordRet = readCoordinate(beg, end, byteOrder); NG_RETURN_IF_ERROR(coordRet); Coordinate coord = coordRet.value(); - return std::make_unique(coord); + return Point(coord); } case GeoShape::LINESTRING: { auto numPointsRet = readUint32(beg, end, byteOrder); @@ -37,7 +37,7 @@ StatusOr> WKBReader::read(uint8_t *&beg, uint8_t *end) auto coordListRet = readCoordinateList(beg, end, byteOrder, numPoints); NG_RETURN_IF_ERROR(coordListRet); std::vector coordList = coordListRet.value(); - return std::make_unique(coordList); + return LineString(coordList); } case GeoShape::POLYGON: { auto numRingsRet = readUint32(beg, end, byteOrder); @@ -46,7 +46,7 @@ StatusOr> WKBReader::read(uint8_t *&beg, uint8_t *end) auto coordListListRet = readCoordinateListList(beg, end, byteOrder, numRings); NG_RETURN_IF_ERROR(coordListListRet); std::vector> coordListList = coordListListRet.value(); - return std::make_unique(coordListList); + return Polygon(coordListList); } default: LOG(FATAL) @@ -56,7 +56,7 @@ StatusOr> WKBReader::read(uint8_t *&beg, uint8_t *end) } } -StatusOr WKBReader::readByteOrder(uint8_t *&beg, uint8_t *end) const { +StatusOr WKBReader::readByteOrder(const uint8_t *&beg, const uint8_t *end) const { auto vRet = readUint8(beg, end); NG_RETURN_IF_ERROR(vRet); uint8_t v = vRet.value(); @@ -67,8 +67,8 @@ StatusOr WKBReader::readByteOrder(uint8_t *&beg, uint8_t *end) const return byteOrder; } -StatusOr WKBReader::readShapeType(uint8_t *&beg, - uint8_t *end, +StatusOr WKBReader::readShapeType(const uint8_t *&beg, + const uint8_t *end, ByteOrder byteOrder) const { auto vRet = readUint32(beg, end, byteOrder); NG_RETURN_IF_ERROR(vRet); @@ -80,8 +80,8 @@ StatusOr WKBReader::readShapeType(uint8_t *&beg, return shapeType; } -StatusOr WKBReader::readCoordinate(uint8_t *&beg, - uint8_t *end, +StatusOr WKBReader::readCoordinate(const uint8_t *&beg, + const uint8_t *end, ByteOrder byteOrder) const { auto xRet = readDouble(beg, end, byteOrder); NG_RETURN_IF_ERROR(xRet); @@ -92,8 +92,8 @@ StatusOr WKBReader::readCoordinate(uint8_t *&beg, return Coordinate(x, y); } -StatusOr> WKBReader::readCoordinateList(uint8_t *&beg, - uint8_t *end, +StatusOr> WKBReader::readCoordinateList(const uint8_t *&beg, + const uint8_t *end, ByteOrder byteOrder, uint32_t num) const { std::vector coordList; @@ -108,7 +108,7 @@ StatusOr> WKBReader::readCoordinateList(uint8_t *&beg, } StatusOr>> WKBReader::readCoordinateListList( - uint8_t *&beg, uint8_t *end, ByteOrder byteOrder, uint32_t num) const { + const uint8_t *&beg, const uint8_t *end, ByteOrder byteOrder, uint32_t num) const { std::vector> coordListList; coordListList.reserve(num); for (size_t i = 0; i < num; ++i) { @@ -123,7 +123,7 @@ StatusOr>> WKBReader::readCoordinateListList return coordListList; } -StatusOr WKBReader::readUint8(uint8_t *&beg, uint8_t *end) const { +StatusOr WKBReader::readUint8(const uint8_t *&beg, const uint8_t *end) const { auto requiredSize = static_cast(sizeof(uint8_t)); if (end - beg < requiredSize) { return Status::Error("Unable to parse uint8_t"); @@ -133,7 +133,9 @@ StatusOr WKBReader::readUint8(uint8_t *&beg, uint8_t *end) const { return v; } -StatusOr WKBReader::readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { +StatusOr WKBReader::readUint32(const uint8_t *&beg, + const uint8_t *end, + ByteOrder byteOrder) const { auto requiredSize = static_cast(sizeof(uint32_t)); if (end - beg < requiredSize) { return Status::Error("Unable to parse uint32_t"); @@ -143,7 +145,9 @@ StatusOr WKBReader::readUint32(uint8_t *&beg, uint8_t *end, ByteOrder return v; } -StatusOr WKBReader::readDouble(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const { +StatusOr WKBReader::readDouble(const uint8_t *&beg, + const uint8_t *end, + ByteOrder byteOrder) const { auto requiredSize = static_cast(sizeof(double)); if (end - beg < requiredSize) { return Status::Error("Unable to parse double"); diff --git a/src/common/geo/io/wkb/WKBReader.h b/src/common/geo/io/wkb/WKBReader.h index a6012ff02fe..597d3dfe50a 100644 --- a/src/common/geo/io/wkb/WKBReader.h +++ b/src/common/geo/io/wkb/WKBReader.h @@ -20,30 +20,34 @@ class WKBReader { ~WKBReader() {} // TODO(jie) Check the validity of geometry when reading the wkb - StatusOr> read(std::string wkb) const; + StatusOr read(const std::string &wkb) const; - StatusOr> read(uint8_t *&beg, uint8_t *end) const; + StatusOr read(const uint8_t *&beg, const uint8_t *end) const; - StatusOr readByteOrder(uint8_t *&beg, uint8_t *end) const; + StatusOr readByteOrder(const uint8_t *&beg, const uint8_t *end) const; - StatusOr readShapeType(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; + StatusOr readShapeType(const uint8_t *&beg, + const uint8_t *end, + ByteOrder byteOrder) const; - StatusOr readCoordinate(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; + StatusOr readCoordinate(const uint8_t *&beg, + const uint8_t *end, + ByteOrder byteOrder) const; - StatusOr> readCoordinateList(uint8_t *&beg, - uint8_t *end, + StatusOr> readCoordinateList(const uint8_t *&beg, + const uint8_t *end, ByteOrder byteOrder, uint32_t num) const; - StatusOr>> readCoordinateListList(uint8_t *&beg, - uint8_t *end, + StatusOr>> readCoordinateListList(const uint8_t *&beg, + const uint8_t *end, ByteOrder byteOrder, uint32_t num) const; - StatusOr readUint8(uint8_t *&beg, uint8_t *end) const; + StatusOr readUint8(const uint8_t *&beg, const uint8_t *end) const; - StatusOr readUint32(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; + StatusOr readUint32(const uint8_t *&beg, const uint8_t *end, ByteOrder byteOrder) const; - StatusOr readDouble(uint8_t *&beg, uint8_t *end, ByteOrder byteOrder) const; + StatusOr readDouble(const uint8_t *&beg, const uint8_t *end, ByteOrder byteOrder) const; }; } // namespace nebula diff --git a/src/common/geo/io/wkb/WKBWriter.cpp b/src/common/geo/io/wkb/WKBWriter.cpp index f5f9878e383..fa28cdf21a6 100644 --- a/src/common/geo/io/wkb/WKBWriter.cpp +++ b/src/common/geo/io/wkb/WKBWriter.cpp @@ -8,33 +8,34 @@ namespace nebula { -std::string WKBWriter::write(const Geometry* geom) const { +// TODO(jie) Check the validity of geom when writing wkb +std::string WKBWriter::write(const Geometry& geom) const { std::string wkb = ""; uint8_t byteOrder = static_cast>(ByteOrderData::getMachineByteOrder()); writeUint8(wkb, byteOrder); - auto shape = geom->shape(); + auto shape = geom.shape(); uint32_t shapeType = folly::to(shape); writeUint32(wkb, shapeType); switch (shape) { case GeoShape::POINT: { - const Point* point = static_cast(geom); - writeCoordinate(wkb, point->coord); + const Point& point = geom.point(); + writeCoordinate(wkb, point.coord); return wkb; } case GeoShape::LINESTRING: { - const LineString* line = static_cast(geom); - auto coordList = line->coordList; + const LineString& line = geom.lineString(); + auto coordList = line.coordList; uint32_t numPoints = coordList.size(); writeUint32(wkb, numPoints); writeCoordinateList(wkb, coordList); return wkb; } case GeoShape::POLYGON: { - const Polygon* polygon = static_cast(geom); - auto coordListList = polygon->coordListList; + const Polygon& polygon = geom.polygon(); + auto coordListList = polygon.coordListList; uint32_t numRings = coordListList.size(); writeUint32(wkb, numRings); writeCoordinateListList(wkb, coordListList); diff --git a/src/common/geo/io/wkb/WKBWriter.h b/src/common/geo/io/wkb/WKBWriter.h index 01d1c3558b4..d43cf7eb4b7 100644 --- a/src/common/geo/io/wkb/WKBWriter.h +++ b/src/common/geo/io/wkb/WKBWriter.h @@ -18,7 +18,7 @@ class WKBWriter { ~WKBWriter() {} - std::string write(const Geometry* geom) const; + std::string write(const Geometry& geom) const; void writeCoordinate(std::string& wkb, const Coordinate& coord) const; diff --git a/src/common/geo/io/wkt/WKTWriter.cpp b/src/common/geo/io/wkt/WKTWriter.cpp index 52eed139870..7e992579a84 100644 --- a/src/common/geo/io/wkt/WKTWriter.cpp +++ b/src/common/geo/io/wkt/WKTWriter.cpp @@ -8,23 +8,23 @@ namespace nebula { -std::string WKTWriter::write(const Geometry* geom) const { +std::string WKTWriter::write(const Geometry& geom) const { std::string wkt = ""; - auto shape = geom->shape(); + auto shape = geom.shape(); switch (shape) { case GeoShape::POINT: { wkt.append("POINT"); - const Point* point = static_cast(geom); + const Point& point = geom.point(); wkt.append("("); - writeCoordinate(wkt, point->coord); + writeCoordinate(wkt, point.coord); wkt.append(")"); return wkt; } case GeoShape::LINESTRING: { wkt.append("LINESTRING"); - const LineString* line = static_cast(geom); - auto coordList = line->coordList; + const LineString& line = geom.lineString(); + auto coordList = line.coordList; uint32_t numPoints = coordList.size(); UNUSED(numPoints); wkt.append("("); @@ -34,8 +34,8 @@ std::string WKTWriter::write(const Geometry* geom) const { } case GeoShape::POLYGON: { wkt.append("POLYGON"); - const Polygon* polygon = static_cast(geom); - auto coordListList = polygon->coordListList; + const Polygon& polygon = geom.polygon(); + auto coordListList = polygon.coordListList; uint32_t numRings = coordListList.size(); UNUSED(numRings); wkt.append("("); @@ -44,7 +44,7 @@ std::string WKTWriter::write(const Geometry* geom) const { return wkt; } default: - LOG(FATAL) + LOG(ERROR) << "Geomtry shapes other than Point/LineString/Polygon are not currently supported"; return ""; } @@ -81,8 +81,6 @@ void WKTWriter::WKTWriter::writeCoordinateListList( void WKTWriter::writeDouble(std::string& wkt, double v) const { wkt.append(folly::to(v)); - LOG(INFO) << "writeDouble(wkt, v): " << wkt << ", " << v - << ", folly::to: " << folly::to(v); } } // namespace nebula diff --git a/src/common/geo/io/wkt/WKTWriter.h b/src/common/geo/io/wkt/WKTWriter.h index 3c2fe8dc258..bcfbef81748 100644 --- a/src/common/geo/io/wkt/WKTWriter.h +++ b/src/common/geo/io/wkt/WKTWriter.h @@ -17,7 +17,7 @@ class WKTWriter { ~WKTWriter() {} - std::string write(const Geometry* geom) const; + std::string write(const Geometry& geom) const; void writeCoordinate(std::string& wkt, const Coordinate& coord) const; diff --git a/src/common/geo/io/wkt/test/WKTParserTest.cpp b/src/common/geo/io/wkt/test/WKTParserTest.cpp index e9cd4774a96..90e2fb64330 100644 --- a/src/common/geo/io/wkt/test/WKTParserTest.cpp +++ b/src/common/geo/io/wkt/test/WKTParserTest.cpp @@ -21,15 +21,15 @@ class WKTParserTest : public ::testing::Test { StatusOr> parse(const std::string& wkt) { auto geomRet = WKTReader().read(wkt); NG_RETURN_IF_ERROR(geomRet); - NG_RETURN_IF_ERROR(check(geomRet.value().get())); + NG_RETURN_IF_ERROR(check(*geomRet.value().get())); return geomRet; } - Status check(const Geometry* geom) { + Status check(const Geometry& geom) { auto wkt = WKTWriter().write(geom); auto geomCopyRet = WKTReader().read(wkt); auto geomCopy = std::move(geomCopyRet).value(); - auto wktCopy = WKTWriter().write(geomCopy.get()); + auto wktCopy = WKTWriter().write(*geomCopy.get()); if (wkt != wktCopy) { return Status::Error("The reparsed geometry `%s' is different from origin `%s'.", diff --git a/src/common/geo/io/wkt/wkt_parser.yy b/src/common/geo/io/wkt/wkt_parser.yy index 4af77374ac9..e589e22cefc 100644 --- a/src/common/geo/io/wkt/wkt_parser.yy +++ b/src/common/geo/io/wkt/wkt_parser.yy @@ -25,7 +25,7 @@ class WKTScanner; } %code { - #include "WKTScanner.h" + #include "common/geo/io/wkt/WKTScanner.h" static int yylex(nebula::WKTParser::semantic_type* yylval, nebula::WKTParser::location_type *yylloc, nebula::WKTScanner& scanner); @@ -72,16 +72,15 @@ class WKTScanner; geometry : point { - $$ = $1; - LOG(INFO) << "jie test" << static_cast($$)->coord.x << static_cast($$)->coord.y; + $$ = new Geometry(*$1); *geom = $$; } | linestring { - $$ = $1; + $$ = new Geometry(*$1); *geom = $$; } | polygon { - $$ = $1; + $$ = new Geometry(*$1); *geom = $$; } ; From c7732552a1546018701ff89168e623d3e9aa22a9 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 28 Sep 2021 21:06:20 +0800 Subject: [PATCH 4/9] add more geo value test --- src/common/datatypes/Geography.cpp | 8 +++--- src/common/datatypes/Geography.h | 13 +++++---- src/common/datatypes/test/GeographyTest.cpp | 24 ++++++++-------- src/common/datatypes/test/ValueTest.cpp | 30 +++++++++++++++++++- src/common/geo/io/wkt/WKTReader.h | 4 +-- src/common/geo/io/wkt/test/WKTParserTest.cpp | 8 +++--- src/common/utils/IndexKeyUtils.h | 5 +++- 7 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/common/datatypes/Geography.cpp b/src/common/datatypes/Geography.cpp index 661199a4519..3318c89f087 100644 --- a/src/common/datatypes/Geography.cpp +++ b/src/common/datatypes/Geography.cpp @@ -19,12 +19,12 @@ namespace nebula { -StatusOr> Geography::fromWKT(const std::string& wkt) { +StatusOr Geography::fromWKT(const std::string& wkt) { auto geomRet = WKTReader().read(wkt); NG_RETURN_IF_ERROR(geomRet); - auto geom = std::move(geomRet).value(); - auto wkb = WKBWriter().write(*geom); - return std::make_unique(wkb); + auto geom = geomRet.value(); + auto wkb = WKBWriter().write(geom); + return Geography(wkb); } GeoShape Geography::shape() const { diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h index 74a99641ad1..4e216c9ffc2 100644 --- a/src/common/datatypes/Geography.h +++ b/src/common/datatypes/Geography.h @@ -45,12 +45,7 @@ struct Geography { Geography() = default; - explicit Geography(const std::string& bytes) { - // TODO(jie): Must ensure the bytes is valid - wkb = bytes; - } - - static StatusOr> fromWKT(const std::string& wkt); + static StatusOr fromWKT(const std::string& wkt); GeoShape shape() const; @@ -74,6 +69,12 @@ struct Geography { bool operator<(const Geography& rhs) const { return wkb < rhs.wkb; } + private: + explicit Geography(const std::string& bytes) { + // TODO(jie): Must ensure the bytes is valid + wkb = bytes; + } + private: std::unique_ptr s2RegionFromGeomtry(const Geometry* geom) const; diff --git a/src/common/datatypes/test/GeographyTest.cpp b/src/common/datatypes/test/GeographyTest.cpp index faa29f9e383..8c0a4b12115 100644 --- a/src/common/datatypes/test/GeographyTest.cpp +++ b/src/common/datatypes/test/GeographyTest.cpp @@ -17,22 +17,22 @@ TEST(Geography, shape) { std::string wkt = "POINT(3 7)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); - auto g = std::move(gRet.value()); - EXPECT_EQ(GeoShape::POINT, g->shape()); + auto g = gRet.value(); + EXPECT_EQ(GeoShape::POINT, g.shape()); } { std::string wkt = "LINESTRING(28.4 79.20,134.25 -28.34)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); - auto g = std::move(gRet.value()); - EXPECT_EQ(GeoShape::LINESTRING, g->shape()); + auto g = gRet.value(); + EXPECT_EQ(GeoShape::LINESTRING, g.shape()); } { std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); - auto g = std::move(gRet.value()); - EXPECT_EQ(GeoShape::POLYGON, g->shape()); + auto g = gRet.value(); + EXPECT_EQ(GeoShape::POLYGON, g.shape()); } } @@ -41,8 +41,8 @@ TEST(Geography, asWKT) { std::string wkt = "POINT(3 7)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); - auto g = std::move(gRet.value()); - auto got = g->asWKT(); + auto g = gRet.value(); + auto got = g.asWKT(); ASSERT_TRUE(!!got); EXPECT_EQ(wkt, *got); } @@ -50,8 +50,8 @@ TEST(Geography, asWKT) { std::string wkt = "LINESTRING(28.4 79.2,134.25 -28.34)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); - auto g = std::move(gRet.value()); - auto got = g->asWKT(); + auto g = gRet.value(); + auto got = g.asWKT(); ASSERT_TRUE(!!got); EXPECT_EQ(wkt, *got); } @@ -59,8 +59,8 @@ TEST(Geography, asWKT) { std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); - auto g = std::move(gRet.value()); - auto got = g->asWKT(); + auto g = gRet.value(); + auto got = g.asWKT(); ASSERT_TRUE(!!got); EXPECT_EQ(wkt, *got); } diff --git a/src/common/datatypes/test/ValueTest.cpp b/src/common/datatypes/test/ValueTest.cpp index 67175a22bfc..9ca5879a8e1 100644 --- a/src/common/datatypes/test/ValueTest.cpp +++ b/src/common/datatypes/test/ValueTest.cpp @@ -291,6 +291,9 @@ TEST(Value, Comparison) { Value vDateTime1(DateTime(1998, 9, 8, 12, 30, 04, 56)); Value vDateTime2(DateTime(1998, 9, 8, 13, 30, 04, 56)); Value vDateTime3(DateTime(1998, 9, 8, 13, 30, 04, 56)); // for further tests + Value vGeo1 = Geography::fromWKT("POINT(4 7)").value(); + Value vGeo2 = Geography::fromWKT("POINT(5 7)").value(); + Value vGeo3 = Geography::fromWKT("POINT(5 7)").value(); // null/empty { @@ -551,6 +554,25 @@ TEST(Value, Comparison) { EXPECT_EQ(Value::Type::BOOL, v.type()); EXPECT_EQ(true, v.getBool()); } + // geography + { + Value v = vGeo1 == vGeo2; + EXPECT_EQ(Value::Type::BOOL, v.type()); + EXPECT_EQ(false, v.getBool()); + + v = vGeo1 != vGeo2; + EXPECT_EQ(Value::Type::BOOL, v.type()); + EXPECT_EQ(true, v.getBool()); + + v = vGeo2 == vGeo3; + EXPECT_EQ(Value::Type::BOOL, v.type()); + EXPECT_EQ(true, v.getBool()); + + v = vGeo2 != vGeo3; + EXPECT_EQ(Value::Type::BOOL, v.type()); + EXPECT_EQ(false, v.getBool()); + // TODO(jie) Add more geo test + } } TEST(Value, Logical) { @@ -1001,6 +1023,7 @@ TEST(Value, typeName) { EXPECT_EQ("map", Value(Map()).typeName()); EXPECT_EQ("set", Value(Set()).typeName()); EXPECT_EQ("dataset", Value(DataSet()).typeName()); + EXPECT_EQ("geography", Value(Geography()).typeName()); EXPECT_EQ("__NULL__", Value::kNullValue.typeName()); EXPECT_EQ("NaN", Value::kNullNaN.typeName()); EXPECT_EQ("BAD_DATA", Value::kNullBadData.typeName()); @@ -1091,6 +1114,9 @@ TEST(Value, DecodeEncode) { // DataSet Value(DataSet({"col1", "col2"})), + + // Geography + Value(Geography::fromWKT("Point(3 8)").value()), }; for (const auto& val : values) { std::string buf; @@ -1129,7 +1155,9 @@ TEST(Value, Ctor) { EXPECT_TRUE(vSet.isSet()); Value vMap(Map({{"a", 9}, {"b", 10}})); EXPECT_TRUE(vMap.isMap()); - + // TODO(jie) Add more geography value test + Value vGeo(Geography::fromWKT("LINESTRING(0 1, 2 7)").value()); + EXPECT_TRUE(vGeo.isGeography()); // Disabled // Lead to compile error // Value v(nullptr); diff --git a/src/common/geo/io/wkt/WKTReader.h b/src/common/geo/io/wkt/WKTReader.h index cf7164b1b94..0e65115c2fd 100644 --- a/src/common/geo/io/wkt/WKTReader.h +++ b/src/common/geo/io/wkt/WKTReader.h @@ -38,7 +38,7 @@ class WKTReader { if (geom_ != nullptr) delete geom_; } - StatusOr> read(std::string wkt) { + StatusOr read(std::string wkt) { // Since WKTScanner needs a writable buffer, we have to copy the query string buffer_ = std::move(wkt); pos_ = &buffer_[0]; @@ -64,7 +64,7 @@ class WKTReader { auto *geom = geom_; geom_ = nullptr; scanner_.setWKT(nullptr); - return std::unique_ptr(geom); + return *geom; } private: diff --git a/src/common/geo/io/wkt/test/WKTParserTest.cpp b/src/common/geo/io/wkt/test/WKTParserTest.cpp index 90e2fb64330..ef4c4512a28 100644 --- a/src/common/geo/io/wkt/test/WKTParserTest.cpp +++ b/src/common/geo/io/wkt/test/WKTParserTest.cpp @@ -18,18 +18,18 @@ class WKTParserTest : public ::testing::Test { void TearDown() override {} protected: - StatusOr> parse(const std::string& wkt) { + StatusOr parse(const std::string& wkt) { auto geomRet = WKTReader().read(wkt); NG_RETURN_IF_ERROR(geomRet); - NG_RETURN_IF_ERROR(check(*geomRet.value().get())); + NG_RETURN_IF_ERROR(check(geomRet.value())); return geomRet; } Status check(const Geometry& geom) { auto wkt = WKTWriter().write(geom); auto geomCopyRet = WKTReader().read(wkt); - auto geomCopy = std::move(geomCopyRet).value(); - auto wktCopy = WKTWriter().write(*geomCopy.get()); + auto geomCopy = geomCopyRet.value(); + auto wktCopy = WKTWriter().write(geomCopy); if (wkt != wktCopy) { return Status::Error("The reparsed geometry `%s' is different from origin `%s'.", diff --git a/src/common/utils/IndexKeyUtils.h b/src/common/utils/IndexKeyUtils.h index 666f126c982..a40c7f5fa17 100644 --- a/src/common/utils/IndexKeyUtils.h +++ b/src/common/utils/IndexKeyUtils.h @@ -176,7 +176,10 @@ class IndexKeyUtils final { static std::string encodeUint64(uint64_t v) { auto val = folly::Endian::big(v); - return {reinterpret_cast(&val), sizeof(uint64_t)}; + std::string raw; + raw.reserve(sizeof(uint64_t)); + raw.append(reinterpret_cast(&val), sizeof(uint64_t)); + return raw; } static std::string encodeRank(EdgeRanking rank) { return IndexKeyUtils::encodeInt64(rank); } From a15a278cf37613c9dcfa46494b3aa5bdb5d338e9 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 28 Sep 2021 22:15:06 +0800 Subject: [PATCH 5/9] fix memory leak and strict aliasing error caused by reinterpretcast --- src/common/geo/io/wkt/WKTReader.h | 2 +- src/common/geo/io/wkt/wkt_parser.yy | 20 ++++++++++---------- src/common/utils/IndexKeyUtils.h | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/common/geo/io/wkt/WKTReader.h b/src/common/geo/io/wkt/WKTReader.h index 0e65115c2fd..8edade1dea9 100644 --- a/src/common/geo/io/wkt/WKTReader.h +++ b/src/common/geo/io/wkt/WKTReader.h @@ -64,7 +64,7 @@ class WKTReader { auto *geom = geom_; geom_ = nullptr; scanner_.setWKT(nullptr); - return *geom; + return std::move(*geom); } private: diff --git a/src/common/geo/io/wkt/wkt_parser.yy b/src/common/geo/io/wkt/wkt_parser.yy index e589e22cefc..6c41dc030df 100644 --- a/src/common/geo/io/wkt/wkt_parser.yy +++ b/src/common/geo/io/wkt/wkt_parser.yy @@ -72,22 +72,22 @@ class WKTScanner; geometry : point { - $$ = new Geometry(*$1); + $$ = new Geometry(std::move(*$1)); *geom = $$; } | linestring { - $$ = new Geometry(*$1); + $$ = new Geometry(std::move(*$1)); *geom = $$; } | polygon { - $$ = new Geometry(*$1); + $$ = new Geometry(std::move(*$1)); *geom = $$; } ; point : KW_POINT L_PAREN coordinate R_PAREN { - $$ = new Point(*$3); + $$ = new Point(std::move(*$3)); } ; @@ -96,7 +96,7 @@ linestring if ($3->size() < 2) { throw nebula::WKTParser::syntax_error(@3, "LineString must have at least 2 coordinates"); } - $$ = new LineString(*$3); + $$ = new LineString(std::move(*$3)); } ; @@ -111,7 +111,7 @@ polygon throw nebula::WKTParser::syntax_error(@3, "Polygon's LinearRing must be closed"); } } - $$ = new Polygon(*$3); + $$ = new Polygon(std::move(*$3)); } ; @@ -130,23 +130,23 @@ coordinate coordinate_list : coordinate { $$ = new std::vector(); - $$->emplace_back(*$1); + $$->emplace_back(std::move(*$1)); } | coordinate_list COMMA coordinate { $$ = $1; - $$->emplace_back(*$3); + $$->emplace_back(std::move(*$3)); } ; coordinate_list_list : L_PAREN coordinate_list R_PAREN { $$ = new std::vector>(); - $$->emplace_back(*$2); + $$->emplace_back(std::move(*$2)); } | coordinate_list_list COMMA L_PAREN coordinate_list R_PAREN { $$ = $1; - $$->emplace_back(*$4); + $$->emplace_back(std::move(*$4)); } ; diff --git a/src/common/utils/IndexKeyUtils.h b/src/common/utils/IndexKeyUtils.h index a40c7f5fa17..bfec7f209b4 100644 --- a/src/common/utils/IndexKeyUtils.h +++ b/src/common/utils/IndexKeyUtils.h @@ -202,9 +202,9 @@ class IndexKeyUtils final { * TODO : now, the -(std::numeric_limits::min()) * have a problem of precision overflow. current return value is -nan. */ - auto i = *reinterpret_cast(&v); + auto i = *reinterpret_cast(static_cast(&v)); i = -(std::numeric_limits::max() + i); - v = *reinterpret_cast(&i); + v = *reinterpret_cast(static_cast(&i)); } auto val = folly::Endian::big(v); auto* c = reinterpret_cast(&val); @@ -221,9 +221,9 @@ class IndexKeyUtils final { auto val = *reinterpret_cast(v); val = folly::Endian::big(val); if (val < 0) { - auto i = *reinterpret_cast(&val); + auto i = *reinterpret_cast(static_cast(&val)); i = -(std::numeric_limits::max() + i); - val = *reinterpret_cast(&i); + val = *reinterpret_cast(static_cast(&i)); } return val; } From 71838e64d3eaaf8c7edcbd58a5912872e7938fc1 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 29 Sep 2021 02:34:03 +0800 Subject: [PATCH 6/9] fix wkt parser and graph parser memory leak --- src/common/datatypes/test/ValueTest.cpp | 8 +++-- src/common/geo/io/Geometry.h | 1 + src/common/geo/io/wkt/WKTReader.h | 6 ++-- src/common/geo/io/wkt/wkt_parser.yy | 30 ++++++++++------ src/parser/parser.yy | 5 +-- src/parser/test/ParserTest.cpp | 47 +++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 16 deletions(-) diff --git a/src/common/datatypes/test/ValueTest.cpp b/src/common/datatypes/test/ValueTest.cpp index 9ca5879a8e1..b6a345ef065 100644 --- a/src/common/datatypes/test/ValueTest.cpp +++ b/src/common/datatypes/test/ValueTest.cpp @@ -1156,8 +1156,12 @@ TEST(Value, Ctor) { Value vMap(Map({{"a", 9}, {"b", 10}})); EXPECT_TRUE(vMap.isMap()); // TODO(jie) Add more geography value test - Value vGeo(Geography::fromWKT("LINESTRING(0 1, 2 7)").value()); - EXPECT_TRUE(vGeo.isGeography()); + 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()); // Disabled // Lead to compile error // Value v(nullptr); diff --git a/src/common/geo/io/Geometry.h b/src/common/geo/io/Geometry.h index c0ad6d8eb66..174a4f32e48 100644 --- a/src/common/geo/io/Geometry.h +++ b/src/common/geo/io/Geometry.h @@ -20,6 +20,7 @@ namespace nebula { struct Coordinate { double x, y; + Coordinate() = default; Coordinate(double lng, double lat) : x(lng), y(lat) {} // TODO(jie) compare double correctly diff --git a/src/common/geo/io/wkt/WKTReader.h b/src/common/geo/io/wkt/WKTReader.h index 8edade1dea9..f1f16cacff6 100644 --- a/src/common/geo/io/wkt/WKTReader.h +++ b/src/common/geo/io/wkt/WKTReader.h @@ -61,10 +61,12 @@ class WKTReader { if (geom_ == nullptr) { return Status::StatementEmpty(); // WKTEmpty() } - auto *geom = geom_; + auto geom = geom_; geom_ = nullptr; scanner_.setWKT(nullptr); - return std::move(*geom); + auto tmp = std::move(*geom); + delete geom; + return tmp; } private: diff --git a/src/common/geo/io/wkt/wkt_parser.yy b/src/common/geo/io/wkt/wkt_parser.yy index 6c41dc030df..ee92a150bd3 100644 --- a/src/common/geo/io/wkt/wkt_parser.yy +++ b/src/common/geo/io/wkt/wkt_parser.yy @@ -37,15 +37,15 @@ class WKTScanner; Point* pointVal; LineString* lineVal; Polygon* polygonVal; - Coordinate* coordVal; + Coordinate coordVal; std::vector* coordListVal; std::vector>* coordListListVal; } /* destructors */ -%destructor {} -%destructor {} -%destructor {} +%destructor {} +%destructor {} +%destructor { delete $$; } <*> /* wkt shape type prefix */ %token KW_POINT KW_LINESTRING KW_POLYGON @@ -73,45 +73,53 @@ class WKTScanner; geometry : point { $$ = new Geometry(std::move(*$1)); + delete $1; *geom = $$; } | linestring { $$ = new Geometry(std::move(*$1)); + delete $1; *geom = $$; } | polygon { $$ = new Geometry(std::move(*$1)); + delete $1; *geom = $$; } ; point : KW_POINT L_PAREN coordinate R_PAREN { - $$ = new Point(std::move(*$3)); + $$ = new Point(std::move($3)); } ; linestring : KW_LINESTRING L_PAREN coordinate_list R_PAREN { if ($3->size() < 2) { + delete $3; throw nebula::WKTParser::syntax_error(@3, "LineString must have at least 2 coordinates"); } $$ = new LineString(std::move(*$3)); + delete $3; } ; polygon : KW_POLYGON L_PAREN coordinate_list_list R_PAREN { for (size_t i = 0; i < $3->size(); ++i) { - auto coordList = (*$3)[i]; + const auto &coordList = (*$3)[i]; if (coordList.size() < 4) { + delete $3; throw nebula::WKTParser::syntax_error(@3, "Polygon's LinearRing must have at least 4 coordinates"); } if (coordList.front() != coordList.back()) { + delete $3; throw nebula::WKTParser::syntax_error(@3, "Polygon's LinearRing must be closed"); } } $$ = new Polygon(std::move(*$3)); + delete $3; } ; @@ -123,18 +131,19 @@ coordinate if (!Coordinate::isValidLat($2)) { throw nebula::WKTParser::syntax_error(@2, "Latitude must be between -90 and 90 degrees"); } - $$ = new Coordinate($1, $2); + $$.x = $1; + $$.y = $2; } ; coordinate_list : coordinate { $$ = new std::vector(); - $$->emplace_back(std::move(*$1)); + $$->emplace_back(std::move($1)); } | coordinate_list COMMA coordinate { $$ = $1; - $$->emplace_back(std::move(*$3)); + $$->emplace_back(std::move($3)); } ; @@ -142,11 +151,12 @@ coordinate_list_list : L_PAREN coordinate_list R_PAREN { $$ = new std::vector>(); $$->emplace_back(std::move(*$2)); - + delete $2; } | coordinate_list_list COMMA L_PAREN coordinate_list R_PAREN { $$ = $1; $$->emplace_back(std::move(*$4)); + delete $4; } ; diff --git a/src/parser/parser.yy b/src/parser/parser.yy index a02699a293c..be539f6eaa3 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -857,10 +857,11 @@ predicate_name predicate_expression : predicate_name L_PAREN expression KW_IN expression KW_WHERE expression R_PAREN { if ($3->kind() != Expression::Kind::kLabel) { + delete $1; throw nebula::GraphParser::syntax_error(@3, "The loop variable must be a label in predicate functions"); } - auto &innerVar = static_cast($3)->name(); - auto *expr = PredicateExpression::make(qctx->objPool(), *$1, innerVar, $5, $7); + std::string innerVar = static_cast($3)->name(); + auto *expr = PredicateExpression::make(qctx->objPool(), *$1, innerVar, $5, $7); // TODO(jie) Use std::unique_ptr nebula::graph::ParserUtil::rewritePred(qctx, expr, innerVar); $$ = expr; delete $1; diff --git a/src/parser/test/ParserTest.cpp b/src/parser/test/ParserTest.cpp index 33454dc38db..8ccc3490942 100644 --- a/src/parser/test/ParserTest.cpp +++ b/src/parser/test/ParserTest.cpp @@ -3015,4 +3015,51 @@ TEST_F(ParserTest, ShowAndKillQueryTest) { ASSERT_EQ(result.value()->toString(), "KILL QUERY (session=123, plan=123)"); } } + +TEST_F(ParserTest, DetectMemoryLeakTest) { + { + std::string query = "YIELD any(n IN [1, 2, 3, 4, 5] WHERE n > 2)"; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + ASSERT_EQ(result.value()->toString(), "YIELD any(n IN [1,2,3,4,5] WHERE (n>2))"); + } + // 3 is not expr of kLabel + { + std::string query = "YIELD [3 IN [1, 2] WHERE n > 2]"; + auto result = parse(query); + ASSERT_FALSE(result.ok()) << result.status(); + } + // a+b is not expr of kLabel + { + std::string query = "YIELD [a+b IN [1, 2] | n + 10]"; + auto result = parse(query); + ASSERT_FALSE(result.ok()) << result.status(); + } + // a+b is not expr of kLabel + { + std::string query = "YIELD [a+b IN [1, 2] WHERE n > 2 | n + 10]"; + auto result = parse(query); + ASSERT_FALSE(result.ok()) << result.status(); + } + { + std::string query = "YIELD EXISTS(v.age)"; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + ASSERT_EQ(result.value()->toString(), "YIELD exists(v.age)"); + } + /// 333 is not expr of kLabelAttribute, kAttribute or kSubscript + { + std::string query = "YIELD EXISTS(233)"; + auto result = parse(query); + ASSERT_FALSE(result.ok()) << result.status(); + } + { + std::string query = "YIELD reduce(totalNum = 10, n IN range(1, 3) | totalNum + n)"; + auto result = parse(query); + ASSERT_TRUE(result.ok()) << result.status(); + ASSERT_EQ(result.value()->toString(), + "YIELD reduce(totalNum = 10, n IN range(1,3) | (totalNum+n))"); + } +} + } // namespace nebula From a51b634eccf194b4e4666f61a36af310fcd3f6dd Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 29 Sep 2021 10:07:57 +0800 Subject: [PATCH 7/9] try to fix the strict alias error --- src/common/datatypes/Geography.h | 11 ----------- src/common/utils/IndexKeyUtils.h | 12 ++++++++---- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h index 4e216c9ffc2..bf8e7c9ebf4 100644 --- a/src/common/datatypes/Geography.h +++ b/src/common/datatypes/Geography.h @@ -74,17 +74,6 @@ struct Geography { // TODO(jie): Must ensure the bytes is valid wkb = bytes; } - - private: - std::unique_ptr s2RegionFromGeomtry(const Geometry* geom) const; - - S2Point s2PointFromCoordinate(const Coordinate& coord) const; - - std::vector s2PointsFromCoordinateList(const std::vector& coordList) const; - - bool isLoopClosed(const std::vector& coordList) const; - - void removeAdjacentDuplicateCoordinates(std::vector& coordList) const; }; inline std::ostream& operator<<(std::ostream& os, const Geography& g) { return os << g.wkb; } diff --git a/src/common/utils/IndexKeyUtils.h b/src/common/utils/IndexKeyUtils.h index bfec7f209b4..23d699abce8 100644 --- a/src/common/utils/IndexKeyUtils.h +++ b/src/common/utils/IndexKeyUtils.h @@ -202,9 +202,11 @@ class IndexKeyUtils final { * TODO : now, the -(std::numeric_limits::min()) * have a problem of precision overflow. current return value is -nan. */ - auto i = *reinterpret_cast(static_cast(&v)); + auto* c1 = reinterpret_cast(&v); + auto i = *reinterpret_cast(c1); i = -(std::numeric_limits::max() + i); - v = *reinterpret_cast(static_cast(&i)); + auto* c2 = reinterpret_cast(&i); + v = *reinterpret_cast(c2); } auto val = folly::Endian::big(v); auto* c = reinterpret_cast(&val); @@ -221,9 +223,11 @@ class IndexKeyUtils final { auto val = *reinterpret_cast(v); val = folly::Endian::big(val); if (val < 0) { - auto i = *reinterpret_cast(static_cast(&val)); + auto* c1 = reinterpret_cast(&val); + auto i = *reinterpret_cast(c1); i = -(std::numeric_limits::max() + i); - val = *reinterpret_cast(static_cast(&i)); + auto* c2 = reinterpret_cast(&i); + val = *reinterpret_cast(c2); } return val; } From 1ec411ab38f1b6c564ea3253ef563f977646ffd0 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 29 Sep 2021 15:39:36 +0800 Subject: [PATCH 8/9] fix strict alias error again --- src/meta/MetaServiceUtils.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/meta/MetaServiceUtils.cpp b/src/meta/MetaServiceUtils.cpp index 735abbbd912..de0de19effb 100644 --- a/src/meta/MetaServiceUtils.cpp +++ b/src/meta/MetaServiceUtils.cpp @@ -857,7 +857,8 @@ std::string MetaServiceUtils::roleSpacePrefix(GraphSpaceID spaceId) { } std::string MetaServiceUtils::parseRoleStr(folly::StringPiece key) { - auto type = *reinterpret_cast(&key); + auto* c = reinterpret_cast(&key); + auto type = *reinterpret_cast(c); std::string role; switch (type) { case cpp2::RoleType::GOD: { From 72c89f0547f0abba8c84292e549451958289744c Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 29 Sep 2021 16:56:08 +0800 Subject: [PATCH 9/9] remove unused header file --- src/common/datatypes/Geography.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h index bf8e7c9ebf4..241f8c5a09d 100644 --- a/src/common/datatypes/Geography.h +++ b/src/common/datatypes/Geography.h @@ -11,10 +11,6 @@ #include #include -#include -#include -#include - #include "common/base/StatusOr.h" #include "common/datatypes/Value.h" #include "common/geo/io/Geometry.h"