From fee8db2ae2d526119d4190857cf366749972442a Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Tue, 19 Oct 2021 19:39:19 +0800 Subject: [PATCH] address comments --- src/common/datatypes/test/GeographyTest.cpp | 8 +- src/common/geo/GeoIndex.cpp | 4 +- src/common/geo/GeoIndex.h | 4 +- src/common/geo/GeoUtils.h | 11 +- src/common/geo/io/wkt/WKTWriter.cpp | 6 +- src/common/utils/IndexKeyUtils.cpp | 166 ++++-------- src/common/utils/IndexKeyUtils.h | 57 ++-- src/common/utils/test/IndexKeyUtilsTest.cpp | 120 +++++---- .../rule/GeoPredicateIndexScanBaseRule.cpp | 7 +- src/graph/util/SchemaUtil.cpp | 5 +- src/kvstore/test/PartTest.cpp | 8 +- src/mock/MockData.cpp | 18 +- src/storage/admin/RebuildEdgeIndexTask.cpp | 33 +-- src/storage/admin/RebuildTagIndexTask.cpp | 30 +-- src/storage/exec/UpdateNode.h | 250 +++++------------- src/storage/mutate/AddEdgesProcessor.cpp | 168 ++++-------- src/storage/mutate/AddEdgesProcessor.h | 14 +- src/storage/mutate/AddVerticesProcessor.cpp | 130 +++------ src/storage/mutate/AddVerticesProcessor.h | 14 +- src/storage/mutate/DeleteEdgesProcessor.cpp | 54 ++-- src/storage/mutate/DeleteTagsProcessor.cpp | 10 +- .../mutate/DeleteVerticesProcessor.cpp | 57 ++-- src/storage/test/IndexScanLimitTest.cpp | 36 +-- src/storage/test/IndexWriteTest.cpp | 6 +- src/storage/test/LookupIndexTest.cpp | 12 +- src/storage/test/QueryTestUtils.h | 15 +- src/tools/db-upgrade/DbUpgrader.cpp | 38 +-- src/tools/db-upgrade/DbUpgrader.h | 20 +- tests/tck/features/geo/GeoBase.feature | 2 + 29 files changed, 486 insertions(+), 817 deletions(-) diff --git a/src/common/datatypes/test/GeographyTest.cpp b/src/common/datatypes/test/GeographyTest.cpp index 0b15b0da36c..d76c90dbf2b 100644 --- a/src/common/datatypes/test/GeographyTest.cpp +++ b/src/common/datatypes/test/GeographyTest.cpp @@ -21,14 +21,14 @@ TEST(Geography, shape) { EXPECT_EQ(GeoShape::POINT, g.shape()); } { - std::string wkt = "LINESTRING(28.4 79.20,134.25 -28.34)"; + std::string wkt = "LINESTRING(28.4 79.20, 134.25 -28.34)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = gRet.value(); EXPECT_EQ(GeoShape::LINESTRING, g.shape()); } { - std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; + std::string wkt = "POLYGON((1 2, 3 4, 5 6, 1 2))"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = gRet.value(); @@ -46,7 +46,7 @@ TEST(Geography, asWKT) { EXPECT_EQ(wkt, got); } { - std::string wkt = "LINESTRING(28.4 79.2,134.25 -28.34)"; + std::string wkt = "LINESTRING(28.4 79.2, 134.25 -28.34)"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = gRet.value(); @@ -54,7 +54,7 @@ TEST(Geography, asWKT) { EXPECT_EQ(wkt, got); } { - std::string wkt = "POLYGON((1 2,3 4,5 6,1 2))"; + std::string wkt = "POLYGON((1 2, 3 4, 5 6, 1 2))"; auto gRet = Geography::fromWKT(wkt); ASSERT_TRUE(gRet.ok()); auto g = gRet.value(); diff --git a/src/common/geo/GeoIndex.cpp b/src/common/geo/GeoIndex.cpp index 9de3b2d25d7..23f297e0ea7 100644 --- a/src/common/geo/GeoIndex.cpp +++ b/src/common/geo/GeoIndex.cpp @@ -33,8 +33,8 @@ nebula::storage::cpp2::IndexColumnHint ScanRange::toIndexColumnHint() { // column_name should be set by the caller if (isRangeScan) { hint.set_scan_type(nebula::storage::cpp2::ScanType::RANGE); - hint.set_begin_value( - IndexKeyUtils::encodeUint64(rangeMin)); // Encode uint64_t as string in advance + // Encode uint64_t as string in advance + hint.set_begin_value(IndexKeyUtils::encodeUint64(rangeMin)); hint.set_end_value(IndexKeyUtils::encodeUint64(rangeMax)); } else { hint.set_scan_type(nebula::storage::cpp2::ScanType::PREFIX); diff --git a/src/common/geo/GeoIndex.h b/src/common/geo/GeoIndex.h index 86883fc8adb..3143c126920 100644 --- a/src/common/geo/GeoIndex.h +++ b/src/common/geo/GeoIndex.h @@ -83,8 +83,8 @@ class GeoIndex { private: RegionCoverParams rcParams_; - bool pointsOnly_{ - false}; // For the column Geography(Point), we don't need to build ancestor cells + // For the column Geography(Point), we don't need to build ancestor cells + bool pointsOnly_{false}; }; } // namespace geo diff --git a/src/common/geo/GeoUtils.h b/src/common/geo/GeoUtils.h index c3b289fb66d..4623ee1a621 100644 --- a/src/common/geo/GeoUtils.h +++ b/src/common/geo/GeoUtils.h @@ -37,10 +37,11 @@ class GeoUtils final { std::vector> s2Loops; s2Loops.reserve(numCoordList); for (const auto& coordList : polygon.coordListList) { - auto s2Points = s2PointsFromCoordinateList( - coordList, true); // S2 doesn't need the redundant last point + // S2 doesn't need the redundant last point + auto s2Points = s2PointsFromCoordinateList(coordList, true); auto s2Loop = std::make_unique(std::move(s2Points), S2Debug::DISABLE); - s2Loop->Normalize(); // All loops must be oriented CCW(counterclockwise) for S2 + // All loops must be oriented CCW(counterclockwise) for S2 + s2Loop->Normalize(); s2Loops.emplace_back(std::move(s2Loop)); } return std::make_unique(std::move(s2Loops), S2Debug::DISABLE); @@ -53,8 +54,8 @@ 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 + // Note: S2Point requires latitude to be first, and longitude to be last + auto latlng = S2LatLng::FromDegrees(coord.y, coord.x); return latlng.ToPoint(); } diff --git a/src/common/geo/io/wkt/WKTWriter.cpp b/src/common/geo/io/wkt/WKTWriter.cpp index 84bfcb7ab64..0a4f8c97b8a 100644 --- a/src/common/geo/io/wkt/WKTWriter.cpp +++ b/src/common/geo/io/wkt/WKTWriter.cpp @@ -61,9 +61,10 @@ 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.append(", "); } wkt.pop_back(); + wkt.pop_back(); } void WKTWriter::WKTWriter::writeCoordinateListList( @@ -75,9 +76,10 @@ void WKTWriter::WKTWriter::writeCoordinateListList( wkt.append("("); writeCoordinateList(wkt, coordList); wkt.append(")"); - wkt.append(","); + wkt.append(", "); } wkt.pop_back(); + wkt.pop_back(); } void WKTWriter::writeDouble(std::string& wkt, double v) const { diff --git a/src/common/utils/IndexKeyUtils.cpp b/src/common/utils/IndexKeyUtils.cpp index 9e38a574db4..0dc9b49279c 100644 --- a/src/common/utils/IndexKeyUtils.cpp +++ b/src/common/utils/IndexKeyUtils.cpp @@ -11,90 +11,73 @@ namespace nebula { // static -std::string IndexKeyUtils::encodeValues(std::vector&& values, - const std::vector& cols) { +std::vector IndexKeyUtils::encodeValues( + std::vector&& values, const std::vector& cols) { bool hasNullCol = false; // An index has a maximum of 16 columns. 2 byte (16 bit) is enough. u_short nullableBitSet = 0; - std::string index; + auto findGeo = [](const meta::cpp2::ColumnDef& col) { + return col.get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY; + }; + bool hasGeo = std::find_if(cols.begin(), cols.end(), findGeo) != cols.end(); + // Only support to create index on a single geography column currently; + DCHECK(!hasGeo || cols.size() == 1); + std::vector indexes; - for (size_t i = 0; i < values.size(); i++) { - auto isNullable = cols[i].nullable_ref().value_or(false); - if (isNullable) { - hasNullCol = true; - } + if (!hasGeo) { + std::string index; + for (size_t i = 0; i < values.size(); i++) { + auto isNullable = cols[i].nullable_ref().value_or(false); + if (isNullable) { + hasNullCol = true; + } - if (!values[i].isNull()) { - // string index need to fill with '\0' if length is less than schema - if (cols[i].type.type == meta::cpp2::PropertyType::FIXED_STRING) { - auto len = static_cast(*cols[i].type.get_type_length()); - index.append(encodeValue(values[i], len)); + if (!values[i].isNull()) { + // string index need to fill with '\0' if length is less than schema + if (cols[i].type.type == meta::cpp2::PropertyType::FIXED_STRING) { + auto len = static_cast(*cols[i].type.get_type_length()); + index.append(encodeValue(values[i], len)); + } else { + index.append(encodeValue(values[i])); + } } else { - index.append(encodeValue(values[i])); + nullableBitSet |= 0x8000 >> i; + auto type = IndexKeyUtils::toValueType(cols[i].type.get_type()); + index.append(encodeNullValue(type, cols[i].type.get_type_length())); } + } + indexes.emplace_back(std::move(index)); + } else { + hasNullCol = cols.back().nullable_ref().value_or(false); + DCHECK_EQ(values.size(), 1); + const auto& value = values.back(); + if (!value.isNull()) { + DCHECK(value.type() == Value::Type::GEOGRAPHY); + indexes = encodeGeography(value.getGeography()); } else { - nullableBitSet |= 0x8000 >> i; - auto type = IndexKeyUtils::toValueType(cols[i].type.get_type()); - index.append(encodeNullValue(type, cols[i].type.get_type_length())); + nullableBitSet |= 0x8000; + auto type = IndexKeyUtils::toValueType(cols.back().type.get_type()); + indexes.emplace_back(encodeNullValue(type, nullptr)); } } // if has nullable field, append nullableBitSet to the end if (hasNullCol) { - index.append(reinterpret_cast(&nullableBitSet), sizeof(u_short)); - } - return index; -} - -std::vector IndexKeyUtils::encodeValueForGeography( - Value&& value, const nebula::meta::cpp2::ColumnDef& col) { - DCHECK(col.get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY); - u_short nullableBitSet = 0; - std::vector indexes; - - auto hasNullCol = col.nullable_ref().value_or(false); - - if (!value.isNull()) { - DCHECK(value.type() == Value::Type::GEOGRAPHY); - indexes = encodeGeography(value.getGeography()); - } else { - nullableBitSet |= 0x8000; - auto type = IndexKeyUtils::toValueType(col.type.get_type()); - indexes.emplace_back(encodeNullValue(type, nullptr)); - } - - if (hasNullCol) { - // if has nullable field, append nullableBitSet to the end for (auto& index : indexes) { index.append(reinterpret_cast(&nullableBitSet), sizeof(u_short)); } } - return indexes; } // static -std::string IndexKeyUtils::vertexIndexKey( - size_t vIdLen, PartitionID partId, IndexID indexId, const VertexID& vId, std::string&& values) { - int32_t item = (partId << kPartitionOffset) | static_cast(NebulaKeyType::kIndex); - std::string key; - key.reserve(256); - key.append(reinterpret_cast(&item), sizeof(int32_t)) - .append(reinterpret_cast(&indexId), sizeof(IndexID)) - .append(values) - .append(vId.data(), vId.size()) - .append(vIdLen - vId.size(), '\0'); - return key; -} - -// static -std::vector IndexKeyUtils::vertexIndexKeysForGeography( - size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& vId, - std::vector&& values) { +std::vector IndexKeyUtils::vertexIndexKeys(size_t vIdLen, + PartitionID partId, + IndexID indexId, + const VertexID& vId, + std::vector&& values) { int32_t item = (partId << kPartitionOffset) | static_cast(NebulaKeyType::kIndex); std::vector keys; + keys.reserve(values.size()); for (const auto& value : values) { std::string key; key.reserve(256); @@ -109,38 +92,16 @@ std::vector IndexKeyUtils::vertexIndexKeysForGeography( } // static -std::string IndexKeyUtils::edgeIndexKey(size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& srcId, - EdgeRanking rank, - const VertexID& dstId, - std::string&& values) { - int32_t item = (partId << kPartitionOffset) | static_cast(NebulaKeyType::kIndex); - std::string key; - key.reserve(256); - key.append(reinterpret_cast(&item), sizeof(int32_t)) - .append(reinterpret_cast(&indexId), sizeof(IndexID)) - .append(values) - .append(srcId.data(), srcId.size()) - .append(vIdLen - srcId.size(), '\0') - .append(IndexKeyUtils::encodeRank(rank)) - .append(dstId.data(), dstId.size()) - .append(vIdLen - dstId.size(), '\0'); - return key; -} - -// static -std::vector IndexKeyUtils::edgeIndexKeysForGeography( - size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& srcId, - EdgeRanking rank, - const VertexID& dstId, - std::vector&& values) { +std::vector IndexKeyUtils::edgeIndexKeys(size_t vIdLen, + PartitionID partId, + IndexID indexId, + const VertexID& srcId, + EdgeRanking rank, + const VertexID& dstId, + std::vector&& values) { int32_t item = (partId << kPartitionOffset) | static_cast(NebulaKeyType::kIndex); std::vector keys; + keys.reserve(values.size()); for (const auto& value : values) { std::string key; key.reserve(256); @@ -196,7 +157,7 @@ Value IndexKeyUtils::parseIndexTTL(const folly::StringPiece& raw) { } // static -StatusOr IndexKeyUtils::collectIndexValues( +StatusOr> IndexKeyUtils::collectIndexValues( RowReader* reader, const std::vector& cols) { if (reader == nullptr) { return Status::Error("Invalid row reader"); @@ -215,25 +176,6 @@ StatusOr IndexKeyUtils::collectIndexValues( return encodeValues(std::move(values), cols); } -// TODO(jie) Should be refactored -// static -StatusOr> IndexKeyUtils::collectIndexValueForGeography( - RowReader* reader, const nebula::meta::cpp2::ColumnDef& col) { - if (reader == nullptr) { - return Status::Error("Invalid row reader"); - } - DCHECK(col.get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY); - - Value value = reader->getValueByName(col.get_name()); - auto isNullable = col.nullable_ref().value_or(false); - auto ret = checkValue(value, isNullable); - if (!ret.ok()) { - LOG(ERROR) << "prop error by : " << col.get_name() << ". status : " << ret; - return ret; - } - return encodeValueForGeography(std::move(value), col); -} - // static Status IndexKeyUtils::checkValue(const Value& v, bool isNullable) { if (!v.isNull()) { diff --git a/src/common/utils/IndexKeyUtils.h b/src/common/utils/IndexKeyUtils.h index c689b64f7f3..049fabebebb 100644 --- a/src/common/utils/IndexKeyUtils.h +++ b/src/common/utils/IndexKeyUtils.h @@ -139,7 +139,7 @@ class IndexKeyUtils final { return encodeDateTime(v.getDateTime()); } case Value::Type::GEOGRAPHY: { - LOG(FATAL) << "Should call encodeValueForGeography separately"; + LOG(FATAL) << "Should call encodeGeography separately"; return ""; } default: @@ -301,15 +301,16 @@ class IndexKeyUtils final { } static std::vector encodeGeography(const nebula::Geography& gg) { - geo::RegionCoverParams - rc; // TODO(jie): Get index params from meta to construct RegionCoverParams - geo::GeoIndex geoIndex(rc, false); // TODO(jie): Get schema meta to know if it's point only + // TODO(jie): Get index params from meta to construct RegionCoverParams + geo::RegionCoverParams rc; + // TODO(jie): Get schema meta to know if it's point only + geo::GeoIndex geoIndex(rc, false); auto cellIds = geoIndex.indexCells(gg); std::vector bufs; for (auto cellId : cellIds) { bufs.emplace_back(encodeUint64(cellId)); } - return bufs; // just support index point here. + return bufs; } static nebula::DateTime decodeDateTime(const folly::StringPiece& raw) { @@ -493,44 +494,30 @@ class IndexKeyUtils final { /** * Generate vertex|edge index key for kv store **/ - static std::string encodeValues(std::vector&& values, - const std::vector& cols); - static std::vector encodeValueForGeography(Value&& value, - const nebula::meta::cpp2::ColumnDef& col); + static std::vector encodeValues( + std::vector&& values, const std::vector& cols); /** * param valueTypes ： column type of each index column. If there are no *nullable columns in the index, the parameter can be empty. **/ - static std::string vertexIndexKey(size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& vId, - std::string&& values); - static std::vector vertexIndexKeysForGeography(size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& vId, - std::vector&& values); + static std::vector vertexIndexKeys(size_t vIdLen, + PartitionID partId, + IndexID indexId, + const VertexID& vId, + std::vector&& values); /** * param valueTypes ： column type of each index column. If there are no *nullable columns in the index, the parameter can be empty. **/ - static std::string edgeIndexKey(size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& srcId, - EdgeRanking rank, - const VertexID& dstId, - std::string&& values); - static std::vector edgeIndexKeysForGeography(size_t vIdLen, - PartitionID partId, - IndexID indexId, - const VertexID& srcId, - EdgeRanking rank, - const VertexID& dstId, - std::vector&& values); + static std::vector edgeIndexKeys(size_t vIdLen, + PartitionID partId, + IndexID indexId, + const VertexID& srcId, + EdgeRanking rank, + const VertexID& dstId, + std::vector&& values); static std::string indexPrefix(PartitionID partId, IndexID indexId); @@ -540,10 +527,8 @@ class IndexKeyUtils final { static Value parseIndexTTL(const folly::StringPiece& raw); - static StatusOr collectIndexValues( + static StatusOr> collectIndexValues( RowReader* reader, const std::vector& cols); - static StatusOr> collectIndexValueForGeography( - RowReader* reader, const nebula::meta::cpp2::ColumnDef& col); private: IndexKeyUtils() = delete; diff --git a/src/common/utils/test/IndexKeyUtilsTest.cpp b/src/common/utils/test/IndexKeyUtilsTest.cpp index e5e7ea1c44e..3f17023fc80 100644 --- a/src/common/utils/test/IndexKeyUtilsTest.cpp +++ b/src/common/utils/test/IndexKeyUtilsTest.cpp @@ -145,7 +145,7 @@ TEST(IndexKeyUtilsTest, encodeDouble) { TEST(IndexKeyUtilsTest, vertexIndexKeyV1) { auto values = getIndexValues(); - auto key = IndexKeyUtils::vertexIndexKey(8, 1, 1, getStringId(1), std::move(values)); + auto key = IndexKeyUtils::vertexIndexKeys(8, 1, 1, getStringId(1), {std::move(values)})[0]; ASSERT_EQ(1, IndexKeyUtils::getIndexId(key)); ASSERT_EQ(getStringId(1), IndexKeyUtils::getIndexVertexID(8, key)); ASSERT_EQ(true, IndexKeyUtils::isIndexKey(key)); @@ -153,19 +153,21 @@ TEST(IndexKeyUtilsTest, vertexIndexKeyV1) { TEST(IndexKeyUtilsTest, vertexIndexKeyV2) { auto values = getIndexValues(); - auto key = IndexKeyUtils::vertexIndexKey(100, 1, 1, "vertex_1_1_1_1", std::move(values)); - ASSERT_EQ(1, IndexKeyUtils::getIndexId(key)); - - VertexID vid = "vertex_1_1_1_1"; - vid.append(100 - vid.size(), '\0'); - ASSERT_EQ(vid, IndexKeyUtils::getIndexVertexID(100, key)); - ASSERT_EQ(true, IndexKeyUtils::isIndexKey(key)); + auto keys = IndexKeyUtils::vertexIndexKeys(100, 1, 1, "vertex_1_1_1_1", {std::move(values)}); + for (auto& key : keys) { + ASSERT_EQ(1, IndexKeyUtils::getIndexId(key)); + + VertexID vid = "vertex_1_1_1_1"; + vid.append(100 - vid.size(), '\0'); + ASSERT_EQ(vid, IndexKeyUtils::getIndexVertexID(100, key)); + ASSERT_EQ(true, IndexKeyUtils::isIndexKey(key)); + } } TEST(IndexKeyUtilsTest, edgeIndexKeyV1) { auto values = getIndexValues(); - auto key = - IndexKeyUtils::edgeIndexKey(8, 1, 1, getStringId(1), 1, getStringId(2), std::move(values)); + auto key = IndexKeyUtils::edgeIndexKeys( + 8, 1, 1, getStringId(1), 1, getStringId(2), {std::move(values)})[0]; ASSERT_EQ(1, IndexKeyUtils::getIndexId(key)); ASSERT_EQ(getStringId(1), IndexKeyUtils::getIndexSrcId(8, key)); ASSERT_EQ(1, IndexKeyUtils::getIndexRank(8, key)); @@ -176,22 +178,31 @@ TEST(IndexKeyUtilsTest, edgeIndexKeyV1) { TEST(IndexKeyUtilsTest, edgeIndexKeyV2) { VertexID vid = "vertex_1_1_1_1"; auto values = getIndexValues(); - auto key = IndexKeyUtils::edgeIndexKey(100, 1, 1, vid, 1, vid, std::move(values)); - ASSERT_EQ(1, IndexKeyUtils::getIndexId(key)); - vid.append(100 - vid.size(), '\0'); - ASSERT_EQ(vid, IndexKeyUtils::getIndexSrcId(100, key)); - ASSERT_EQ(1, IndexKeyUtils::getIndexRank(100, key)); - ASSERT_EQ(vid, IndexKeyUtils::getIndexDstId(100, key)); - ASSERT_EQ(true, IndexKeyUtils::isIndexKey(key)); + auto keys = IndexKeyUtils::edgeIndexKeys(100, 1, 1, vid, 1, vid, {std::move(values)}); + for (auto& key : keys) { + ASSERT_EQ(1, IndexKeyUtils::getIndexId(key)); + vid.append(100 - vid.size(), '\0'); + ASSERT_EQ(vid, IndexKeyUtils::getIndexSrcId(100, key)); + ASSERT_EQ(1, IndexKeyUtils::getIndexRank(100, key)); + ASSERT_EQ(vid, IndexKeyUtils::getIndexDstId(100, key)); + ASSERT_EQ(true, IndexKeyUtils::isIndexKey(key)); + } - key = IndexKeyUtils::edgeIndexKey(100, 1, 1, vid, -1, vid, std::move(values)); - ASSERT_EQ(-1, IndexKeyUtils::getIndexRank(100, key)); + keys = IndexKeyUtils::edgeIndexKeys(100, 1, 1, vid, -1, vid, {std::move(values)}); + for (auto& key : keys) { + ASSERT_EQ(-1, IndexKeyUtils::getIndexRank(100, key)); + } - key = IndexKeyUtils::edgeIndexKey(100, 1, 1, vid, 9223372036854775807, vid, std::move(values)); - ASSERT_EQ(9223372036854775807, IndexKeyUtils::getIndexRank(100, key)); + keys = + IndexKeyUtils::edgeIndexKeys(100, 1, 1, vid, 9223372036854775807, vid, {std::move(values)}); + for (auto& key : keys) { + ASSERT_EQ(9223372036854775807, IndexKeyUtils::getIndexRank(100, key)); + } - key = IndexKeyUtils::edgeIndexKey(100, 1, 1, vid, 0, vid, std::move(values)); - ASSERT_EQ(0, IndexKeyUtils::getIndexRank(100, key)); + keys = IndexKeyUtils::edgeIndexKeys(100, 1, 1, vid, 0, vid, {std::move(values)}); + for (auto& key : keys) { + ASSERT_EQ(0, IndexKeyUtils::getIndexRank(100, key)); + } } TEST(IndexKeyUtilsTest, nullableValue) { @@ -212,11 +223,12 @@ TEST(IndexKeyUtilsTest, nullableValue) { values.emplace_back(Value(NullType::__NULL__)); cols.emplace_back(nullCol(folly::stringPrintf("col%ld", j), meta::cpp2::PropertyType::BOOL)); } - auto raw = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + // TODO(jie) Add index key tests for geography + auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); u_short s = 0xfc00; /* the binary is '11111100 00000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); - auto result = raw.substr(raw.size() - sizeof(u_short), sizeof(u_short)); + auto result = raws[0].substr(raws[0].size() - sizeof(u_short), sizeof(u_short)); ASSERT_EQ(expected, result); } { @@ -227,11 +239,11 @@ TEST(IndexKeyUtilsTest, nullableValue) { for (int64_t j = 1; j <= 2; j++) { cols.emplace_back(nullCol(folly::stringPrintf("col%ld", j), meta::cpp2::PropertyType::BOOL)); } - auto raw = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); u_short s = 0x4000; /* the binary is '01000000 00000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); - auto result = raw.substr(raw.size() - sizeof(u_short), sizeof(u_short)); + auto result = raws[0].substr(raws[0].size() - sizeof(u_short), sizeof(u_short)); ASSERT_EQ(expected, result); } { @@ -242,11 +254,11 @@ TEST(IndexKeyUtilsTest, nullableValue) { for (int64_t j = 1; j <= 2; j++) { cols.emplace_back(nullCol(folly::stringPrintf("col%ld", j), meta::cpp2::PropertyType::BOOL)); } - auto raw = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); u_short s = 0x0000; /* the binary is '01000000 00000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); - auto result = raw.substr(raw.size() - sizeof(u_short), sizeof(u_short)); + auto result = raws[0].substr(raws[0].size() - sizeof(u_short), sizeof(u_short)); ASSERT_EQ(expected, result); } { @@ -257,11 +269,11 @@ TEST(IndexKeyUtilsTest, nullableValue) { cols.emplace_back(nullCol(folly::stringPrintf("col%ld", i), meta::cpp2::PropertyType::INT64)); } - auto raw = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); u_short s = 0xfff0; /* the binary is '11111111 11110000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); - auto result = raw.substr(raw.size() - sizeof(u_short), sizeof(u_short)); + auto result = raws[0].substr(raws[0].size() - sizeof(u_short), sizeof(u_short)); ASSERT_EQ(expected, result); } { @@ -295,11 +307,11 @@ TEST(IndexKeyUtilsTest, nullableValue) { cols.emplace_back(nullCol(folly::stringPrintf("col_%ld_%ld", i, j), types[j])); } } - auto raw = IndexKeyUtils::encodeValues(std::move(values), cols); + auto raws = IndexKeyUtils::encodeValues(std::move(values), cols); u_short s = 0xaaa0; /* the binary is '10101010 10100000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); - auto result = raw.substr(raw.size() - sizeof(u_short), sizeof(u_short)); + auto result = raws[0].substr(raws[0].size() - sizeof(u_short), sizeof(u_short)); ASSERT_EQ(expected, result); } { @@ -309,11 +321,11 @@ TEST(IndexKeyUtilsTest, nullableValue) { values.emplace_back(Value(NullType::__NULL__)); cols.emplace_back(nullCol(folly::stringPrintf("col%ld", i), meta::cpp2::PropertyType::BOOL)); } - auto raw = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); + auto raws = IndexKeyUtils::encodeValues(std::move(values), std::move(cols)); u_short s = 0xff80; /* the binary is '11111111 10000000'*/ std::string expected; expected.append(reinterpret_cast(&s), sizeof(u_short)); - auto result = raw.substr(raw.size() - sizeof(u_short), sizeof(u_short)); + auto result = raws[0].substr(raws[0].size() - sizeof(u_short), sizeof(u_short)); ASSERT_EQ(expected, result); } } @@ -408,9 +420,12 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { std::vector indexKeys; for (auto& row : vertices) { auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); - ASSERT_EQ(indexValueSize, values.size()); - indexKeys.emplace_back( - IndexKeyUtils::vertexIndexKey(vIdLen, partId, indexId, row.first, std::move(values))); + ASSERT_EQ(indexValueSize, values[0].size()); + auto keys = + IndexKeyUtils::vertexIndexKeys(vIdLen, partId, indexId, row.first, std::move(values)); + for (auto& key : keys) { + indexKeys.emplace_back(key); + } } verifyDecodeIndexKey(false, false, vIdLen, expected, indexKeys, cols); @@ -427,9 +442,13 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { std::vector indexKeys; for (auto& row : edges) { auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); - ASSERT_EQ(indexValueSize, values.size()); - indexKeys.emplace_back(IndexKeyUtils::edgeIndexKey( - vIdLen, partId, indexId, row.first, 0, row.first, std::move(values))); + ASSERT_EQ(indexValueSize, values[0].size()); + + auto keys = IndexKeyUtils::edgeIndexKeys( + vIdLen, partId, indexId, row.first, 0, row.first, std::move(values)); + for (auto& key : keys) { + indexKeys.emplace_back(key); + } } verifyDecodeIndexKey(true, false, vIdLen, expected, indexKeys, cols); @@ -459,10 +478,12 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { std::vector indexKeys; for (auto& row : vertices) { auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); - ASSERT_EQ(indexValueSize, values.size()); - auto key = - IndexKeyUtils::vertexIndexKey(vIdLen, partId, indexId, row.first, std::move(values)); - indexKeys.emplace_back(key); + ASSERT_EQ(indexValueSize, values[0].size()); + auto keys = + IndexKeyUtils::vertexIndexKeys(vIdLen, partId, indexId, row.first, std::move(values)); + for (auto& key : keys) { + indexKeys.emplace_back(key); + } } verifyDecodeIndexKey(false, true, vIdLen, expected, indexKeys, cols); } @@ -484,9 +505,12 @@ TEST(IndexKeyUtilsTest, getValueFromIndexKeyTest) { std::vector indexKeys; for (auto& row : edges) { auto values = IndexKeyUtils::encodeValues(std::move(row.second), cols); - ASSERT_EQ(indexValueSize, values.size()); - indexKeys.emplace_back(IndexKeyUtils::edgeIndexKey( - vIdLen, partId, indexId, row.first, 0, row.first, std::move(values))); + ASSERT_EQ(indexValueSize, values[0].size()); + auto keys = IndexKeyUtils::edgeIndexKeys( + vIdLen, partId, indexId, row.first, 0, row.first, std::move(values)); + for (auto& key : keys) { + indexKeys.emplace_back(key); + } } verifyDecodeIndexKey(true, true, vIdLen, expected, indexKeys, cols); diff --git a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp index d4894e58e0e..5f7eacd82d9 100644 --- a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp +++ b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp @@ -84,9 +84,10 @@ StatusOr GeoPredicateIndexScanBaseRule::transform( DCHECK(secondVal.type() == Value::Type::GEOGRAPHY); const auto& geog = secondVal.getGeography(); - geo::RegionCoverParams - rc; // TODO(jie): Get index params from meta to construct RegionCoverParams - geo::GeoIndex geoIndex(rc, false); // TODO(jie): Get schema meta to know if it's point only + // TODO(jie): Get index params from meta to construct RegionCoverParams + geo::RegionCoverParams rc; + // TODO(jie): Get schema meta to know if it's point only + geo::GeoIndex geoIndex(rc, false); std::vector scanRanges; if (geoPredicateName == "st_intersects") { scanRanges = geoIndex.intersects(geog); diff --git a/src/graph/util/SchemaUtil.cpp b/src/graph/util/SchemaUtil.cpp index c638efa5365..b3d054b711d 100644 --- a/src/graph/util/SchemaUtil.cpp +++ b/src/graph/util/SchemaUtil.cpp @@ -263,10 +263,9 @@ std::string SchemaUtil::typeToString(const meta::cpp2::ColumnTypeDef &col) { } else if (col.get_type() == meta::cpp2::PropertyType::GEOGRAPHY) { auto geoShape = *col.get_geo_shape(); if (geoShape == meta::cpp2::GeoShape::ANY) { - return folly::stringPrintf("%s", type.c_str()); + return type; } - return folly::stringPrintf( - "%s(%s)", type.c_str(), apache::thrift::util::enumNameSafe(geoShape).c_str()); + return folly::sformat("{}({})", type, apache::thrift::util::enumNameSafe(geoShape)); } return type; } diff --git a/src/kvstore/test/PartTest.cpp b/src/kvstore/test/PartTest.cpp index e27e1a81767..ab28851f0b4 100644 --- a/src/kvstore/test/PartTest.cpp +++ b/src/kvstore/test/PartTest.cpp @@ -159,9 +159,11 @@ TEST(PartTest, PartCleanTest) { } IndexID indexId = 5; for (int i = 0; i < 10; i++) { - auto key = - IndexKeyUtils::vertexIndexKey(kDefaultVIdLen, partId, indexId, std::to_string(i), "123"); - data.emplace_back(key, folly::stringPrintf("val%d", i)); + auto keys = IndexKeyUtils::vertexIndexKeys( + kDefaultVIdLen, partId, indexId, std::to_string(i), {"123"}); + for (auto& key : keys) { + data.emplace_back(key, folly::stringPrintf("val%d", i)); + } } data.emplace_back(NebulaKeyUtils::systemCommitKey(partId), "123"); diff --git a/src/mock/MockData.cpp b/src/mock/MockData.cpp index 87e84b15b54..c9f8356ce84 100644 --- a/src/mock/MockData.cpp +++ b/src/mock/MockData.cpp @@ -736,9 +736,11 @@ std::vector> MockData::mockPlayerIndexKeys(b values.append(encodeFixedStr(name, 20)); values.append(IndexKeyUtils::encodeValue(player.age_)); values.append(IndexKeyUtils::encodeValue(player.playing_)); - auto key = IndexKeyUtils::vertexIndexKey(32, part, 1, name, std::move(values)); - auto pair = std::make_pair(part, std::move(key)); - keys.emplace_back(std::move(pair)); + auto indexKeys = IndexKeyUtils::vertexIndexKeys(32, part, 1, name, {std::move(values)}); + for (auto& indexKey : indexKeys) { + auto pair = std::make_pair(part, std::move(indexKey)); + keys.emplace_back(std::move(pair)); + } } return keys; } @@ -839,10 +841,12 @@ std::vector> MockData::mockServeIndexKeys() values.append(encodeFixedStr(serve.playerName_, 20)); values.append(encodeFixedStr(serve.teamName_, 20)); values.append(IndexKeyUtils::encodeValue(serve.startYear_)); - auto key = IndexKeyUtils::edgeIndexKey( - 32, part, 101, serve.playerName_, serve.startYear_, serve.teamName_, std::move(values)); - auto pair = std::make_pair(part, std::move(key)); - keys.emplace_back(std::move(pair)); + auto idxKeys = IndexKeyUtils::edgeIndexKeys( + 32, part, 101, serve.playerName_, serve.startYear_, serve.teamName_, {std::move(values)}); + for (auto& idxkey : idxKeys) { + auto pair = std::make_pair(part, std::move(idxkey)); + keys.emplace_back(std::move(pair)); + } } return keys; } diff --git a/src/storage/admin/RebuildEdgeIndexTask.cpp b/src/storage/admin/RebuildEdgeIndexTask.cpp index 5ffe839f29d..31006c99929 100644 --- a/src/storage/admin/RebuildEdgeIndexTask.cpp +++ b/src/storage/admin/RebuildEdgeIndexTask.cpp @@ -137,38 +137,19 @@ nebula::cpp2::ErrorCode RebuildEdgeIndexTask::buildIndexGlobal(GraphSpaceID spac for (const auto& item : items) { if (item->get_schema_id().get_edge_type() == edgeType) { - if (item->get_fields().size() == 1 && item->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto valuesRet = - IndexKeyUtils::collectIndexValueForGeography(reader.get(), item->get_fields().back()); - if (!valuesRet.ok()) { - LOG(WARNING) << "Collect index value failed"; - continue; - } - auto indexKeys = IndexKeyUtils::edgeIndexKeysForGeography(vidSize, - part, - item->get_index_id(), - source.toString(), - ranking, - destination.toString(), - std::move(valuesRet).value()); - for (auto& indexKey : indexKeys) { - batchSize += indexKey.size() + indexVal.size(); - data.emplace_back(std::move(indexKey), indexVal); - } - } else { - auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), item->get_fields()); - if (!valuesRet.ok()) { - LOG(WARNING) << "Collect index value failed"; - continue; - } - auto indexKey = IndexKeyUtils::edgeIndexKey(vidSize, + auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), item->get_fields()); + if (!valuesRet.ok()) { + LOG(WARNING) << "Collect index value failed"; + continue; + } + auto indexKeys = IndexKeyUtils::edgeIndexKeys(vidSize, part, item->get_index_id(), source.toString(), ranking, destination.toString(), std::move(valuesRet).value()); + for (auto& indexKey : indexKeys) { batchSize += indexKey.size() + indexVal.size(); data.emplace_back(std::move(indexKey), indexVal); } diff --git a/src/storage/admin/RebuildTagIndexTask.cpp b/src/storage/admin/RebuildTagIndexTask.cpp index 15231cd27c0..006571fb021 100644 --- a/src/storage/admin/RebuildTagIndexTask.cpp +++ b/src/storage/admin/RebuildTagIndexTask.cpp @@ -128,28 +128,14 @@ nebula::cpp2::ErrorCode RebuildTagIndexTask::buildIndexGlobal(GraphSpaceID space for (const auto& item : items) { if (item->get_schema_id().get_tag_id() == tagID) { - if (item->get_fields().size() == 1 && item->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto valuesRet = - IndexKeyUtils::collectIndexValueForGeography(reader.get(), item->get_fields().back()); - if (!valuesRet.ok()) { - LOG(WARNING) << "Collect index value for geography failed"; - continue; - } - auto indexKeys = IndexKeyUtils::vertexIndexKeysForGeography( - vidSize, part, item->get_index_id(), vertex.toString(), std::move(valuesRet).value()); - for (auto& indexKey : indexKeys) { - batchSize += indexKey.size() + indexVal.size(); - data.emplace_back(std::move(indexKey), indexVal); - } - } else { - auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), item->get_fields()); - if (!valuesRet.ok()) { - LOG(WARNING) << "Collect index value failed"; - continue; - } - auto indexKey = IndexKeyUtils::vertexIndexKey( - vidSize, part, item->get_index_id(), vertex.toString(), std::move(valuesRet).value()); + auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), item->get_fields()); + if (!valuesRet.ok()) { + LOG(WARNING) << "Collect index value failed"; + continue; + } + auto indexKeys = IndexKeyUtils::vertexIndexKeys( + vidSize, part, item->get_index_id(), vertex.toString(), std::move(valuesRet).value()); + for (auto& indexKey : indexKeys) { batchSize += indexKey.size() + indexVal.size(); data.emplace_back(std::move(indexKey), indexVal); } diff --git a/src/storage/exec/UpdateNode.h b/src/storage/exec/UpdateNode.h index 17bb543e6ca..f36e284c082 100644 --- a/src/storage/exec/UpdateNode.h +++ b/src/storage/exec/UpdateNode.h @@ -362,37 +362,19 @@ class UpdateTagNode : public UpdateNode { LOG(ERROR) << "Bad format row"; return folly::none; } - if (index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto ois = indexKeysForGeography(partId, vId, reader_, index); - if (!ois.empty()) { - auto iState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(iState)) { - for (auto& oi : ois) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(oi)); - } - } else if (context_->env()->checkIndexLocked(iState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { - for (auto& oi : ois) { - batchHolder->remove(std::move(oi)); - } + auto ois = indexKeys(partId, vId, reader_, index); + if (!ois.empty()) { + auto iState = context_->env()->getIndexState(context_->spaceId(), partId); + if (context_->env()->checkRebuilding(iState)) { + auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); + for (auto& oi : ois) { + batchHolder->put(std::string(deleteOpKey), std::move(oi)); } - } - } else { - auto oi = indexKey(partId, vId, reader_, index); - if (!oi.empty()) { - auto iState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(iState)) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(oi)); - } else if (context_->env()->checkIndexLocked(iState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { + } else if (context_->env()->checkIndexLocked(iState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + return folly::none; + } else { + for (auto& oi : ois) { batchHolder->remove(std::move(oi)); } } @@ -408,41 +390,22 @@ class UpdateTagNode : public UpdateNode { LOG(ERROR) << "Bad format row"; return folly::none; } - if (index->get_fields().size() == 1 && index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto nis = indexKeysForGeography(partId, vId, nReader.get(), index); - if (!nis.empty()) { - auto v = CommonUtils::ttlValue(schema_, nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - auto indexState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(indexState)) { - for (auto& ni : nis) { - auto modifyKey = OperationKeyUtils::modifyOperationKey(partId, std::move(ni)); - batchHolder->put(std::move(modifyKey), std::string(niv)); - } - } else if (context_->env()->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { - for (auto& ni : nis) { - batchHolder->put(std::move(ni), std::string(niv)); - } - } - } - } else { - auto ni = indexKey(partId, vId, nReader.get(), index); - if (!ni.empty()) { - auto v = CommonUtils::ttlValue(schema_, nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - auto indexState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(indexState)) { + auto nis = indexKeys(partId, vId, nReader.get(), index); + if (!nis.empty()) { + auto v = CommonUtils::ttlValue(schema_, nReader.get()); + auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; + auto indexState = context_->env()->getIndexState(context_->spaceId(), partId); + if (context_->env()->checkRebuilding(indexState)) { + for (auto& ni : nis) { auto modifyKey = OperationKeyUtils::modifyOperationKey(partId, std::move(ni)); - batchHolder->put(std::move(modifyKey), std::move(niv)); - } else if (context_->env()->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { - batchHolder->put(std::move(ni), std::move(niv)); + batchHolder->put(std::move(modifyKey), std::string(niv)); + } + } else if (context_->env()->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + return folly::none; + } else { + for (auto& ni : nis) { + batchHolder->put(std::move(ni), std::string(niv)); } } } @@ -454,30 +417,15 @@ class UpdateTagNode : public UpdateNode { return encodeBatchValue(batchHolder->getBatch()); } - std::string indexKey(PartitionID partId, - const VertexID& vId, - RowReader* reader, - std::shared_ptr index) { + std::vector indexKeys(PartitionID partId, + const VertexID& vId, + RowReader* reader, + std::shared_ptr index) { auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); - if (!values.ok()) { - return ""; - } - return IndexKeyUtils::vertexIndexKey( - context_->vIdLen(), partId, index->get_index_id(), vId, std::move(values).value()); - } - - std::vector indexKeysForGeography( - PartitionID partId, - const VertexID& vId, - RowReader* reader, - std::shared_ptr index) { - DCHECK(index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY); - auto values = IndexKeyUtils::collectIndexValueForGeography(reader, index->get_fields().back()); if (!values.ok()) { return {}; } - return IndexKeyUtils::vertexIndexKeysForGeography( + return IndexKeyUtils::vertexIndexKeys( context_->vIdLen(), partId, index->get_index_id(), vId, std::move(values).value()); } @@ -737,37 +685,19 @@ class UpdateEdgeNode : public UpdateNode { LOG(ERROR) << "Bad format row"; return folly::none; } - if (index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto ois = indexKeysForGeography(partId, reader_, edgeKey, index); - if (!ois.empty()) { - auto iState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(iState)) { - for (auto& oi : ois) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(oi)); - } - } else if (context_->env()->checkIndexLocked(iState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { - for (auto& oi : ois) { - batchHolder->remove(std::move(oi)); - } + auto ois = indexKeys(partId, reader_, edgeKey, index); + if (!ois.empty()) { + auto iState = context_->env()->getIndexState(context_->spaceId(), partId); + if (context_->env()->checkRebuilding(iState)) { + auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); + for (auto& oi : ois) { + batchHolder->put(std::string(deleteOpKey), std::move(oi)); } - } - } else { - auto oi = indexKey(partId, reader_, edgeKey, index); - if (!oi.empty()) { - auto iState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(iState)) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(oi)); - } else if (context_->env()->checkIndexLocked(iState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { + } else if (context_->env()->checkIndexLocked(iState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + return folly::none; + } else { + for (auto& oi : ois) { batchHolder->remove(std::move(oi)); } } @@ -783,41 +713,22 @@ class UpdateEdgeNode : public UpdateNode { LOG(ERROR) << "Bad format row"; return folly::none; } - if (index->get_fields().size() == 1 && index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto niks = indexKeysForGeography(partId, nReader.get(), edgeKey, index); - if (!niks.empty()) { - auto v = CommonUtils::ttlValue(schema_, nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - auto indexState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(indexState)) { - for (auto& nik : niks) { - auto modifyKey = OperationKeyUtils::modifyOperationKey(partId, std::move(nik)); - batchHolder->put(std::move(modifyKey), std::string(niv)); - } - } else if (context_->env()->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { - for (auto& nik : niks) { - batchHolder->put(std::move(nik), std::string(niv)); - } - } - } - } else { - auto nik = indexKey(partId, nReader.get(), edgeKey, index); - if (!nik.empty()) { - auto v = CommonUtils::ttlValue(schema_, nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - auto indexState = context_->env()->getIndexState(context_->spaceId(), partId); - if (context_->env()->checkRebuilding(indexState)) { + auto niks = indexKeys(partId, nReader.get(), edgeKey, index); + if (!niks.empty()) { + auto v = CommonUtils::ttlValue(schema_, nReader.get()); + auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; + auto indexState = context_->env()->getIndexState(context_->spaceId(), partId); + if (context_->env()->checkRebuilding(indexState)) { + for (auto& nik : niks) { auto modifyKey = OperationKeyUtils::modifyOperationKey(partId, std::move(nik)); - batchHolder->put(std::move(modifyKey), std::move(niv)); - } else if (context_->env()->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return folly::none; - } else { - batchHolder->put(std::move(nik), std::move(niv)); + batchHolder->put(std::move(modifyKey), std::string(niv)); + } + } else if (context_->env()->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + return folly::none; + } else { + for (auto& nik : niks) { + batchHolder->put(std::move(nik), std::string(niv)); } } } @@ -837,42 +748,21 @@ class UpdateEdgeNode : public UpdateNode { return encodeBatchValue(batchHolder->getBatch()); } - // TODO(jie) The indexKey code has been defined many times in many files... - std::string indexKey(PartitionID partId, - RowReader* reader, - const cpp2::EdgeKey& edgeKey, - std::shared_ptr index) { + std::vector indexKeys(PartitionID partId, + RowReader* reader, + const cpp2::EdgeKey& edgeKey, + std::shared_ptr index) { auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); - if (!values.ok()) { - return ""; - } - return IndexKeyUtils::edgeIndexKey(context_->vIdLen(), - partId, - index->get_index_id(), - edgeKey.get_src().getStr(), - edgeKey.get_ranking(), - edgeKey.get_dst().getStr(), - std::move(values).value()); - } - - std::vector indexKeysForGeography( - PartitionID partId, - RowReader* reader, - const cpp2::EdgeKey& edgeKey, - std::shared_ptr index) { - DCHECK(index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY); - auto values = IndexKeyUtils::collectIndexValueForGeography(reader, index->get_fields().back()); if (!values.ok()) { return {}; } - return IndexKeyUtils::edgeIndexKeysForGeography(context_->vIdLen(), - partId, - index->get_index_id(), - edgeKey.get_src().getStr(), - edgeKey.get_ranking(), - edgeKey.get_dst().getStr(), - std::move(values).value()); + return IndexKeyUtils::edgeIndexKeys(context_->vIdLen(), + partId, + index->get_index_id(), + edgeKey.get_src().getStr(), + edgeKey.get_ranking(), + edgeKey.get_dst().getStr(), + std::move(values).value()); } private: diff --git a/src/storage/mutate/AddEdgesProcessor.cpp b/src/storage/mutate/AddEdgesProcessor.cpp index f2d9ff8513a..bc266bd379c 100644 --- a/src/storage/mutate/AddEdgesProcessor.cpp +++ b/src/storage/mutate/AddEdgesProcessor.cpp @@ -245,41 +245,21 @@ void AddEdgesProcessor::doProcessWithIndex(const cpp2::AddEdgesRequest& req) { * step 1 , Delete old version index if exists. */ if (oReader != nullptr) { - if (index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto ois = indexKeysForGeography(partId, oReader.get(), key, index); - if (!ois.empty()) { - // Check the index is building for the specified partition or not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - for (auto& oi : ois) { - auto delOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(delOpKey), std::move(oi)); - } - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { - for (auto& oi : ois) { - batchHolder->remove(std::move(oi)); - } + auto ois = indexKeys(partId, oReader.get(), key, index); + if (!ois.empty()) { + // Check the index is building for the specified partition or not. + auto indexState = env_->getIndexState(spaceId_, partId); + if (env_->checkRebuilding(indexState)) { + auto delOpKey = OperationKeyUtils::deleteOperationKey(partId); + for (auto& oi : ois) { + batchHolder->put(std::string(delOpKey), std::move(oi)); } - } - } else { - auto oi = indexKey(partId, oReader.get(), key, index); - if (!oi.empty()) { - // Check the index is building for the specified partition or not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - auto delOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(delOpKey), std::move(oi)); - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { + } else if (env_->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; + break; + } else { + for (auto& oi : ois) { batchHolder->remove(std::move(oi)); } } @@ -289,46 +269,24 @@ void AddEdgesProcessor::doProcessWithIndex(const cpp2::AddEdgesRequest& req) { * step 2 , Insert new edge index */ if (nReader != nullptr) { - if (index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto niks = indexKeysForGeography(partId, nReader.get(), key, index); - if (!niks.empty()) { - auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - // Check the index is building for the specified partition or not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - for (auto& nik : niks) { - auto opKey = OperationKeyUtils::modifyOperationKey(partId, std::move(nik)); - batchHolder->put(std::move(opKey), std::string(niv)); - } - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { - for (auto& nik : niks) { - batchHolder->put(std::move(nik), std::string(niv)); - } - } - } - } else { - auto nik = indexKey(partId, nReader.get(), key, index); - if (!nik.empty()) { - auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - // Check the index is building for the specified partition or not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { + auto niks = indexKeys(partId, nReader.get(), key, index); + if (!niks.empty()) { + auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); + auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; + // Check the index is building for the specified partition or not. + auto indexState = env_->getIndexState(spaceId_, partId); + if (env_->checkRebuilding(indexState)) { + for (auto& nik : niks) { auto opKey = OperationKeyUtils::modifyOperationKey(partId, std::move(nik)); - batchHolder->put(std::move(opKey), std::move(niv)); - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { - batchHolder->put(std::move(nik), std::move(niv)); + batchHolder->put(std::move(opKey), std::string(niv)); + } + } else if (env_->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; + break; + } else { + for (auto& nik : niks) { + batchHolder->put(std::move(nik), std::string(niv)); } } } @@ -418,18 +376,22 @@ ErrorOr AddEdgesProcessor::addEdges( } if (!val.empty()) { - auto oi = indexKey(partId, oReader.get(), e.first, index); - if (!oi.empty()) { + auto ois = indexKeys(partId, oReader.get(), e.first, index); + if (!ois.empty()) { // Check the index is building for the specified partition or not. auto indexState = env_->getIndexState(spaceId_, partId); if (env_->checkRebuilding(indexState)) { auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(oi)); + for (auto& oi : ois) { + batchHolder->put(std::string(deleteOpKey), std::move(oi)); + } } else if (env_->checkIndexLocked(indexState)) { LOG(ERROR) << "The index has been locked: " << index->get_index_name(); return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; } else { - batchHolder->remove(std::move(oi)); + for (auto& oi : ois) { + batchHolder->remove(std::move(oi)); + } } } } @@ -446,20 +408,24 @@ ErrorOr AddEdgesProcessor::addEdges( } } - auto nik = indexKey(partId, nReader.get(), e.first, index); - if (!nik.empty()) { + auto niks = indexKeys(partId, nReader.get(), e.first, index); + if (!niks.empty()) { auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; // Check the index is building for the specified partition or not. auto indexState = env_->getIndexState(spaceId_, partId); if (env_->checkRebuilding(indexState)) { - auto modifyOpKey = OperationKeyUtils::modifyOperationKey(partId, std::move(nik)); - batchHolder->put(std::move(modifyOpKey), std::move(niv)); + for (auto& nik : niks) { + auto modifyOpKey = OperationKeyUtils::modifyOperationKey(partId, std::move(nik)); + batchHolder->put(std::move(modifyOpKey), std::string(niv)); + } } else if (env_->checkIndexLocked(indexState)) { LOG(ERROR) << "The index has been locked: " << index->get_index_name(); return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; } else { - batchHolder->put(std::move(nik), std::move(niv)); + for (auto& nik : niks) { + batchHolder->put(std::move(nik), std::string(niv)); + } } } } @@ -495,42 +461,22 @@ ErrorOr AddEdgesProcessor::findOldValue( } } -std::string AddEdgesProcessor::indexKey(PartitionID partId, - RowReader* reader, - const folly::StringPiece& rawKey, - std::shared_ptr index) { - auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); - if (!values.ok()) { - return ""; - } - return IndexKeyUtils::edgeIndexKey(spaceVidLen_, - partId, - index->get_index_id(), - NebulaKeyUtils::getSrcId(spaceVidLen_, rawKey).str(), - NebulaKeyUtils::getRank(spaceVidLen_, rawKey), - NebulaKeyUtils::getDstId(spaceVidLen_, rawKey).str(), - std::move(values).value()); -} - -std::vector AddEdgesProcessor::indexKeysForGeography( +std::vector AddEdgesProcessor::indexKeys( PartitionID partId, RowReader* reader, const folly::StringPiece& rawKey, std::shared_ptr index) { - DCHECK(index->get_fields().size() == 1 && - index->get_fields().back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY); - auto values = IndexKeyUtils::collectIndexValueForGeography(reader, index->get_fields().back()); + auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); if (!values.ok()) { return {}; } - return IndexKeyUtils::edgeIndexKeysForGeography( - spaceVidLen_, - partId, - index->get_index_id(), - NebulaKeyUtils::getSrcId(spaceVidLen_, rawKey).str(), - NebulaKeyUtils::getRank(spaceVidLen_, rawKey), - NebulaKeyUtils::getDstId(spaceVidLen_, rawKey).str(), - std::move(values).value()); + return IndexKeyUtils::edgeIndexKeys(spaceVidLen_, + partId, + index->get_index_id(), + NebulaKeyUtils::getSrcId(spaceVidLen_, rawKey).str(), + NebulaKeyUtils::getRank(spaceVidLen_, rawKey), + NebulaKeyUtils::getDstId(spaceVidLen_, rawKey).str(), + std::move(values).value()); } } // namespace storage diff --git a/src/storage/mutate/AddEdgesProcessor.h b/src/storage/mutate/AddEdgesProcessor.h index fead9dfdbc1..ec509cb2f89 100644 --- a/src/storage/mutate/AddEdgesProcessor.h +++ b/src/storage/mutate/AddEdgesProcessor.h @@ -43,16 +43,10 @@ class AddEdgesProcessor : public BaseProcessor { ErrorOr findOldValue(PartitionID partId, const folly::StringPiece& rawKey); - std::string indexKey(PartitionID partId, - RowReader* reader, - const folly::StringPiece& rawKey, - std::shared_ptr index); - - std::vector indexKeysForGeography( - PartitionID partId, - RowReader* reader, - const folly::StringPiece& rawKey, - std::shared_ptr index); + std::vector indexKeys(PartitionID partId, + RowReader* reader, + const folly::StringPiece& rawKey, + std::shared_ptr index); private: GraphSpaceID spaceId_; diff --git a/src/storage/mutate/AddVerticesProcessor.cpp b/src/storage/mutate/AddVerticesProcessor.cpp index b5950c16b4a..4f3186864b0 100644 --- a/src/storage/mutate/AddVerticesProcessor.cpp +++ b/src/storage/mutate/AddVerticesProcessor.cpp @@ -223,42 +223,22 @@ void AddVerticesProcessor::doProcessWithIndex(const cpp2::AddVerticesRequest& re * step 1 , Delete old version index if exists. */ if (oReader != nullptr) { - if (indexFields.size() == 1 && - indexFields.back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY) { - auto ois = indexKeysForGeography(partId, vid, oReader.get(), index); - if (!ois.empty()) { - // Check the index is building for the specified partition or - // not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - for (auto& oi : ois) { - auto delOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(delOpKey), std::move(oi)); - } - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { - for (auto& oi : ois) { - batchHolder->remove(std::move(oi)); - } + auto ois = indexKeys(partId, vid, oReader.get(), index); + if (!ois.empty()) { + // Check the index is building for the specified partition or + // not. + auto indexState = env_->getIndexState(spaceId_, partId); + if (env_->checkRebuilding(indexState)) { + auto delOpKey = OperationKeyUtils::deleteOperationKey(partId); + for (auto& oi : ois) { + batchHolder->put(std::string(delOpKey), std::move(oi)); } - } - } else { - auto oi = indexKey(partId, vid, oReader.get(), index); - if (!oi.empty()) { - // Check the index is building for the specified partition or - // not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - auto delOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(delOpKey), std::move(oi)); - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { + } else if (env_->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; + break; + } else { + for (auto& oi : ois) { batchHolder->remove(std::move(oi)); } } @@ -269,47 +249,25 @@ void AddVerticesProcessor::doProcessWithIndex(const cpp2::AddVerticesRequest& re * step 2 , Insert new vertex index */ if (nReader != nullptr) { - if (indexFields.size() == 1 && - indexFields.back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY) { - auto niks = indexKeysForGeography(partId, vid, nReader.get(), index); - if (!niks.empty()) { - auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - // Check the index is building for the specified partition or - // not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - for (const auto& nik : niks) { - auto opKey = OperationKeyUtils::modifyOperationKey(partId, nik); - batchHolder->put(std::move(opKey), std::string(niv)); - } - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { - for (auto& nik : niks) { - batchHolder->put(std::move(nik), std::string(niv)); - } - } - } - } else { - auto nik = indexKey(partId, vid, nReader.get(), index); - if (!nik.empty()) { - auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); - auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; - // Check the index is building for the specified partition or - // not. - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { + auto niks = indexKeys(partId, vid, nReader.get(), index); + if (!niks.empty()) { + auto v = CommonUtils::ttlValue(schema.get(), nReader.get()); + auto niv = v.ok() ? IndexKeyUtils::indexVal(std::move(v).value()) : ""; + // Check the index is building for the specified partition or + // not. + auto indexState = env_->getIndexState(spaceId_, partId); + if (env_->checkRebuilding(indexState)) { + for (auto& nik : niks) { auto opKey = OperationKeyUtils::modifyOperationKey(partId, nik); - batchHolder->put(std::move(opKey), std::move(niv)); - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - break; - } else { - batchHolder->put(std::move(nik), std::move(niv)); + batchHolder->put(std::move(opKey), std::string(niv)); + } + } else if (env_->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + code = nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; + break; + } else { + for (auto& nik : niks) { + batchHolder->put(std::move(nik), std::string(niv)); } } } @@ -364,33 +322,17 @@ ErrorOr AddVerticesProcessor::findOldValue } } -std::string AddVerticesProcessor::indexKey(PartitionID partId, - const VertexID& vId, - RowReader* reader, - std::shared_ptr index) { - auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); - if (!values.ok()) { - return ""; - } - - return IndexKeyUtils::vertexIndexKey( - spaceVidLen_, partId, index->get_index_id(), vId, std::move(values).value()); -} - -std::vector AddVerticesProcessor::indexKeysForGeography( +std::vector AddVerticesProcessor::indexKeys( PartitionID partId, const VertexID& vId, RowReader* reader, std::shared_ptr index) { - const auto& fields = index->get_fields(); - DCHECK(fields.size() == 1 && - fields.back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY); - auto values = IndexKeyUtils::collectIndexValueForGeography(reader, fields.back()); + auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); if (!values.ok()) { return {}; } - return IndexKeyUtils::vertexIndexKeysForGeography( + return IndexKeyUtils::vertexIndexKeys( spaceVidLen_, partId, index->get_index_id(), vId, std::move(values).value()); } diff --git a/src/storage/mutate/AddVerticesProcessor.h b/src/storage/mutate/AddVerticesProcessor.h index 0ec041ac212..3baf8e5ebcf 100644 --- a/src/storage/mutate/AddVerticesProcessor.h +++ b/src/storage/mutate/AddVerticesProcessor.h @@ -39,16 +39,10 @@ class AddVerticesProcessor : public BaseProcessor { const VertexID& vId, TagID tagId); - std::string indexKey(PartitionID partId, - const VertexID& vId, - RowReader* reader, - std::shared_ptr index); - - std::vector indexKeysForGeography( - PartitionID partId, - const VertexID& vId, - RowReader* reader, - std::shared_ptr index); + std::vector indexKeys(PartitionID partId, + const VertexID& vId, + RowReader* reader, + std::shared_ptr index); private: GraphSpaceID spaceId_; diff --git a/src/storage/mutate/DeleteEdgesProcessor.cpp b/src/storage/mutate/DeleteEdgesProcessor.cpp index ced5e6494a9..4bf1b54b87e 100644 --- a/src/storage/mutate/DeleteEdgesProcessor.cpp +++ b/src/storage/mutate/DeleteEdgesProcessor.cpp @@ -162,46 +162,24 @@ ErrorOr DeleteEdgesProcessor::deleteEdges( return nebula::cpp2::ErrorCode::E_INVALID_DATA; } } - if (index->get_fields().size() == 1 && index->get_fields().back().get_type().get_type() == - meta::cpp2::PropertyType::GEOGRAPHY) { - auto valuesRet = IndexKeyUtils::collectIndexValueForGeography( - reader.get(), index->get_fields().back()); - if (!valuesRet.ok()) { - continue; - } - auto indexKeys = IndexKeyUtils::edgeIndexKeysForGeography( - spaceVidLen_, partId, indexId, srcId, rank, dstId, std::move(valuesRet).value()); - - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - for (auto& indexKey : indexKeys) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(indexKey)); - } - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - } else { - for (auto& indexKey : indexKeys) { - batchHolder->remove(std::move(indexKey)); - } + auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), index->get_fields()); + if (!valuesRet.ok()) { + continue; + } + auto indexKeys = IndexKeyUtils::edgeIndexKeys( + spaceVidLen_, partId, indexId, srcId, rank, dstId, std::move(valuesRet).value()); + + auto indexState = env_->getIndexState(spaceId_, partId); + if (env_->checkRebuilding(indexState)) { + auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); + for (auto& indexKey : indexKeys) { + batchHolder->put(std::string(deleteOpKey), std::move(indexKey)); } + } else if (env_->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; } else { - auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), index->get_fields()); - if (!valuesRet.ok()) { - continue; - } - auto indexKey = IndexKeyUtils::edgeIndexKey( - spaceVidLen_, partId, indexId, srcId, rank, dstId, std::move(valuesRet).value()); - - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(indexKey)); - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - } else { + for (auto& indexKey : indexKeys) { batchHolder->remove(std::move(indexKey)); } } diff --git a/src/storage/mutate/DeleteTagsProcessor.cpp b/src/storage/mutate/DeleteTagsProcessor.cpp index b856504e746..311587d7b61 100644 --- a/src/storage/mutate/DeleteTagsProcessor.cpp +++ b/src/storage/mutate/DeleteTagsProcessor.cpp @@ -128,18 +128,22 @@ ErrorOr DeleteTagsProcessor::deleteTags( if (!valuesRet.ok()) { return nebula::cpp2::ErrorCode::E_INVALID_DATA; } - auto indexKey = IndexKeyUtils::vertexIndexKey( + auto indexKeys = IndexKeyUtils::vertexIndexKeys( spaceVidLen_, partId, indexId, vId, std::move(valuesRet).value()); // Check the index is building for the specified partition or not auto indexState = env_->getIndexState(spaceId_, partId); if (env_->checkRebuilding(indexState)) { auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(indexKey)); + for (auto& indexKey : indexKeys) { + batchHolder->put(std::string(deleteOpKey), std::move(indexKey)); + } } else if (env_->checkIndexLocked(indexState)) { return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; } else { - batchHolder->remove(std::move(indexKey)); + for (auto& indexKey : indexKeys) { + batchHolder->remove(std::move(indexKey)); + } } } } diff --git a/src/storage/mutate/DeleteVerticesProcessor.cpp b/src/storage/mutate/DeleteVerticesProcessor.cpp index 3edf09d95ae..617bafa0db9 100644 --- a/src/storage/mutate/DeleteVerticesProcessor.cpp +++ b/src/storage/mutate/DeleteVerticesProcessor.cpp @@ -146,48 +146,25 @@ ErrorOr DeleteVerticesProcessor::deleteVer } } const auto& cols = index->get_fields(); - if (cols.size() == 1 && - cols.back().get_type().get_type() == meta::cpp2::PropertyType::GEOGRAPHY) { - auto valuesRet = - IndexKeyUtils::collectIndexValueForGeography(reader.get(), cols.back()); - if (!valuesRet.ok()) { - continue; - } - auto indexKeys = IndexKeyUtils::vertexIndexKeysForGeography( - spaceVidLen_, partId, indexId, vertex.getStr(), std::move(valuesRet).value()); - - // Check the index is building for the specified partition or not - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - for (auto& indexKey : indexKeys) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(indexKey)); - } - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - } else { - for (auto& indexKey : indexKeys) { - batchHolder->remove(std::move(indexKey)); - } + auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), cols); + if (!valuesRet.ok()) { + continue; + } + auto indexKeys = IndexKeyUtils::vertexIndexKeys( + spaceVidLen_, partId, indexId, vertex.getStr(), std::move(valuesRet).value()); + + // Check the index is building for the specified partition or not + auto indexState = env_->getIndexState(spaceId_, partId); + if (env_->checkRebuilding(indexState)) { + auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); + for (auto& indexKey : indexKeys) { + batchHolder->put(std::string(deleteOpKey), std::move(indexKey)); } + } else if (env_->checkIndexLocked(indexState)) { + LOG(ERROR) << "The index has been locked: " << index->get_index_name(); + return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; } else { - auto valuesRet = IndexKeyUtils::collectIndexValues(reader.get(), cols); - if (!valuesRet.ok()) { - continue; - } - auto indexKey = IndexKeyUtils::vertexIndexKey( - spaceVidLen_, partId, indexId, vertex.getStr(), std::move(valuesRet).value()); - - // Check the index is building for the specified partition or not - auto indexState = env_->getIndexState(spaceId_, partId); - if (env_->checkRebuilding(indexState)) { - auto deleteOpKey = OperationKeyUtils::deleteOperationKey(partId); - batchHolder->put(std::move(deleteOpKey), std::move(indexKey)); - } else if (env_->checkIndexLocked(indexState)) { - LOG(ERROR) << "The index has been locked: " << index->get_index_name(); - return nebula::cpp2::ErrorCode::E_DATA_CONFLICT_ERROR; - } else { + for (auto& indexKey : indexKeys) { batchHolder->remove(std::move(indexKey)); } } diff --git a/src/storage/test/IndexScanLimitTest.cpp b/src/storage/test/IndexScanLimitTest.cpp index 3dd2482c339..a73e0186a80 100644 --- a/src/storage/test/IndexScanLimitTest.cpp +++ b/src/storage/test/IndexScanLimitTest.cpp @@ -124,24 +124,28 @@ class IndexScanLimitTest : public ::testing::Test { data.emplace_back(std::move(vertexKey), std::move(val)); if (indexMan_ != nullptr) { if (indexMan_->getTagIndex(spaceId, tagIndex).ok()) { - auto vertexIndexKey = - IndexKeyUtils::vertexIndexKey(vertexLen, - pId, - tagIndex, - vertex, - IndexKeyUtils::encodeValues({col1Val}, genCols())); - data.emplace_back(std::move(vertexIndexKey), ""); + auto vertexIndexKeys = + IndexKeyUtils::vertexIndexKeys(vertexLen, + pId, + tagIndex, + vertex, + IndexKeyUtils::encodeValues({col1Val}, genCols())); + for (auto& vertexIndexKey : vertexIndexKeys) { + data.emplace_back(std::move(vertexIndexKey), ""); + } } if (indexMan_->getEdgeIndex(spaceId, edgeIndex).ok()) { - auto edgeIndexKey = - IndexKeyUtils::edgeIndexKey(vertexLen, - pId, - edgeIndex, - vertex, - 0, - vertex, - IndexKeyUtils::encodeValues({col1Val}, genCols())); - data.emplace_back(std::move(edgeIndexKey), ""); + auto edgeIndexKeys = + IndexKeyUtils::edgeIndexKeys(vertexLen, + pId, + edgeIndex, + vertex, + 0, + vertex, + IndexKeyUtils::encodeValues({col1Val}, genCols())); + for (auto& edgeIndexKey : edgeIndexKeys) { + data.emplace_back(std::move(edgeIndexKey), ""); + } } } } diff --git a/src/storage/test/IndexWriteTest.cpp b/src/storage/test/IndexWriteTest.cpp index 8b08c5384e4..7a8dc0bd85b 100644 --- a/src/storage/test/IndexWriteTest.cpp +++ b/src/storage/test/IndexWriteTest.cpp @@ -319,13 +319,13 @@ TEST(IndexTest, VerticesValueTest) { values.emplace_back(Value(date)); // col_date_null values.emplace_back(nullValue); - auto index = + auto indexes = IndexKeyUtils::encodeValues(std::move(values), mock::MockData::mockTypicaIndexColumns()); for (auto partId = 1; partId <= 6; partId++) { auto prefix = IndexKeyUtils::indexPrefix(partId, indexId); - auto indexKey = IndexKeyUtils::vertexIndexKey( - vIdLen, partId, indexId, convertVertexId(vIdLen, partId), std::move(index)); + auto indexKey = IndexKeyUtils::vertexIndexKeys( + vIdLen, partId, indexId, convertVertexId(vIdLen, partId), std::move(indexes))[0]; std::unique_ptr iter; auto ret = env->kvstore_->prefix(spaceId, partId, prefix, &iter); EXPECT_EQ(nebula::cpp2::ErrorCode::SUCCEEDED, ret); diff --git a/src/storage/test/LookupIndexTest.cpp b/src/storage/test/LookupIndexTest.cpp index c9d35dc892e..1f0fc4986d2 100644 --- a/src/storage/test/LookupIndexTest.cpp +++ b/src/storage/test/LookupIndexTest.cpp @@ -94,11 +94,15 @@ TEST_P(LookupIndexTest, LookupIndexTestV1) { indexVal1.append(IndexKeyUtils::encodeValue("row1")); std::string indexVal2 = indexVal1; - key = IndexKeyUtils::vertexIndexKey(vIdLen.value(), 1, 3, vId1, std::move(indexVal1)); - keyValues.emplace_back(std::move(key), ""); + auto keys = IndexKeyUtils::vertexIndexKeys(vIdLen.value(), 1, 3, vId1, {std::move(indexVal1)}); + for (auto& k : keys) { + keyValues.emplace_back(std::move(k), ""); + } - key = IndexKeyUtils::vertexIndexKey(vIdLen.value(), 1, 3, vId2, std::move(indexVal2)); - keyValues.emplace_back(std::move(key), ""); + keys = IndexKeyUtils::vertexIndexKeys(vIdLen.value(), 1, 3, vId2, {std::move(indexVal2)}); + for (auto& k : keys) { + keyValues.emplace_back(std::move(k), ""); + } folly::Baton baton; env->kvstore_->asyncMultiPut( diff --git a/src/storage/test/QueryTestUtils.h b/src/storage/test/QueryTestUtils.h index 28cd5872a54..b6cbca6037a 100644 --- a/src/storage/test/QueryTestUtils.h +++ b/src/storage/test/QueryTestUtils.h @@ -260,9 +260,12 @@ class QueryTestUtils { row.append(IndexKeyUtils::encodeValue(v)); } } - auto index = IndexKeyUtils::vertexIndexKey(spaceVidLen, partId, indexId, vId, std::move(row)); + auto indexes = + IndexKeyUtils::vertexIndexKeys(spaceVidLen, partId, indexId, vId, {std::move(row)}); auto val = FLAGS_mock_ttl_col ? IndexKeyUtils::indexVal(time::WallClock::fastNowInSec()) : ""; - data.emplace_back(std::move(index), std::move(val)); + for (auto& index : indexes) { + data.emplace_back(std::move(index), std::move(val)); + } } static void encodeEdgeIndex(size_t spaceVidLen, @@ -283,10 +286,12 @@ class QueryTestUtils { row.append(IndexKeyUtils::encodeValue(v)); } } - auto index = IndexKeyUtils::edgeIndexKey( - spaceVidLen, partId, indexId, srcId, rank, dstId, std::move(row)); + auto indexes = IndexKeyUtils::edgeIndexKeys( + spaceVidLen, partId, indexId, srcId, rank, dstId, {std::move(row)}); auto val = FLAGS_mock_ttl_col ? IndexKeyUtils::indexVal(time::WallClock::fastNowInSec()) : ""; - data.emplace_back(std::move(index), std::move(val)); + for (auto& index : indexes) { + data.emplace_back(std::move(index), val); + } } static cpp2::GetNeighborsRequest buildRequest( diff --git a/src/tools/db-upgrade/DbUpgrader.cpp b/src/tools/db-upgrade/DbUpgrader.cpp index abf39881a5e..e1d5664203a 100644 --- a/src/tools/db-upgrade/DbUpgrader.cpp +++ b/src/tools/db-upgrade/DbUpgrader.cpp @@ -656,8 +656,8 @@ void UpgraderSpace::encodeVertexValue(PartitionID partId, return; } for (auto& index : it->second) { - auto newIndexKey = indexVertexKey(partId, strVid, nReader.get(), index); - if (!newIndexKey.empty()) { + auto newIndexKeys = indexVertexKeys(partId, strVid, nReader.get(), index); + for (auto& newIndexKey : newIndexKeys) { data.emplace_back(std::move(newIndexKey), ""); } } @@ -881,15 +881,16 @@ std::string UpgraderSpace::encodeRowVal(const RowReader* reader, return std::move(rowWrite).moveEncodedStr(); } -std::string UpgraderSpace::indexVertexKey(PartitionID partId, - VertexID& vId, - RowReader* reader, - std::shared_ptr index) { +std::vector UpgraderSpace::indexVertexKeys( + PartitionID partId, + VertexID& vId, + RowReader* reader, + std::shared_ptr index) { auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); if (!values.ok()) { - return ""; + return {}; } - return IndexKeyUtils::vertexIndexKey( + return IndexKeyUtils::vertexIndexKeys( spaceVidLen_, partId, index->get_index_id(), vId, std::move(values).value()); } @@ -926,25 +927,26 @@ void UpgraderSpace::encodeEdgeValue(PartitionID partId, return; } for (auto& index : it->second) { - auto newIndexKey = indexEdgeKey(partId, nReader.get(), svId, rank, dstId, index); - if (!newIndexKey.empty()) { + auto newIndexKeys = indexEdgeKeys(partId, nReader.get(), svId, rank, dstId, index); + for (auto& newIndexKey : newIndexKeys) { data.emplace_back(std::move(newIndexKey), ""); } } } } -std::string UpgraderSpace::indexEdgeKey(PartitionID partId, - RowReader* reader, - VertexID& svId, - EdgeRanking rank, - VertexID& dstId, - std::shared_ptr index) { +std::vector UpgraderSpace::indexEdgeKeys( + PartitionID partId, + RowReader* reader, + VertexID& svId, + EdgeRanking rank, + VertexID& dstId, + std::shared_ptr index) { auto values = IndexKeyUtils::collectIndexValues(reader, index->get_fields()); if (!values.ok()) { - return ""; + return {}; } - return IndexKeyUtils::edgeIndexKey( + return IndexKeyUtils::edgeIndexKeys( spaceVidLen_, partId, index->get_index_id(), svId, rank, dstId, std::move(values).value()); } diff --git a/src/tools/db-upgrade/DbUpgrader.h b/src/tools/db-upgrade/DbUpgrader.h index 7b0ab61aaec..e0ed041eb66 100644 --- a/src/tools/db-upgrade/DbUpgrader.h +++ b/src/tools/db-upgrade/DbUpgrader.h @@ -82,10 +82,10 @@ class UpgraderSpace { const meta::NebulaSchemaProvider* schema, std::vector& fieldName); - std::string indexVertexKey(PartitionID partId, - VertexID& vId, - RowReader* reader, - std::shared_ptr index); + std::vector indexVertexKeys(PartitionID partId, + VertexID& vId, + RowReader* reader, + std::shared_ptr index); void encodeEdgeValue(PartitionID partId, RowReader* reader, @@ -97,12 +97,12 @@ class UpgraderSpace { VertexID& dstId, std::vector& data); - std::string indexEdgeKey(PartitionID partId, - RowReader* reader, - VertexID& svId, - EdgeRanking rank, - VertexID& dstId, - std::shared_ptr index); + std::vector indexEdgeKeys(PartitionID partId, + RowReader* reader, + VertexID& svId, + EdgeRanking rank, + VertexID& dstId, + std::shared_ptr index); WriteResult convertValue(const meta::NebulaSchemaProvider* newSchema, const meta::SchemaProviderIf* oldSchema, diff --git a/tests/tck/features/geo/GeoBase.feature b/tests/tck/features/geo/GeoBase.feature index fc98f316f9e..848d90a8466 100644 --- a/tests/tck/features/geo/GeoBase.feature +++ b/tests/tck/features/geo/GeoBase.feature @@ -67,6 +67,7 @@ Feature: Geo base Then the result should be, in any order: | Tag | Create Tag | | "only_point" | 'CREATE TAG `only_point` (\n `geo` geography(point) NULL\n) ttl_duration = 0, ttl_col = ""' | + Then drop the used space Scenario: test geo CURD # Any geo shape(point/linestring/polygon) is allowed to insert to the column geography @@ -702,3 +703,4 @@ Feature: Geo base DROP EDGE any_shape_edge; """ Then the execution should be successful + Then drop the used space