Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@jievince
jievince committed Oct 20, 2021
1 parent 79b1cf3 commit 2940bdc
Show file tree
Hide file tree
Showing 29 changed files with 486 additions and 817 deletions.
8 changes: 4 additions & 4 deletions src/common/datatypes/test/GeographyTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -46,15 +46,15 @@ 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();
std::string got = g.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();
Expand Down
4 changes: 2 additions & 2 deletions src/common/geo/GeoIndex.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/common/geo/GeoIndex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions src/common/geo/GeoUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ class GeoUtils final {
std::vector<std::unique_ptr<S2Loop>> 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<S2Loop>(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<S2Polygon>(std::move(s2Loops), S2Debug::DISABLE);
Expand All @@ -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();
}

Expand Down
6 changes: 4 additions & 2 deletions src/common/geo/io/wkt/WKTWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ void WKTWriter::writeCoordinateList(std::string& wkt,
const std::vector<Coordinate>& 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(
Expand All @@ -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 {
Expand Down
166 changes: 54 additions & 112 deletions src/common/utils/IndexKeyUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,90 +11,73 @@
namespace nebula {

// static
std::string IndexKeyUtils::encodeValues(std::vector<Value>&& values,
const std::vector<nebula::meta::cpp2::ColumnDef>& cols) {
std::vector<std::string> IndexKeyUtils::encodeValues(
std::vector<Value>&& values, const std::vector<nebula::meta::cpp2::ColumnDef>& 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<std::string> 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<size_t>(*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<size_t>(*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<const char*>(&nullableBitSet), sizeof(u_short));
}
return index;
}

std::vector<std::string> 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<std::string> 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<const char*>(&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<uint32_t>(NebulaKeyType::kIndex);
std::string key;
key.reserve(256);
key.append(reinterpret_cast<const char*>(&item), sizeof(int32_t))
.append(reinterpret_cast<const char*>(&indexId), sizeof(IndexID))
.append(values)
.append(vId.data(), vId.size())
.append(vIdLen - vId.size(), '\0');
return key;
}

// static
std::vector<std::string> IndexKeyUtils::vertexIndexKeysForGeography(
size_t vIdLen,
PartitionID partId,
IndexID indexId,
const VertexID& vId,
std::vector<std::string>&& values) {
std::vector<std::string> IndexKeyUtils::vertexIndexKeys(size_t vIdLen,
PartitionID partId,
IndexID indexId,
const VertexID& vId,
std::vector<std::string>&& values) {
int32_t item = (partId << kPartitionOffset) | static_cast<uint32_t>(NebulaKeyType::kIndex);
std::vector<std::string> keys;
keys.reserve(values.size());
for (const auto& value : values) {
std::string key;
key.reserve(256);
Expand All @@ -109,38 +92,16 @@ std::vector<std::string> 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<uint32_t>(NebulaKeyType::kIndex);
std::string key;
key.reserve(256);
key.append(reinterpret_cast<const char*>(&item), sizeof(int32_t))
.append(reinterpret_cast<const char*>(&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<std::string> IndexKeyUtils::edgeIndexKeysForGeography(
size_t vIdLen,
PartitionID partId,
IndexID indexId,
const VertexID& srcId,
EdgeRanking rank,
const VertexID& dstId,
std::vector<std::string>&& values) {
std::vector<std::string> IndexKeyUtils::edgeIndexKeys(size_t vIdLen,
PartitionID partId,
IndexID indexId,
const VertexID& srcId,
EdgeRanking rank,
const VertexID& dstId,
std::vector<std::string>&& values) {
int32_t item = (partId << kPartitionOffset) | static_cast<uint32_t>(NebulaKeyType::kIndex);
std::vector<std::string> keys;
keys.reserve(values.size());
for (const auto& value : values) {
std::string key;
key.reserve(256);
Expand Down Expand Up @@ -196,7 +157,7 @@ Value IndexKeyUtils::parseIndexTTL(const folly::StringPiece& raw) {
}

// static
StatusOr<std::string> IndexKeyUtils::collectIndexValues(
StatusOr<std::vector<std::string>> IndexKeyUtils::collectIndexValues(
RowReader* reader, const std::vector<nebula::meta::cpp2::ColumnDef>& cols) {
if (reader == nullptr) {
return Status::Error("Invalid row reader");
Expand All @@ -215,25 +176,6 @@ StatusOr<std::string> IndexKeyUtils::collectIndexValues(
return encodeValues(std::move(values), cols);
}

// TODO(jie) Should be refactored
// static
StatusOr<std::vector<std::string>> 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()) {
Expand Down
Loading

0 comments on commit 2940bdc

Please sign in to comment.