From 1944639f903e2349939e5e3f3453aea0ba5d750b Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Fri, 3 Dec 2021 17:41:39 +0800 Subject: [PATCH] support s2 index params --- src/clients/meta/MetaClient.cpp | 28 +-- src/clients/meta/MetaClient.h | 30 ++-- src/common/datatypes/Geography.h | 2 +- src/common/geo/GeoIndex.cpp | 12 +- src/common/geo/GeoIndex.h | 5 +- src/common/geo/test/GeoIndexTest.cpp | 161 +++++++++++++++++- src/common/utils/IndexKeyUtils.cpp | 12 +- src/common/utils/IndexKeyUtils.h | 22 ++- src/common/utils/test/IndexKeyUtilsTest.cpp | 115 +++++++++++-- .../executor/maintain/EdgeIndexExecutor.cpp | 4 +- .../executor/maintain/TagIndexExecutor.cpp | 4 +- .../rule/GeoPredicateIndexScanBaseRule.cpp | 17 +- src/graph/planner/plan/Maintain.cpp | 4 +- src/graph/planner/plan/Maintain.h | 48 ++---- src/graph/util/IndexUtil.cpp | 29 ++++ src/graph/util/IndexUtil.h | 3 + src/graph/util/SchemaUtil.cpp | 31 ---- src/graph/util/ToJson.cpp | 14 ++ src/graph/util/ToJson.h | 2 + src/graph/validator/MaintainValidator.cpp | 20 +-- src/graph/validator/MaintainValidator.h | 9 +- src/interface/meta.thrift | 20 ++- .../index/CreateEdgeIndexProcessor.cpp | 11 +- .../index/CreateTagIndexProcessor.cpp | 11 +- src/parser/MaintainSentences.h | 4 +- src/parser/parser.yy | 6 +- src/storage/exec/UpdateNode.h | 4 +- src/storage/mutate/AddEdgesProcessor.cpp | 2 +- src/storage/mutate/AddVerticesProcessor.cpp | 2 +- src/storage/mutate/DeleteEdgesProcessor.cpp | 2 +- src/storage/test/IndexScanLimitTest.cpp | 30 ++-- src/storage/test/IndexTest.cpp | 149 ++++++++++++++-- src/storage/test/IndexTestUtil.h | 7 +- src/storage/test/IndexWriteTest.cpp | 5 +- 34 files changed, 599 insertions(+), 226 deletions(-) diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index 45c7de5645a..40d0d831201 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -1712,24 +1712,14 @@ folly::Future> MetaClient::createTagIndex(GraphSpaceID spaceID std::string tagName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2_max_level, - const int* s2_max_cells) { + cpp2::IndexParams indexParams) { cpp2::CreateTagIndexReq req; req.set_space_id(spaceID); req.set_index_name(std::move(indexName)); req.set_tag_name(std::move(tagName)); req.set_fields(std::move(fields)); req.set_if_not_exists(ifNotExists); - if (comment != nullptr) { - req.set_comment(*comment); - } - if (s2_max_level != nullptr) { - req.set_s2_max_level(*s2_max_level); - } - if (s2_max_cells != nullptr) { - req.set_s2_max_cells(*s2_max_cells); - } + req.set_index_params(std::move(indexParams)); folly::Promise> promise; auto future = promise.getFuture(); @@ -1832,24 +1822,14 @@ folly::Future> MetaClient::createEdgeIndex( std::string edgeName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2_max_level, - const int* s2_max_cells) { + cpp2::IndexParams indexParams) { cpp2::CreateEdgeIndexReq req; req.set_space_id(spaceID); req.set_index_name(std::move(indexName)); req.set_edge_name(std::move(edgeName)); req.set_fields(std::move(fields)); req.set_if_not_exists(ifNotExists); - if (comment != nullptr) { - req.set_comment(*comment); - } - if (s2_max_level != nullptr) { - req.set_s2_max_level(*s2_max_level); - } - if (s2_max_cells != nullptr) { - req.set_s2_max_cells(*s2_max_cells); - } + req.set_index_params(std::move(indexParams)); folly::Promise> promise; auto future = promise.getFuture(); diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h index 90f1968a1f7..275785cb4e4 100644 --- a/src/clients/meta/MetaClient.h +++ b/src/clients/meta/MetaClient.h @@ -311,14 +311,13 @@ class MetaClient { bool ifExists = false); // Operations for index - folly::Future> createTagIndex(GraphSpaceID spaceID, - std::string indexName, - std::string tagName, - std::vector fields, - bool ifNotExists = false, - const std::string* comment = nullptr, - const int* s2_max_level = nullptr, - const int* s2_max_cells = nullptr); + folly::Future> createTagIndex( + GraphSpaceID spaceID, + std::string indexName, + std::string tagName, + std::vector fields, + bool ifNotExists = false, + meta::cpp2::IndexParams indexParams = cpp2::IndexParams{}); // Remove the define of tag index folly::Future> dropTagIndex(GraphSpaceID spaceId, @@ -333,14 +332,13 @@ class MetaClient { folly::Future>> listTagIndexStatus(GraphSpaceID spaceId); - folly::Future> createEdgeIndex(GraphSpaceID spaceID, - std::string indexName, - std::string edgeName, - std::vector fields, - bool ifNotExists = false, - const std::string* comment = nullptr, - const int* s2_max_level = nullptr, - const int* s2_max_cells = nullptr); + folly::Future> createEdgeIndex( + GraphSpaceID spaceID, + std::string indexName, + std::string edgeName, + std::vector fields, + bool ifNotExists = false, + cpp2::IndexParams indexParams = cpp2::IndexParams{}); // Remove the definition of edge index folly::Future> dropEdgeIndex(GraphSpaceID spaceId, diff --git a/src/common/datatypes/Geography.h b/src/common/datatypes/Geography.h index 45315df611b..cea5bd9c82e 100644 --- a/src/common/datatypes/Geography.h +++ b/src/common/datatypes/Geography.h @@ -141,7 +141,7 @@ struct Geography { bool needNormalize = false, bool verifyValidity = false); - Geography() {} + Geography() = default; Geography(const Point& v) : geo_(v) {} // NOLINT Geography(Point&& v) : geo_(std::move(v)) {} // NOLINT Geography(const LineString& v) : geo_(v) {} // NOLINT diff --git a/src/common/geo/GeoIndex.cpp b/src/common/geo/GeoIndex.cpp index 1d7e3249ef7..e17f266df48 100644 --- a/src/common/geo/GeoIndex.cpp +++ b/src/common/geo/GeoIndex.cpp @@ -30,7 +30,17 @@ namespace nebula { namespace geo { -nebula::storage::cpp2::IndexColumnHint ScanRange::toIndexColumnHint() { +bool ScanRange::operator==(const ScanRange& rhs) const { + if (isRangeScan != rhs.isRangeScan) { + return false; + } + if (isRangeScan) { + return rangeMin == rhs.rangeMin && rangeMax == rhs.rangeMax; + } + return rangeMin == rhs.rangeMin; +} + +nebula::storage::cpp2::IndexColumnHint ScanRange::toIndexColumnHint() const { nebula::storage::cpp2::IndexColumnHint hint; // column_name should be set by the caller if (isRangeScan) { diff --git a/src/common/geo/GeoIndex.h b/src/common/geo/GeoIndex.h index deaff94d028..fa2a6781cb8 100644 --- a/src/common/geo/GeoIndex.h +++ b/src/common/geo/GeoIndex.h @@ -52,7 +52,9 @@ struct ScanRange { explicit ScanRange(uint64_t v) : rangeMin(v), isRangeScan(false) {} - nebula::storage::cpp2::IndexColumnHint toIndexColumnHint(); + bool operator==(const ScanRange& rhs) const; + + nebula::storage::cpp2::IndexColumnHint toIndexColumnHint() const; }; class GeoIndex { @@ -82,6 +84,7 @@ class GeoIndex { private: RegionCoverParams rcParams_; + // pointsOnly_ indicates whether the indexed column only has points. // For the column Geography(Point), we don't need to build ancestor cells bool pointsOnly_{false}; }; diff --git a/src/common/geo/test/GeoIndexTest.cpp b/src/common/geo/test/GeoIndexTest.cpp index fcd377b7770..90801a12835 100644 --- a/src/common/geo/test/GeoIndexTest.cpp +++ b/src/common/geo/test/GeoIndexTest.cpp @@ -5,6 +5,7 @@ #include +#include #include #include "common/base/Base.h" @@ -13,22 +14,23 @@ namespace nebula { namespace geo { -std::vector toUint64Vector(std::vector expect) { - auto reinterpretInt64AsUint64 = [](int64_t i) -> uint64_t { - const char* c = reinterpret_cast(&i); - uint64_t u = *reinterpret_cast(c); - return u; - }; +uint64_t asUint64(int64_t i) { + const char* c = reinterpret_cast(&i); + uint64_t u = *reinterpret_cast(c); + return u; +} +std::vector toUint64Vector(std::vector expect) { std::vector transformedExpect; transformedExpect.reserve(expect.size()); for (int64_t i : expect) { - transformedExpect.push_back(reinterpretInt64AsUint64(i)); + transformedExpect.push_back(asUint64(i)); } return transformedExpect; } // The tested wkt data is generated by https://clydedacruz.github.io/openstreetmap-wkt-playground/ +// And the expected result comes from BigQuery TEST(indexCells, point) { geo::RegionCoverParams rc; geo::GeoIndex geoIndex(rc); @@ -105,6 +107,7 @@ TEST(indexCells, lineString) { EXPECT_EQ(toUint64Vector(expect), cells); } } + TEST(indexCells, polygon) { geo::RegionCoverParams rc; geo::GeoIndex geoIndex(rc); @@ -172,6 +175,150 @@ TEST(indexCells, polygon) { } } +TEST(indexCells, tunningRegionCoverParamsForPoint) { + // TODO(jie) +} +TEST(indexCells, tunningRegionCoverParamsForLineString) { + // TODO(jie) +} +TEST(indexCells, tunningRegionCoverParamsForPolygon) { + // TODO(jie) +} + +TEST(intersects, point) { + geo::RegionCoverParams rc(0, 30, 8); + { + bool pointsOnly = false; + geo::GeoIndex geoIndex(rc, pointsOnly); + auto point = Geography::fromWKT("POINT(1.0 1.0)").value(); + auto got = geoIndex.intersects(point); + + std::vector expect; + auto cells = toUint64Vector({1153277837650709461}); + expect.reserve(cells.size()); + for (uint64_t cellId : cells) { + expect.emplace_back(cellId); + } + + EXPECT_EQ(expect, got); + } + { + bool pointsOnly = false; // The indexed geo column only has points; + geo::GeoIndex geoIndex(rc, pointsOnly); + auto point = Geography::fromWKT("POINT(1.0 1.0)").value(); + auto got = geoIndex.intersects(point); + + std::vector expect; + auto cells = toUint64Vector({1153277837650709461}); + expect.reserve(cells.size()); + for (uint64_t cellId : cells) { + expect.emplace_back(cellId); + } + + EXPECT_EQ(expect, got); + } +} + +TEST(intersects, lineString) { + geo::RegionCoverParams rc(0, 30, 8); + { + bool pointsOnly = false; + geo::GeoIndex geoIndex(rc, pointsOnly); + auto line = Geography::fromWKT("LINESTRING(68.9 48.9,76.1 35.5,125.7 28.2)").value(); + auto got = geoIndex.intersects(line); + + std::vector expect; + auto cells = toUint64Vector({3765009288481734656, + 3818771009033469952, + 3909124476557590528, + 3928264774973915136, + 4017210867614482432, + 4053239664633446400, + 4089268461652410368, + 4773815605012725760}); + expect.reserve(cells.size()); + for (uint64_t cellId : cells) { + expect.emplace_back(cellId); + } + + EXPECT_EQ(expect, got); + } + { + bool pointsOnly = true; // The indexed geo column only has points; + geo::GeoIndex geoIndex(rc, pointsOnly); + auto line = Geography::fromWKT("POINT(1.0 1.0)").value(); + auto got = geoIndex.intersects(line); + + std::vector expect; + auto cells = toUint64Vector({3765009288481734656, + 3818771009033469952, + 3909124476557590528, + 3928264774973915136, + 4017210867614482432, + 4053239664633446400, + 4089268461652410368, + 4773815605012725760}); + expect.reserve(cells.size()); + for (uint64_t cellId : cells) { + expect.emplace_back(cellId); + } + + EXPECT_EQ(expect, got); + } +} + +TEST(intersects, polygon) { + geo::RegionCoverParams rc(0, 30, 8); + { + bool pointsOnly = false; + geo::GeoIndex geoIndex(rc, pointsOnly); + auto point = + Geography::fromWKT("POLYGON((102.5 33.5, 110.6 36.9,113.6 30.4,102.7 27.3,102.5 33.5))") + .value(); + auto got = geoIndex.intersects(point); + + std::vector expect; + auto cells = toUint64Vector({3759379788947521536, + 3879094614979772416, + 3915809507254468608, + 3917005775905488896, + 3922635275439702016, + 3931642474694443008, + 3949656873203924992, + 3958664072458665984}); + expect.reserve(cells.size()); + for (uint64_t cellId : cells) { + expect.emplace_back(cellId); + } + + EXPECT_EQ(expect, got); + } + { + bool pointsOnly = false; // The indexed geo column only has points; + geo::GeoIndex geoIndex(rc, pointsOnly); + auto point = + Geography::fromWKT("POLYGON((102.5 33.5, 110.6 36.9,113.6 30.4,102.7 27.3,102.5 33.5))") + .value(); + auto got = geoIndex.intersects(point); + + std::vector expect; + auto cells = toUint64Vector({3759379788947521536, + 3879094614979772416, + 3915809507254468608, + 3917005775905488896, + 3922635275439702016, + 3931642474694443008, + 3949656873203924992, + 3958664072458665984}); + expect.reserve(cells.size()); + for (uint64_t cellId : cells) { + expect.emplace_back(cellId); + } + + EXPECT_EQ(expect, got); + } +} + } // namespace geo } // namespace nebula diff --git a/src/common/utils/IndexKeyUtils.cpp b/src/common/utils/IndexKeyUtils.cpp index 15d4634d08a..b0528707929 100644 --- a/src/common/utils/IndexKeyUtils.cpp +++ b/src/common/utils/IndexKeyUtils.cpp @@ -7,6 +7,8 @@ #include +#include "common/geo/GeoIndex.h" + namespace nebula { // static @@ -53,8 +55,14 @@ std::vector IndexKeyUtils::encodeValues(std::vector&& values const auto& value = values.back(); if (!value.isNull()) { DCHECK(value.type() == Value::Type::GEOGRAPHY); - indexes = encodeGeography( - value.getGeography(), indexItem->get_s2_max_level(), indexItem->get_s2_max_cells()); + geo::RegionCoverParams rc; + if (indexItem->get_s2_max_level()) { + rc.maxCellLevel_ = *indexItem->get_s2_max_level(); + } + if (indexItem->get_s2_max_cells()) { + rc.maxCellNum_ = *indexItem->get_s2_max_level(); + } + indexes = encodeGeography(value.getGeography(), rc); } else { nullableBitSet |= 0x8000; auto type = IndexKeyUtils::toValueType(cols.back().type.get_type()); diff --git a/src/common/utils/IndexKeyUtils.h b/src/common/utils/IndexKeyUtils.h index 44e3d366efc..e1c8f25614b 100644 --- a/src/common/utils/IndexKeyUtils.h +++ b/src/common/utils/IndexKeyUtils.h @@ -7,6 +7,7 @@ #define COMMON_UTILS_INDEXKEYUTILS_H_ #include +#include #include "codec/RowReader.h" #include "common/base/Base.h" @@ -183,6 +184,12 @@ class IndexKeyUtils final { return raw; } + static uint64_t decodeUint64(const folly::StringPiece& raw) { + auto val = *reinterpret_cast(raw.data()); + val = folly::Endian::big(val); + return val; + } + static std::string encodeRank(EdgeRanking rank) { return IndexKeyUtils::encodeInt64(rank); } static EdgeRanking decodeRank(const folly::StringPiece& raw) { @@ -301,15 +308,7 @@ class IndexKeyUtils final { } static std::vector encodeGeography(const nebula::Geography& gg, - const int* s2_max_level, - const int* s2_max_cells) { - geo::RegionCoverParams rc; - if (!s2_max_level) { - rc.maxCellLevel_ = *s2_max_level; - } - if (!s2_max_cells) { - rc.maxCellNum_ = *s2_max_cells; - } + const geo::RegionCoverParams& rc) { geo::GeoIndex geoIndex(rc); auto cellIds = geoIndex.indexCells(gg); std::vector bufs; @@ -320,6 +319,11 @@ class IndexKeyUtils final { return bufs; } + // NOTE(jie): The decoded data is not the original Geography data, but the uint64 type S2CellID. + // decodeValue() should not call this function, it should turn for the data table instead. + // It's only used for tests. + static uint64_t decodeGeography(const folly::StringPiece& raw) { return decodeUint64(raw); } + static nebula::DateTime decodeDateTime(const folly::StringPiece& raw) { int16_t year = *reinterpret_cast(raw.data()); int8_t month = *reinterpret_cast(raw.data() + sizeof(int16_t)); diff --git a/src/common/utils/test/IndexKeyUtilsTest.cpp b/src/common/utils/test/IndexKeyUtilsTest.cpp index 79b298bf2b3..b02ce630ce8 100644 --- a/src/common/utils/test/IndexKeyUtilsTest.cpp +++ b/src/common/utils/test/IndexKeyUtilsTest.cpp @@ -5,7 +5,12 @@ #include +#include + #include "common/base/Base.h" +#include "common/datatypes/DataSet.h" +#include "common/datatypes/Geography.h" +#include "common/geo/GeoIndex.h" #include "common/utils/IndexKeyUtils.h" namespace nebula { @@ -96,6 +101,18 @@ bool evalDateTime(nebula::DateTime val) { return val == res.getDateTime(); } +std::vector evalGeography(nebula::Geography val) { + geo::RegionCoverParams rc(0, 30, 8); + auto vals = IndexKeyUtils::encodeGeography(val, rc); + std::vector got; + for (auto& str : vals) { + auto res = IndexKeyUtils::decodeGeography(str); + got.emplace_back(res); + } + // Return the actual cellIds + return got; +} + TEST(IndexKeyUtilsTest, encodeValue) { EXPECT_TRUE(evalInt64(1)); EXPECT_TRUE(evalInt64(0)); @@ -135,6 +152,63 @@ TEST(IndexKeyUtilsTest, encodeValue) { EXPECT_TRUE(evalDateTime(dt)); } +TEST(IndexKeyUtilsTest, encodeGeography) { + // The expected result comes from BigQuery + auto toUint64Vector = [](std::vector expect) { + auto asUint64 = [](int64_t i) -> uint64_t { + const char* c = reinterpret_cast(&i); + uint64_t u = *reinterpret_cast(c); + return u; + }; + + std::vector transformedExpect; + transformedExpect.reserve(expect.size()); + for (int64_t i : expect) { + transformedExpect.push_back(asUint64(i)); + } + return transformedExpect; + }; + + auto pt = Geography::fromWKT("POINT(108.1 32.5)").value(); + EXPECT_EQ(toUint64Vector({3929814733111767011}), evalGeography(pt)); + + auto ls = Geography::fromWKT("LINESTRING(68.9 48.9,76.1 35.5,125.7 28.2)").value(); + EXPECT_EQ(toUint64Vector({3765009288481734656, + 3818771009033469952, + 3909124476557590528, + 3928264774973915136, + 4017210867614482432, + 4053239664633446400, + 4089268461652410368, + 4773815605012725760}), + evalGeography(ls)); + + auto pg1 = + Geography::fromWKT("POLYGON((102.5 33.5, 110.6 36.9,113.6 30.4,102.7 27.3,102.5 33.5))") + .value(); + EXPECT_EQ(toUint64Vector({3759379788947521536, + 3879094614979772416, + 3915809507254468608, + 3917005775905488896, + 3922635275439702016, + 3931642474694443008, + 3949656873203924992, + 3958664072458665984}), + evalGeography(pg1)); + + auto pg2 = + Geography::fromWKT("POLYGON((72.2 54.6,134.6 54.6,134.6 18.2,72.2 18.2,72.2 54.6))").value(); + EXPECT_EQ(toUint64Vector({3746994889972252672, + 4107282860161892352, + 4183844053827190784, + 4192851253081931776, + 4305441243766194176, + 4827858800541171712, + 6701356245527298048, + 6845471433603153920}), + evalGeography(pg2)); +} + TEST(IndexKeyUtilsTest, encodeDouble) { EXPECT_TRUE(evalDouble(100.5)); EXPECT_TRUE(evalDouble(200.5)); @@ -225,7 +299,9 @@ TEST(IndexKeyUtilsTest, nullableValue) { cols.emplace_back(nullCol(folly::stringPrintf("col%ld", j), PropertyType::BOOL)); } // TODO(jie) Add index key tests for geography - auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); u_short s = 0xfc00; /* the binary is '11111100 00000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); @@ -240,7 +316,9 @@ TEST(IndexKeyUtilsTest, nullableValue) { for (int64_t j = 1; j <= 2; j++) { cols.emplace_back(nullCol(folly::stringPrintf("col%ld", j), PropertyType::BOOL)); } - auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); u_short s = 0x4000; /* the binary is '01000000 00000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); @@ -255,7 +333,9 @@ TEST(IndexKeyUtilsTest, nullableValue) { for (int64_t j = 1; j <= 2; j++) { cols.emplace_back(nullCol(folly::stringPrintf("col%ld", j), PropertyType::BOOL)); } - auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); u_short s = 0x0000; /* the binary is '01000000 00000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); @@ -269,8 +349,9 @@ TEST(IndexKeyUtilsTest, nullableValue) { values.emplace_back(Value(NullType::__NULL__)); cols.emplace_back(nullCol(folly::stringPrintf("col%ld", i), PropertyType::INT64)); } - - auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); u_short s = 0xfff0; /* the binary is '11111111 11110000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); @@ -308,7 +389,9 @@ TEST(IndexKeyUtilsTest, nullableValue) { cols.emplace_back(nullCol(folly::stringPrintf("col_%ld_%ld", i, j), types[j])); } } - auto raws = IndexKeyUtils::encodeValues(std::move(values), cols); + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); u_short s = 0xaaa0; /* the binary is '10101010 10100000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); @@ -322,7 +405,9 @@ TEST(IndexKeyUtilsTest, nullableValue) { values.emplace_back(Value(NullType::__NULL__)); cols.emplace_back(nullCol(folly::stringPrintf("col%ld", i), PropertyType::BOOL)); } - auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); u_short s = 0xff80; /* the binary is '11111111 10000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); @@ -418,9 +503,11 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { }; auto expected = vertices; + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); std::vector indexKeys; for (auto& row : vertices) { - auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); + auto values = IndexKeyUtils::encodeValues(std::move(row.second), indexItem.get()); ASSERT_EQ(indexValueSize, values[0].size()); auto keys = IndexKeyUtils::vertexIndexKeys(vIdLen, partId, indexId, row.first, std::move(values)); @@ -440,9 +527,11 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { }; auto expected = edges; + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); std::vector indexKeys; for (auto& row : edges) { - auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); + auto values = IndexKeyUtils::encodeValues(std::move(row.second), indexItem.get()); ASSERT_EQ(indexValueSize, values[0].size()); auto keys = IndexKeyUtils::edgeIndexKeys( @@ -476,9 +565,11 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { {"9", {null, null, null, null, null, null}}}; auto expected = vertices; + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); std::vector indexKeys; for (auto& row : vertices) { - auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); + auto values = IndexKeyUtils::encodeValues(std::move(row.second), indexItem.get()); ASSERT_EQ(indexValueSize, values[0].size()); auto keys = IndexKeyUtils::vertexIndexKeys(vIdLen, partId, indexId, row.first, std::move(values)); @@ -503,9 +594,11 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { {"9", {null, null, null, null, null, null}}}; auto expected = edges; + auto indexItem = std::make_unique(); + indexItem->set_fields(cols); std::vector indexKeys; for (auto& row : edges) { - auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); + auto values = IndexKeyUtils::encodeValues(std::move(row.second), indexItem.get()); ASSERT_EQ(indexValueSize, values[0].size()); auto keys = IndexKeyUtils::edgeIndexKeys( vIdLen, partId, indexId, row.first, 0, row.first, std::move(values)); diff --git a/src/graph/executor/maintain/EdgeIndexExecutor.cpp b/src/graph/executor/maintain/EdgeIndexExecutor.cpp index 40f7ba0b163..a4e16c2d2f0 100644 --- a/src/graph/executor/maintain/EdgeIndexExecutor.cpp +++ b/src/graph/executor/maintain/EdgeIndexExecutor.cpp @@ -23,9 +23,7 @@ folly::Future CreateEdgeIndexExecutor::execute() { ceiNode->getSchemaName(), ceiNode->getFields(), ceiNode->getIfNotExists(), - ceiNode->getComment(), - ceiNode->getS2MaxLevel(), - ceiNode->getS2MaxCells()) + ceiNode->getIndexParams()) .via(runner()) .thenValue([ceiNode, spaceId](StatusOr resp) { if (!resp.ok()) { diff --git a/src/graph/executor/maintain/TagIndexExecutor.cpp b/src/graph/executor/maintain/TagIndexExecutor.cpp index f4a0d99701c..dc1cbec87c0 100644 --- a/src/graph/executor/maintain/TagIndexExecutor.cpp +++ b/src/graph/executor/maintain/TagIndexExecutor.cpp @@ -23,9 +23,7 @@ folly::Future CreateTagIndexExecutor::execute() { ctiNode->getSchemaName(), ctiNode->getFields(), ctiNode->getIfNotExists(), - ctiNode->getComment(), - ctiNode->getS2MaxLevel(), - ctiNode->getS2MaxCells()) + ctiNode->getIndexParams()) .via(runner()) .thenValue([ctiNode, spaceId](StatusOr resp) { if (!resp.ok()) { diff --git a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp index be019f7356f..7d262c58387 100644 --- a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp +++ b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp @@ -85,18 +85,17 @@ StatusOr GeoPredicateIndexScanBaseRule::transform( DCHECK_EQ(fields.size(), 1); // geo field auto& geoField = fields.back(); auto& geoColumnTypeDef = geoField.get_type(); - bool indexedGeoColumnOnlyHasPoints = - geoColumnTypeDef.geo_shape_ref().has_value() && - geoColumnTypeDef.geo_shape_ref().value() == meta::cpp2::GeoShape::POINT; + bool isPointColumn = geoColumnTypeDef.geo_shape_ref().has_value() && + geoColumnTypeDef.geo_shape_ref().value() == meta::cpp2::GeoShape::POINT; geo::RegionCoverParams rc; - if (indexItem.s2_max_level_ref().has_value()) { - rc.maxCellLevel_ = indexItem.s2_max_level_ref().value(); + if (indexItem->s2_max_level_ref().has_value()) { + rc.maxCellLevel_ = indexItem->s2_max_level_ref().value(); } - if (indexItem.s2_max_cells_ref().has_value()) { - rc.maxCellNum_ = indexItem.s2_max_cells_ref().value(); + if (indexItem->s2_max_cells_ref().has_value()) { + rc.maxCellNum_ = indexItem->s2_max_cells_ref().value(); } - geo::GeoIndex geoIndex(rc, indexedGeoColumnOnlyHasPoints); + geo::GeoIndex geoIndex(rc, isPointColumn); std::vector scanRanges; if (geoPredicateName == "st_intersects") { scanRanges = geoIndex.intersects(geog); @@ -115,7 +114,7 @@ StatusOr GeoPredicateIndexScanBaseRule::transform( } std::vector idxCtxs; idxCtxs.reserve(scanRanges.size()); - auto fieldName = fields.back().get_name(); + auto fieldName = geoField.get_name(); for (auto& scanRange : scanRanges) { IndexQueryContext ictx; auto indexColumnHint = scanRange.toIndexColumnHint(); diff --git a/src/graph/planner/plan/Maintain.cpp b/src/graph/planner/plan/Maintain.cpp index 287025748a6..10327c22955 100644 --- a/src/graph/planner/plan/Maintain.cpp +++ b/src/graph/planner/plan/Maintain.cpp @@ -52,9 +52,7 @@ std::unique_ptr CreateIndexNode::explain() const { } addDescription("fields", folly::toJson(util::toJson(fields)), desc.get()); addDescription("ifNotExists", folly::to(ifNotExists_), desc.get()); - addDescription("comment", folly::toJson(util::toJson(*comment_)), desc.get()); - addDescription("s2_max_level", folly::toJson(util::toJson(*s2MaxLevel_)), desc.get()); - addDescription("s2_max_cells", folly::toJson(util::toJson(*s2MaxCells_)), desc.get()); + addDescription("indexParams", folly::toJson(util::toJson(indexParams_)), desc.get()); return desc; } diff --git a/src/graph/planner/plan/Maintain.h b/src/graph/planner/plan/Maintain.h index 0ecff03e580..ffacb464f6b 100644 --- a/src/graph/planner/plan/Maintain.h +++ b/src/graph/planner/plan/Maintain.h @@ -300,17 +300,13 @@ class CreateIndexNode : public SingleDependencyNode { std::string indexName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2MaxLevel, - const int* s2MaxCells) + meta::cpp2::IndexParams indexParams) : SingleDependencyNode(qctx, kind, input), schemaName_(std::move(schemaName)), indexName_(std::move(indexName)), fields_(std::move(fields)), ifNotExists_(ifNotExists), - comment_(comment), - s2MaxLevel_(s2MaxLevel), - s2MaxCells_(s2MaxCells) {} + indexParams_(indexParams) {} public: const std::string& getSchemaName() const { return schemaName_; } @@ -321,9 +317,7 @@ class CreateIndexNode : public SingleDependencyNode { bool getIfNotExists() const { return ifNotExists_; } - const std::string* getComment() const { return comment_; } - const int* getS2MaxLevel() const { return s2MaxLevel_; } - const int* getS2MaxCells() const { return s2MaxCells_; } + meta::cpp2::IndexParams getIndexParams() const { return indexParams_; } std::unique_ptr explain() const override; @@ -332,9 +326,7 @@ class CreateIndexNode : public SingleDependencyNode { std::string indexName_; std::vector fields_; bool ifNotExists_; - const std::string* comment_; - const int* s2MaxLevel_; - const int* s2MaxCells_; + meta::cpp2::IndexParams indexParams_; }; class CreateTagIndex final : public CreateIndexNode { @@ -345,18 +337,14 @@ class CreateTagIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2MaxLevel, - const int* s2MaxCells) { + meta::cpp2::IndexParams indexParams) { return qctx->objPool()->add(new CreateTagIndex(qctx, input, std::move(tagName), std::move(indexName), std::move(fields), ifNotExists, - comment, - s2MaxLevel, - s2MaxCells)); + indexParams)); } private: @@ -366,9 +354,7 @@ class CreateTagIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2MaxLevel, - const int* s2MaxCells) + meta::cpp2::IndexParams indexParams) : CreateIndexNode(qctx, input, Kind::kCreateTagIndex, @@ -376,9 +362,7 @@ class CreateTagIndex final : public CreateIndexNode { std::move(indexName), std::move(fields), ifNotExists, - comment, - s2MaxLevel, - s2MaxCells) {} + indexParams) {} }; class CreateEdgeIndex final : public CreateIndexNode { @@ -389,18 +373,14 @@ class CreateEdgeIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2MaxLevel, - const int* s2MaxCells) { + meta::cpp2::IndexParams indexParams) { return qctx->objPool()->add(new CreateEdgeIndex(qctx, input, std::move(edgeName), std::move(indexName), std::move(fields), ifNotExists, - comment, - s2MaxLevel, - s2MaxCells)); + indexParams)); } private: @@ -410,9 +390,7 @@ class CreateEdgeIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - const std::string* comment, - const int* s2MaxLevel, - const int* s2MaxCells) + meta::cpp2::IndexParams indexParams) : CreateIndexNode(qctx, input, Kind::kCreateEdgeIndex, @@ -420,9 +398,7 @@ class CreateEdgeIndex final : public CreateIndexNode { std::move(indexName), std::move(fields), ifNotExists, - comment, - s2MaxLevel, - s2MaxCells) {} + indexParams) {} }; class DescIndexNode : public SingleDependencyNode { diff --git a/src/graph/util/IndexUtil.cpp b/src/graph/util/IndexUtil.cpp index 2ff3dfd6f4d..c28815fb896 100644 --- a/src/graph/util/IndexUtil.cpp +++ b/src/graph/util/IndexUtil.cpp @@ -23,6 +23,35 @@ Status IndexUtil::validateColumns(const std::vector &fields) { return Status::OK(); } +// static +Status IndexUtil::validateIndexParams(const std::vector ¶ms, + meta::cpp2::IndexParams &indexParams) { + for (auto *param : params) { + switch (param->getParamType()) { + case IndexParamItem::COMMENT: { + auto ret = param->getComment(); + NG_RETURN_IF_ERROR(ret); + indexParams.set_comment(std::move(ret).value()); + break; + } + case IndexParamItem::S2_MAX_LEVEL: { + auto ret2 = param->getS2MaxLevel(); + NG_RETURN_IF_ERROR(ret2); + indexParams.set_s2_max_level(std::move(ret2).value()); + break; + } + case IndexParamItem::S2_MAX_CELLS: { + auto ret3 = param->getS2MaxCells(); + NG_RETURN_IF_ERROR(ret3); + indexParams.set_s2_max_cells(std::move(ret3).value()); + break; + } + } + } + + return Status::OK(); +} + StatusOr IndexUtil::toDescIndex(const meta::cpp2::IndexItem &indexItem) { DataSet dataSet({"Field", "Type"}); for (auto &col : indexItem.get_fields()) { diff --git a/src/graph/util/IndexUtil.h b/src/graph/util/IndexUtil.h index c1f08360a66..22100690363 100644 --- a/src/graph/util/IndexUtil.h +++ b/src/graph/util/IndexUtil.h @@ -19,6 +19,9 @@ class IndexUtil final { static Status validateColumns(const std::vector &fields); + static Status validateIndexParams(const std::vector ¶ms, + meta::cpp2::IndexParams &indexParams); + static StatusOr toDescIndex(const meta::cpp2::IndexItem &indexItem); static StatusOr toShowCreateIndex(bool isTagIndex, diff --git a/src/graph/util/SchemaUtil.cpp b/src/graph/util/SchemaUtil.cpp index 19f94487284..1c3c095a284 100644 --- a/src/graph/util/SchemaUtil.cpp +++ b/src/graph/util/SchemaUtil.cpp @@ -53,37 +53,6 @@ Status SchemaUtil::validateProps(const std::vector &schemaProp return Status::OK(); } -// static -Status SchemaUtil::validateIndexParams(const std::vector ¶ms, - const std::string *&comment, - const int *&s2MaxLevel, - const int *&s2MaxCells) { - for (auto *param : params) { - switch (param->getParamType()) { - case IndexParamItem::COMMENT: { - auto ret = param->getComment(); - NG_RETURN_IF_ERROR(ret); - comment = std::move(ret).value(); - break; - } - case IndexParamItem::S2_MAX_LEVEL: { - auto ret2 = param->getS2MaxLevel(); - NG_RETURN_IF_ERROR(ret2); - s2MaxLevel = std::move(ret2).value(); - break; - } - case IndexParamItem::S2_MAX_CELLS: { - auto ret3 = param->getS2MaxCells(); - NG_RETURN_IF_ERROR(ret3); - s2MaxCells = std::move(ret3).value(); - break; - } - } - } - - return Status::OK(); -} - // static std::shared_ptr SchemaUtil::generateSchemaProvider( ObjectPool *pool, const SchemaVer ver, const meta::cpp2::Schema &schema) { diff --git a/src/graph/util/ToJson.cpp b/src/graph/util/ToJson.cpp index e3ec2d5dcbe..16d0b23e7d1 100644 --- a/src/graph/util/ToJson.cpp +++ b/src/graph/util/ToJson.cpp @@ -96,6 +96,20 @@ folly::dynamic toJson(const meta::cpp2::SchemaProp &prop) { return object; } +folly::dynamic toJson(const meta::cpp2::IndexParams ¶ms) { + folly::dynamic object = folly::dynamic::object(); + if (params.comment_ref().has_value()) { + object.insert("comment", *params.comment_ref()); + } + if (params.s2_max_level_ref().has_value()) { + object.insert("s2_max_level", *params.s2_max_level_ref()); + } + if (params.s2_max_cells_ref().has_value()) { + object.insert("s2_max_cells", *params.s2_max_cells_ref()); + } + return object; +} + folly::dynamic toJson(const meta::cpp2::AlterSchemaItem &item) { folly::dynamic json = folly::dynamic::object(); if (item.schema_ref().is_set()) { diff --git a/src/graph/util/ToJson.h b/src/graph/util/ToJson.h index 01f39a54e10..eed062480a7 100644 --- a/src/graph/util/ToJson.h +++ b/src/graph/util/ToJson.h @@ -25,6 +25,7 @@ class AlterSchemaItem; class ColumnDef; class Schema; class SchemaProp; +class IndexParams; } // namespace cpp2 } // namespace meta @@ -68,6 +69,7 @@ folly::dynamic toJson(const meta::cpp2::SpaceDesc &desc); folly::dynamic toJson(const meta::cpp2::ColumnDef &column); folly::dynamic toJson(const meta::cpp2::Schema &schema); folly::dynamic toJson(const meta::cpp2::SchemaProp &prop); +folly::dynamic toJson(const meta::cpp2::IndexParams &indexParams); folly::dynamic toJson(const meta::cpp2::AlterSchemaItem &item); folly::dynamic toJson(const storage::cpp2::EdgeKey &edgeKey); folly::dynamic toJson(const storage::cpp2::NewTag &tag); diff --git a/src/graph/validator/MaintainValidator.cpp b/src/graph/validator/MaintainValidator.cpp index f83e72f2b25..6c124424dc2 100644 --- a/src/graph/validator/MaintainValidator.cpp +++ b/src/graph/validator/MaintainValidator.cpp @@ -295,8 +295,10 @@ Status CreateTagIndexValidator::validateImpl() { fields_ = sentence->fields(); ifNotExist_ = sentence->isIfNotExist(); // TODO(darion) Save the index - NG_RETURN_IF_ERROR(SchemaUtil::validateIndexParams( - sentence->getIndexParams(), comment_, s2Maxlevel_, s2MaxCells_)); + auto *indexParamList = sentence->getIndexParamList(); + if (indexParamList) { + NG_RETURN_IF_ERROR(IndexUtil::validateIndexParams(indexParamList->getParams(), indexParams_)); + } return Status::OK(); } @@ -308,9 +310,7 @@ Status CreateTagIndexValidator::toPlan() { *sentence->indexName(), sentence->fields(), sentence->isIfNotExist(), - comment_, - s2MaxLevel_, - s2MaxCells_); + indexParams_); root_ = doNode; tail_ = root_; return Status::OK(); @@ -323,8 +323,10 @@ Status CreateEdgeIndexValidator::validateImpl() { fields_ = sentence->fields(); ifNotExist_ = sentence->isIfNotExist(); // TODO(darion) Save the index - NG_RETURN_IF_ERROR(SchemaUtil::validateIndexParams( - sentence->getIndexParams(), comment_, s2Maxlevel_, s2MaxCells_)); + auto *indexParamList = sentence->getIndexParamList(); + if (indexParamList) { + NG_RETURN_IF_ERROR(IndexUtil::validateIndexParams(indexParamList->getParams(), indexParams_)); + } return Status::OK(); } @@ -336,9 +338,7 @@ Status CreateEdgeIndexValidator::toPlan() { *sentence->indexName(), sentence->fields(), sentence->isIfNotExist(), - comment_, - s2MaxLevel_, - s2MaxCells_); + indexParams_); root_ = doNode; tail_ = root_; return Status::OK(); diff --git a/src/graph/validator/MaintainValidator.h b/src/graph/validator/MaintainValidator.h index 9a6ae22a3eb..38c3bbab437 100644 --- a/src/graph/validator/MaintainValidator.h +++ b/src/graph/validator/MaintainValidator.h @@ -9,6 +9,7 @@ #include "clients/meta/MetaClient.h" #include "graph/context/ast/QueryAstContext.h" #include "graph/validator/Validator.h" +#include "interface/gen-cpp2/meta_types.h" #include "parser/AdminSentences.h" namespace nebula { @@ -157,9 +158,7 @@ class CreateTagIndexValidator final : public Validator { std::string index_; std::vector fields_; bool ifNotExist_; - const std::string* comment_; - const int* s2MaxLevel_; - const int* s2MaxCells_; + meta::cpp2::IndexParams indexParams_; }; class CreateEdgeIndexValidator final : public Validator { @@ -176,9 +175,7 @@ class CreateEdgeIndexValidator final : public Validator { std::string index_; std::vector fields_; bool ifNotExist_; - const std::string* comment_; - const int* s2MaxLevel_; - const int* s2MaxCells_; + meta::cpp2::IndexParams indexParams_; }; class DropTagIndexValidator final : public Validator { diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index f8e0b6b79c8..196468b2310 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -141,15 +141,21 @@ struct EdgeItem { 4: Schema schema, } +struct IndexParams { + 1: optional binary comment, + 2: optional i32 s2_max_level, + 3: optional i32 s2_max_cells, +} + struct IndexItem { 1: common.IndexID index_id, 2: binary index_name, 3: common.SchemaID schema_id 4: binary schema_name, 5: list fields, - 6: optional comment, - 7: optional s2_max_level, - 8: optional s2_max_cells, + 6: optional binary comment, + 7: optional i32 s2_max_level, + 8: optional i32 s2_max_cells, } enum HostStatus { @@ -586,9 +592,7 @@ struct CreateTagIndexReq { 3: binary tag_name, 4: list fields, 5: bool if_not_exists, - 6: optional comment, - 7: optional s2_max_level, - 8: optional s2_max_cells, + 6: IndexParams index_params, } struct DropTagIndexReq { @@ -624,9 +628,7 @@ struct CreateEdgeIndexReq { 3: binary edge_name, 4: list fields, 5: bool if_not_exists, - 6: optional comment, - 7: optional s2_max_level, - 8: optional s2_max_cells, + 6: IndexParams index_params, } struct DropEdgeIndexReq { diff --git a/src/meta/processors/index/CreateEdgeIndexProcessor.cpp b/src/meta/processors/index/CreateEdgeIndexProcessor.cpp index afa04f3cdc4..7d01ba5d48c 100644 --- a/src/meta/processors/index/CreateEdgeIndexProcessor.cpp +++ b/src/meta/processors/index/CreateEdgeIndexProcessor.cpp @@ -183,7 +183,16 @@ void CreateEdgeIndexProcessor::process(const cpp2::CreateEdgeIndexReq& req) { item.set_schema_id(schemaID); item.set_schema_name(edgeName); item.set_fields(std::move(columns)); - item.set_index_params(std::move(req.get_index_params())); + auto& indexParams = req.get_index_params(); + if (indexParams.comment_ref().has_value()) { + item.set_comment(indexParams.comment_ref().value()); + } + if (indexParams.s2_max_level_ref().has_value()) { + item.set_s2_max_level(indexParams.s2_max_level_ref().value()); + } + if (indexParams.s2_max_cells_ref().has_value()) { + item.set_s2_max_cells(indexParams.s2_max_cells_ref().value()); + } data.emplace_back(MetaKeyUtils::indexIndexKey(space, indexName), std::string(reinterpret_cast(&edgeIndex), sizeof(IndexID))); diff --git a/src/meta/processors/index/CreateTagIndexProcessor.cpp b/src/meta/processors/index/CreateTagIndexProcessor.cpp index 99f9ae25479..1aa914ad24c 100644 --- a/src/meta/processors/index/CreateTagIndexProcessor.cpp +++ b/src/meta/processors/index/CreateTagIndexProcessor.cpp @@ -181,7 +181,16 @@ void CreateTagIndexProcessor::process(const cpp2::CreateTagIndexReq& req) { item.set_schema_id(schemaID); item.set_schema_name(tagName); item.set_fields(std::move(columns)); - item.set_index_params(std::move(req.get_index_params())); + auto& indexParams = req.get_index_params(); + if (indexParams.comment_ref().has_value()) { + item.set_comment(indexParams.comment_ref().value()); + } + if (indexParams.s2_max_level_ref().has_value()) { + item.set_s2_max_level(indexParams.s2_max_level_ref().value()); + } + if (indexParams.s2_max_cells_ref().has_value()) { + item.set_s2_max_cells(indexParams.s2_max_cells_ref().value()); + } data.emplace_back(MetaKeyUtils::indexIndexKey(space, indexName), std::string(reinterpret_cast(&tagIndex), sizeof(IndexID))); diff --git a/src/parser/MaintainSentences.h b/src/parser/MaintainSentences.h index 5f4a6b4d030..0c219a01f4f 100644 --- a/src/parser/MaintainSentences.h +++ b/src/parser/MaintainSentences.h @@ -584,7 +584,7 @@ class CreateTagIndexSentence final : public CreateSentence { return result; } - std::vector getIndexParams() const { return indexParams_->getParams(); } + IndexParamList *getIndexParamList() const { return indexParams_.get(); } private: std::unique_ptr indexName_; @@ -627,7 +627,7 @@ class CreateEdgeIndexSentence final : public CreateSentence { return result; } - std::vector getIndexParams() const { return indexParams_->getParams(); } + IndexParamList *getIndexParamList() const { return indexParams_.get(); } private: std::unique_ptr indexName_; diff --git a/src/parser/parser.yy b/src/parser/parser.yy index f6d0c855598..2c5a8f45401 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -280,7 +280,7 @@ static constexpr size_t kCommentLengthLimit = 256; %type alter_schema_prop_list %type alter_schema_prop_item %type opt_index_param_list index_param_list -%type index_param_item +%type index_param_item %type order_factor %type order_factors %type config_module_enum @@ -2627,7 +2627,7 @@ index_param_list index_param_item : comment_prop_assignment { - $$ = new IndexParamItem(IndexParamItem::COMMENT, $1); + $$ = new IndexParamItem(IndexParamItem::COMMENT, *$1); } | KW_S2_MAX_LEVEL ASSIGN legal_integer { if ($3 < 0 || $3 > 30) { @@ -2636,7 +2636,7 @@ index_param_item $$ = new IndexParamItem(IndexParamItem::S2_MAX_LEVEL, $3); } | KW_S2_MAX_CELLS ASSIGN legal_integer { - if ($3 < 0 || $3 > 30) { + if ($3 < 1 || $3 > 32) { throw nebula::GraphParser::syntax_error(@3, "'s2_max_cells' value must be between 1 and 32 inclusive"); } $$ = new IndexParamItem(IndexParamItem::S2_MAX_CELLS, $3); diff --git a/src/storage/exec/UpdateNode.h b/src/storage/exec/UpdateNode.h index 9b83c248106..f3e7b2fb61b 100644 --- a/src/storage/exec/UpdateNode.h +++ b/src/storage/exec/UpdateNode.h @@ -420,7 +420,7 @@ class UpdateTagNode : public UpdateNode { const VertexID& vId, RowReader* reader, std::shared_ptr index) { - auto values = IndexKeyUtils::collectIndexValues(reader, item.get()); + auto values = IndexKeyUtils::collectIndexValues(reader, index.get()); if (!values.ok()) { return {}; } @@ -751,7 +751,7 @@ class UpdateEdgeNode : public UpdateNode { RowReader* reader, const cpp2::EdgeKey& edgeKey, std::shared_ptr index) { - auto values = IndexKeyUtils::collectIndexValues(reader, item.get()); + auto values = IndexKeyUtils::collectIndexValues(reader, index.get()); if (!values.ok()) { return {}; } diff --git a/src/storage/mutate/AddEdgesProcessor.cpp b/src/storage/mutate/AddEdgesProcessor.cpp index 1a1ba2fc26d..3b0e100c775 100644 --- a/src/storage/mutate/AddEdgesProcessor.cpp +++ b/src/storage/mutate/AddEdgesProcessor.cpp @@ -465,7 +465,7 @@ std::vector AddEdgesProcessor::indexKeys( RowReader* reader, const folly::StringPiece& rawKey, std::shared_ptr index) { - auto values = IndexKeyUtils::collectIndexValues(reader, item.get()); + auto values = IndexKeyUtils::collectIndexValues(reader, index.get()); if (!values.ok()) { return {}; } diff --git a/src/storage/mutate/AddVerticesProcessor.cpp b/src/storage/mutate/AddVerticesProcessor.cpp index f8a4ddb12c8..e2c1a70807f 100644 --- a/src/storage/mutate/AddVerticesProcessor.cpp +++ b/src/storage/mutate/AddVerticesProcessor.cpp @@ -326,7 +326,7 @@ std::vector AddVerticesProcessor::indexKeys( const VertexID& vId, RowReader* reader, std::shared_ptr index) { - auto values = IndexKeyUtils::collectIndexValues(reader, item.get()); + auto values = IndexKeyUtils::collectIndexValues(reader, index.get()); if (!values.ok()) { return {}; } diff --git a/src/storage/mutate/DeleteEdgesProcessor.cpp b/src/storage/mutate/DeleteEdgesProcessor.cpp index 265e5c0ecc3..47d63d24db3 100644 --- a/src/storage/mutate/DeleteEdgesProcessor.cpp +++ b/src/storage/mutate/DeleteEdgesProcessor.cpp @@ -161,7 +161,7 @@ ErrorOr DeleteEdgesProcessor::deleteEdges( return nebula::cpp2::ErrorCode::E_INVALID_DATA; } } - auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), item.get()); + auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), index.get()); if (!valuesRet.ok()) { continue; } diff --git a/src/storage/test/IndexScanLimitTest.cpp b/src/storage/test/IndexScanLimitTest.cpp index 83e678c0d4c..f4988f494a7 100644 --- a/src/storage/test/IndexScanLimitTest.cpp +++ b/src/storage/test/IndexScanLimitTest.cpp @@ -122,26 +122,28 @@ class IndexScanLimitTest : public ::testing::Test { data.emplace_back(std::move(edgeKey), val); data.emplace_back(std::move(tagKey), std::move(val)); if (indexMan_ != nullptr) { + auto indexItem = std::make_unique(); + indexItem->set_fields(genCols()); if (indexMan_->getTagIndex(spaceId, tagIndex).ok()) { - auto vertexIndexKeys = - IndexKeyUtils::vertexIndexKeys(vertexLen, - pId, - tagIndex, - vertex, - IndexKeyUtils::encodeValues({col1Val}, genCols())); + auto vertexIndexKeys = IndexKeyUtils::vertexIndexKeys( + vertexLen, + pId, + tagIndex, + vertex, + IndexKeyUtils::encodeValues({col1Val}, indexItem.get())); for (auto& vertexIndexKey : vertexIndexKeys) { data.emplace_back(std::move(vertexIndexKey), ""); } } if (indexMan_->getEdgeIndex(spaceId, edgeIndex).ok()) { - auto edgeIndexKeys = - IndexKeyUtils::edgeIndexKeys(vertexLen, - pId, - edgeIndex, - vertex, - 0, - vertex, - IndexKeyUtils::encodeValues({col1Val}, genCols())); + auto edgeIndexKeys = IndexKeyUtils::edgeIndexKeys( + vertexLen, + pId, + edgeIndex, + vertex, + 0, + vertex, + IndexKeyUtils::encodeValues({col1Val}, indexItem.get())); for (auto& edgeIndexKey : edgeIndexKeys) { data.emplace_back(std::move(edgeIndexKey), ""); } diff --git a/src/storage/test/IndexTest.cpp b/src/storage/test/IndexTest.cpp index 2b269da938f..3a4294a2a93 100644 --- a/src/storage/test/IndexTest.cpp +++ b/src/storage/test/IndexTest.cpp @@ -4,6 +4,7 @@ */ #include +#include #include #include @@ -14,6 +15,7 @@ #include "common/expression/ConstantExpression.h" #include "common/expression/PropertyExpression.h" #include "common/expression/RelationalExpression.h" +#include "common/utils/IndexKeyUtils.h" #include "common/utils/NebulaKeyUtils.h" #include "kvstore/KVEngine.h" #include "kvstore/KVIterator.h" @@ -38,7 +40,7 @@ using std::string_literals::operator""s; * 1. Vertex/Edge * 2. back to table or not * 3. different value type - * a. int/float/bool/fix_string/time/date/datetime + * a. int/float/bool/fix_string/time/date/datetime/geography * b. compound index * 4. range/prefix * a. prefix(equal) @@ -163,10 +165,18 @@ class IndexScanTest : public ::testing::Test { RowReaderWrapper reader(schema.get(), folly::StringPiece(value), schemaVer); for (size_t j = 0; j < indices.size(); j++) { auto& index = indices[j]; - auto indexValue = IndexKeyUtils::collectIndexValues(&reader, index->get_fields()).value(); - auto indexKey = IndexKeyUtils::vertexIndexKeys( - 8, 0, index->get_index_id(), std::to_string(i), std::move(indexValue))[0]; - CHECK(ret[j + 1].insert({indexKey, ""}).second); + auto indexValue = IndexKeyUtils::collectIndexValues(&reader, index.get()).value(); + auto indexKeys = IndexKeyUtils::vertexIndexKeys( + 8, 0, index->get_index_id(), std::to_string(i), std::move(indexValue)); + for (auto& indexKey : indexKeys) { + CHECK(ret[j + 1].insert({indexKey, ""}).second); + } + if (index->get_index_name() == "geo") { + LOG(INFO) << "encodeTag fro geography"; + for (auto& indexKey : indexKeys) { + LOG(INFO) << indexKey; + } + } } } return ret; @@ -189,15 +199,17 @@ class IndexScanTest : public ::testing::Test { RowReaderWrapper reader(schema.get(), folly::StringPiece(value), schemaVer); for (size_t j = 0; j < indices.size(); j++) { auto& index = indices[j]; - auto indexValue = IndexKeyUtils::collectIndexValues(&reader, index->get_fields()).value(); - auto indexKey = IndexKeyUtils::edgeIndexKeys(8, - 0, - index->get_index_id(), - std::to_string(i), - i, - std::to_string(i), - std::move(indexValue))[0]; - CHECK(ret[j + 1].insert({indexKey, ""}).second); + auto indexValue = IndexKeyUtils::collectIndexValues(&reader, index.get()).value(); + auto indexKeys = IndexKeyUtils::edgeIndexKeys(8, + 0, + index->get_index_id(), + std::to_string(i), + i, + std::to_string(i), + std::move(indexValue)); + for (auto& indexKey : indexKeys) { + CHECK(ret[j + 1].insert({indexKey, ""}).second); + } } } return ret; @@ -1730,6 +1742,115 @@ TEST_F(IndexScanTest, Date) { TEST_F(IndexScanTest, DateTime) { // TODO(hs.zhang): add unittest } + +TEST_F(IndexScanTest, Geography) { + auto rows = R"( + geography + POINT(108.1 32.5) + LINESTRING(68.9 48.9,76.1 35.5,125.7 28.2) + POLYGON((102.5 33.5, 110.6 36.9,113.6 30.4,102.7 27.3,102.5 33.5)) + POLYGON((72.2 54.6,134.6 54.6,134.6 18.2,72.2 18.2,72.2 54.6)) + )"_row; + /* Format: WKT:CellIDs. The expected result comes from BigQuery + POINT(108.1 32.5): [3929814733111767011] + LINESTRING(68.9 48.9,76.1 35.5,125.7 28.2): [3765009288481734656, 3818771009033469952, + 3909124476557590528, 3928264774973915136, 4017210867614482432, 4053239664633446400, + 4089268461652410368, 4773815605012725760] + POLYGON((102.5 33.5, 110.6 36.9,113.6 30.4,102.7 27.3,102.5 33.5)): [3759379788947521536, + 3879094614979772416, 3915809507254468608, 3917005775905488896, 3922635275439702016, + 3931642474694443008, 3949656873203924992, 3958664072458665984] + POLYGON((72.2 54.6,134.6 54.6,134.6 18.2,72.2 18.2,72.2 54.6)): [3819052484010180608, + 3963167672086036480, 4107282860161892352, 4773815605012725760] + [3746994889972252672, 4107282860161892352, 4183844053827190784, 4192851253081931776, + 4305441243766194176, 4827858800541171712, 6701356245527298048, 6845471433603153920] + */ + + auto schema = R"( + geo | geography | | false + )"_schema; + auto indices = R"( + TAG(t,1) + (i1,2):geo + )"_index(schema); + auto kv = encodeTag(rows, 1, schema, indices); + auto kvstore = std::make_unique(); + for (auto& iter : kv) { + for (auto& item : iter) { + kvstore->put(item.first, item.second); + } + } + auto check = [&](std::shared_ptr index, + const std::vector& columnHints, + const std::vector& expect, + const std::string& case_) { + auto context = makeContext(1, 0); + auto scanNode = + std::make_unique(context.get(), 0, columnHints, kvstore.get()); + IndexScanTestHelper helper; + helper.setIndex(scanNode.get(), index); + helper.setTag(scanNode.get(), schema); + InitContext initCtx; + initCtx.requiredColumns = {kVid}; + scanNode->init(initCtx); + scanNode->execute(0); + + std::vector result; + while (true) { + auto res = scanNode->next(); + ASSERT(res.success()); + if (!res.hasData()) { + break; + } + result.emplace_back(std::move(res).row()); + } + EXPECT_EQ(result, expect) << "Fail at case " << case_; + }; + auto expect = [](auto... vidList) { + std::vector ret; + std::vector value; + (value.push_back(std::to_string(vidList)), ...); + for (auto& v : value) { + Row row; + row.emplace_back(v); + ret.emplace_back(std::move(row)); + } + return ret; + }; + auto encodeCellId = [](int64_t i) -> std::string { + // First, reinterpret the int64_t as uint64_t + const char* c = reinterpret_cast(&i); + // Then, encode the uint64_t as string + uint64_t u = *reinterpret_cast(c); + return IndexKeyUtils::encodeUint64(u); + }; + // For the Geography type, there are only two cases: prefix and [x, y]. + /* Case 1: Prefix */ + { + std::vector columnHints = { + makeColumnHint("geo", encodeCellId(3929814733111767011))}; + check(indices[0], columnHints, expect(0), "case1.1"); + columnHints = {makeColumnHint("geo", encodeCellId(4773815605012725760))}; + check(indices[1], columnHints, expect(1, 3), "case1.2"); + columnHints = {makeColumnHint("geo", encodeCellId(3931642474694443008))}; + check(indices[1], columnHints, expect(2), "case1.3"); + } + /* Case 2: [x, y] */ + { + auto hint = [&encodeCellId](const char* name, int64_t begin, int64_t end) { + return std::vector{ + makeColumnHint(name, encodeCellId(begin), encodeCellId(end))}; + }; + auto columnHint = hint("geo", 3759379788947521536, 4773815605012725760); + check(indices[0], columnHint, expect(0, 1, 2, 3), "case2.1"); + columnHint = hint("geo", 3759379788947521536, 3765009288481734656); + check(indices[0], columnHint, expect(2, 1), "case2.2"); + columnHint = hint("geo", 3928264774973915136, 3928264774973915136); + check(indices[1], columnHint, expect(1), "case2.3"); + columnHint = hint("geo", 3929814733111767011, 3958664072458665984); + check(indices[1], columnHint, expect(0, 2), "case2.4"); + } +} + TEST_F(IndexScanTest, Compound) { // TODO(hs.zhang): add unittest } diff --git a/src/storage/test/IndexTestUtil.h b/src/storage/test/IndexTestUtil.h index 73e457e6167..f5022ce3dd9 100644 --- a/src/storage/test/IndexTestUtil.h +++ b/src/storage/test/IndexTestUtil.h @@ -15,6 +15,7 @@ #include "common/meta/NebulaSchemaProvider.h" #include "folly/Conv.h" #include "folly/String.h" +#include "interface/gen-cpp2/common_types.h" #include "kvstore/KVIterator.h" #include "kvstore/KVStore.h" #include "storage/exec/IndexNode.h" @@ -436,7 +437,8 @@ class RowParser { {"int", [](const std::string& str) { return Value(std::stol(str)); }}, {"string", [](const std::string& str) { return Value(str); }}, {"float", [](const std::string& str) { return Value(folly::to(str)); }}, - {"bool", [](const std::string& str) { return Value(str == "true" ? true : false); }}}; + {"bool", [](const std::string& str) { return Value(str == "true" ? true : false); }}, + {"geography", [](const std::string& str) { return Value(Geography::fromWKT(str).value()); }}}; }; /** @@ -491,7 +493,8 @@ class SchemaParser { {"int", ::nebula::cpp2::PropertyType::INT64}, {"double", ::nebula::cpp2::PropertyType::DOUBLE}, {"string", ::nebula::cpp2::PropertyType::STRING}, - {"bool", ::nebula::cpp2::PropertyType::BOOL}}; + {"bool", ::nebula::cpp2::PropertyType::BOOL}, + {"geography", ::nebula::cpp2::PropertyType::GEOGRAPHY}}; }; /** diff --git a/src/storage/test/IndexWriteTest.cpp b/src/storage/test/IndexWriteTest.cpp index 91f30a0f53d..23a92c4e5c6 100644 --- a/src/storage/test/IndexWriteTest.cpp +++ b/src/storage/test/IndexWriteTest.cpp @@ -318,8 +318,9 @@ TEST(IndexTest, VerticesValueTest) { values.emplace_back(Value(date)); // col_date_null values.emplace_back(nullValue); - auto indexes = - IndexKeyUtils::encodeValues(std::move(values), mock::MockData::mockTypicaIndexColumns()); + auto indexItem = std::make_unique(); + indexItem->set_fields(mock::MockData::mockTypicaIndexColumns()); + auto indexes = IndexKeyUtils::encodeValues(std::move(values), indexItem.get()); for (auto partId = 1; partId <= 6; partId++) { auto prefix = IndexKeyUtils::indexPrefix(partId, indexId);