Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spatial join area #1713

Open
wants to merge 50 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
c7a1ea8
untested changes
Jonathan24680 Dec 17, 2024
a266201
continued development
Jonathan24680 Dec 18, 2024
2cc8504
added all areas to the dataset and some tests
Jonathan24680 Dec 23, 2024
d784513
make file_buffer_size in the test index be a parameter
Jonathan24680 Dec 28, 2024
f6e1647
bugfix area dataset
Jonathan24680 Dec 28, 2024
1a4ff8f
backup commit
Jonathan24680 Dec 30, 2024
e3a2644
area test with points and areas working
Jonathan24680 Dec 31, 2024
f38cb1d
moved some stuff from dev space into the project
Jonathan24680 Jan 2, 2025
4d71c4e
new tests
Jonathan24680 Jan 4, 2025
38c9585
first test works now for the areas as well
Jonathan24680 Jan 11, 2025
ddea6d3
all tests are now executed and passed
Jonathan24680 Jan 12, 2025
1902d44
clang format
Jonathan24680 Jan 12, 2025
31e7054
merge master
Jonathan24680 Jan 12, 2025
cba4324
backup with removed filebuffersize compiling but not testing
Jonathan24680 Jan 17, 2025
d5c934c
make buffer size of test index variable
Jonathan24680 Jan 17, 2025
c1dc963
clean up code
Jonathan24680 Jan 17, 2025
c5284ef
sonarqube
Jonathan24680 Jan 20, 2025
fc1a1fe
correct bug for function without inline in header file
Jonathan24680 Jan 21, 2025
ec75e89
arbitrary geometry types and areas dont need to be approximated as po…
Jonathan24680 Jan 25, 2025
4fd698c
PR feedback
Jonathan24680 Jan 31, 2025
173e646
PR feedback
Jonathan24680 Jan 31, 2025
7dd8156
backup
Jonathan24680 Feb 1, 2025
1c60bf9
PR feedback
Jonathan24680 Feb 1, 2025
3b8ea1a
spelling mistake
Jonathan24680 Feb 1, 2025
a121ef2
Sonarqube issues
Jonathan24680 Feb 4, 2025
1168f32
PR changes
Jonathan24680 Feb 4, 2025
ca7f08e
PR changes
Jonathan24680 Feb 4, 2025
9d2956e
PR changes
Jonathan24680 Feb 4, 2025
14032bd
PR changes
Jonathan24680 Feb 4, 2025
29171b5
Sonarqube issues
Jonathan24680 Feb 4, 2025
457f8d4
format check
Jonathan24680 Feb 4, 2025
bf886c4
backup
Jonathan24680 Feb 7, 2025
10bdf7b
strange bug
Jonathan24680 Feb 7, 2025
5f12f22
solved strange bug
Jonathan24680 Feb 7, 2025
978b512
PR feedback
Jonathan24680 Feb 7, 2025
4132701
Sonarqube issue
Jonathan24680 Feb 7, 2025
d711586
Merge branch 'master' into SpatialJoinArea
Jonathan24680 Feb 7, 2025
f3c5b01
consistent naming of QueryBox and BoundingBox
Jonathan24680 Feb 8, 2025
a46fccd
add test for distance between GeoPoints and Areas
Jonathan24680 Feb 8, 2025
2cc0fef
improve true area distance calculation
Jonathan24680 Feb 8, 2025
96a903a
backup
Jonathan24680 Feb 10, 2025
47ba454
backup
Jonathan24680 Feb 10, 2025
61c6b98
PR changes
Jonathan24680 Feb 10, 2025
c0bf282
clang format
Jonathan24680 Feb 10, 2025
988ba8d
Sonarqube issus and bugfix
Jonathan24680 Feb 12, 2025
e42d9d0
format check
Jonathan24680 Feb 12, 2025
5b88f47
Merge branch 'master' into SpatialJoinArea
Jonathan24680 Feb 14, 2025
5a99080
Sonarqube issue remove unused operator
Jonathan24680 Feb 14, 2025
a7f4695
PR feedback
Jonathan24680 Feb 15, 2025
e5121f6
suggested parentheses around and condition
Jonathan24680 Feb 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 93 additions & 85 deletions src/engine/SpatialJoinAlgorithms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,22 @@
if (pos1 != std::string::npos && pos2 != std::string::npos) {
return extractFrom.substr(pos1 + 1, pos2 - pos1 - 1);
} else {
return extractFrom;
}

Check warning on line 50 in src/engine/SpatialJoinAlgorithms.cpp

View check run for this annotation

Codecov / codecov/patch

src/engine/SpatialJoinAlgorithms.cpp#L49-L50

Added lines #L49 - L50 were not covered by tests
}

std::optional<AnyGeometry> SpatialJoinAlgorithms::getAnyGeometry(
const IdTable* idtable, size_t row, size_t col) const {
auto printWarning = [alreadyWarned = false,
&spatialJoin = spatialJoin_]() mutable {
if (!alreadyWarned) {
std::optional<size_t> SpatialJoinAlgorithms::getAnyGeometry(
const IdTable* idtable, size_t row, size_t col) {
auto printWarning = [this, &spatialJoin = spatialJoin_]() mutable {
if (this->nrOfFailedParsedGeometries_ == 0) {
std::string warning =
"The input to a spatial join contained at least one element, "
"that is not a Point, Linestring, Polygon, MultiPoint, "
"MultiLinestring or MultiPolygon geometry and is thus skipped. Note "
"that QLever currently only accepts those geometries for "
"the spatial joins";
AD_LOG_WARN << warning << std::endl;
alreadyWarned = true;
this->nrOfFailedParsedGeometries_ += 1;
if (spatialJoin.has_value()) {
AD_CORRECTNESS_CHECK(spatialJoin.value() != nullptr);
spatialJoin.value()->addWarning(warning);
Expand All @@ -82,32 +81,37 @@
AnyGeometry geometry;
try {
bg::read_wkt(str, geometry);
geometries_.push_back(std::move(geometry));
} catch (...) {
printWarning();
return std::nullopt;
}
return geometry;
return geometries_.size() - 1; // index of the last element
}

// ____________________________________________________________________________
double SpatialJoinAlgorithms::computeDist(const AnyGeometry& geometry1,
const AnyGeometry& geometry2) const {
return boost::apply_visitor(DistanceVisitor(), geometry1, geometry2) * 78.630;
double SpatialJoinAlgorithms::computeDist(
const std::optional<size_t> geometryIndex1,
const std::optional<size_t> geometryIndex2) const {
return boost::apply_visitor(ClosestPointVisitor(),
geometries_.at(geometryIndex1.value()),
geometries_.at(geometryIndex2.value()));
};

// ____________________________________________________________________________
Point SpatialJoinAlgorithms::convertGeoPointToPoint(GeoPoint point) const {
return Point(point.getLng(), point.getLat());
size_t SpatialJoinAlgorithms::convertGeoPointToPoint(GeoPoint point) {
geometries_.emplace_back(Point(point.getLng(), point.getLat()));
return geometries_.size() - 1; // index of the last element
};

// ____________________________________________________________________________
Id SpatialJoinAlgorithms::computeDist(const rtreeEntry& geo1,
const rtreeEntry& geo2) const {
auto convertPoint = [&](const AnyGeometry& geometry,
const std::optional<Box>& bbox) {
Box areaBox;
areaBox =
bbox.value_or(boost::apply_visitor(BoundingBoxVisitor(), geometry));
Id SpatialJoinAlgorithms::computeDist(rtreeEntry& geo1, rtreeEntry& geo2) {
auto convertPoint = [&](size_t geometryIndex, std::optional<Box> bbox) {
if (!bbox.has_value()) {
bbox = boost::apply_visitor(BoundingBoxVisitor(),
geometries_.at(geometryIndex));
}

Check warning on line 113 in src/engine/SpatialJoinAlgorithms.cpp

View check run for this annotation

Codecov / codecov/patch

src/engine/SpatialJoinAlgorithms.cpp#L111-L113

Added lines #L111 - L113 were not covered by tests
const auto& areaBox = bbox.value();
Point p = calculateMidpointOfBox(areaBox);
return GeoPoint(p.get<1>(), p.get<0>());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think below the code gets much more readable if you do
if (!useMidpoint) { return computeDist(index1, index2);}; //.
Then it becomes much clearer that the whole complexity is just for the midpoint case (is this actually useful still?)
If you want to keep the midpoint algorithm then you can move the code into a separate function computeDistUsingMidpoints and then the actual computeDist becomes very short and readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't get what you mean. The if else code blocks in the current implementation are equally large and complex for both cases.

};
Expand All @@ -116,30 +120,32 @@
auto point1 = geo1.geoPoint_;
auto point2 = geo2.geoPoint_;
if (!point1) {
point1 = convertPoint(geo1.geometry_.value(), geo1.boundingBox_);
point1 =
convertPoint(geo1.geometryIndex_.value(), geo1.boundingBox_.value());
}
if (!point2) {
point2 = convertPoint(geo2.geometry_.value(), geo2.boundingBox_);
point2 =
convertPoint(geo2.geometryIndex_.value(), geo2.boundingBox_.value());
}
return Id::makeFromDouble(
ad_utility::detail::wktDistImpl(point1.value(), point2.value()));
} else {
if (geo1.geoPoint_.has_value() && geo2.geoPoint_.has_value()) {
return Id::makeFromDouble(ad_utility::detail::wktDistImpl(
geo1.geoPoint_.value(), geo2.geoPoint_.value()));
} else if (geo1.geometry_.has_value() && geo2.geometry_.has_value()) {
} else if (geo1.geometryIndex_.has_value() &&
geo2.geometryIndex_.has_value()) {
return Id::makeFromDouble(
computeDist(geo1.geometry_.value(), geo2.geometry_.value()));
computeDist(geo1.geometryIndex_, geo2.geometryIndex_));
} else {
// one point and one area
std::optional<AnyGeometry> geom1 = geo1.geometry_;
std::optional<AnyGeometry> geom2 = geo2.geometry_;
if (geo1.geoPoint_.has_value()) {
geom1 = convertGeoPointToPoint(geo1.geoPoint_.value());
} else if (geo2.geoPoint_.has_value()) {
geom2 = convertGeoPointToPoint(geo2.geoPoint_.value());
if (!geo1.geometryIndex_) {
geo1.geometryIndex_ = convertGeoPointToPoint(geo1.geoPoint_.value());
} else if (!geo2.geometryIndex_) {
geo2.geometryIndex_ = convertGeoPointToPoint(geo2.geoPoint_.value());
}
return Id::makeFromDouble(computeDist(geom1.value(), geom2.value()));
return Id::makeFromDouble(
computeDist(geo1.geometryIndex_, geo2.geometryIndex_));
}
}
}
Expand All @@ -149,35 +155,32 @@
const IdTable* idTableRight,
size_t rowLeft, size_t rowRight,
ColumnIndex leftPointCol,
ColumnIndex rightPointCol) const {
auto getAreaOrPointGeometry = [&](const IdTable* idtable, size_t row,
size_t col, std::optional<GeoPoint> point) {
std::optional<AnyGeometry> geometry;

ColumnIndex rightPointCol) {
auto getAreaOrPointGeometry =
[&](const IdTable* idtable, size_t row, size_t col,
std::optional<GeoPoint> point) -> std::optional<size_t> {
if (!point) {
// nothing to do. When parsing a point or an area fails, a warning
// message gets printed at another place and the point/area just gets
// skipped
geometry = getAnyGeometry(idtable, row, col);
} else {
geometry = convertGeoPointToPoint(point.value());
return getAnyGeometry(idtable, row, col);
}

return geometry;
return convertGeoPointToPoint(point.value());
};

// for now we need to get the data from the disk, but in the future, this
// information will be stored in an ID, just like GeoPoint
auto getAreaPoint = [&](const IdTable* idtable, size_t row,
size_t col) -> std::optional<GeoPoint> {
std::optional<AnyGeometry> geometry = getAnyGeometry(idtable, row, col);
if (!geometry.has_value()) {
std::optional<size_t> geometryIndex = getAnyGeometry(idtable, row, col);
if (!geometryIndex) {
// nothing to do. When parsing a point or an area fails, a warning message
// gets printed at another place and the point/area just gets skipped
return std::nullopt;
}

Box areaBox = boost::apply_visitor(BoundingBoxVisitor(), geometry.value());
Box areaBox = boost::apply_visitor(BoundingBoxVisitor(),
geometries_.at(geometryIndex.value()));

Point p = calculateMidpointOfBox(areaBox);
return GeoPoint(p.get<1>(), p.get<0>());
Expand All @@ -203,15 +206,16 @@
return Id::makeUndefined();
}
} else {
entryLeft.geometry_ = getAreaOrPointGeometry(
entryLeft.geometryIndex_ = getAreaOrPointGeometry(
idTableLeft, rowLeft, leftPointCol, entryLeft.geoPoint_);
entryRight.geometry_ = getAreaOrPointGeometry(
entryRight.geometryIndex_ = getAreaOrPointGeometry(
idTableRight, rowRight, rightPointCol, entryRight.geoPoint_);
if (entryLeft.geometry_ && entryRight.geometry_) {
if (entryLeft.geometryIndex_.has_value() &&
entryRight.geometryIndex_.has_value()) {
return computeDist(entryLeft, entryRight);
} else {
return Id::makeUndefined();
}

Check warning on line 218 in src/engine/SpatialJoinAlgorithms.cpp

View check run for this annotation

Codecov / codecov/patch

src/engine/SpatialJoinAlgorithms.cpp#L217-L218

Added lines #L217 - L218 were not covered by tests
}
}

Expand Down Expand Up @@ -403,7 +407,7 @@
}

// ____________________________________________________________________________
std::vector<Box> SpatialJoinAlgorithms::computeBoundingBox(
std::vector<Box> SpatialJoinAlgorithms::computeQueryBox(
const Point& startPoint, double additionalDist) const {
const auto [idTableLeft, resultLeft, idTableRight, resultRight, leftJoinCol,
rightJoinCol, rightSelectedCols, numColumns, maxDist,
Expand Down Expand Up @@ -434,7 +438,7 @@
// a single bounding box for the whole planet, do an optimized version
if (static_cast<double>(maxDist.value()) > circumferenceMax_ / 4.0 &&
static_cast<double>(maxDist.value()) < circumferenceMax_ / 2.01) {
return computeBoundingBoxForLargeDistances(startPoint);
return computeQueryBoxForLargeDistances(startPoint);
}

// compute latitude bound
Expand Down Expand Up @@ -487,7 +491,7 @@
}

// ____________________________________________________________________________
std::vector<Box> SpatialJoinAlgorithms::computeBoundingBoxForLargeDistances(
std::vector<Box> SpatialJoinAlgorithms::computeQueryBoxForLargeDistances(
const Point& startPoint) const {
const auto [idTableLeft, resultLeft, idTableRight, resultRight, leftJoinCol,
rightJoinCol, rightSelectedCols, numColumns, maxDist,
Expand Down Expand Up @@ -628,29 +632,44 @@
}

// ____________________________________________________________________________
Result SpatialJoinAlgorithms::BoundingBoxAlgorithm() {
auto getRtreeEntry = [&](const IdTable* idTable, const size_t row,
const ColumnIndex col) -> std::optional<rtreeEntry> {
rtreeEntry entry{row, std::nullopt, std::nullopt, std::nullopt};
entry.geoPoint_ = getPoint(idTable, row, col);

if (!entry.geoPoint_) {
entry.geometry_ = getAnyGeometry(idTable, row, col);
if (!entry.geometry_.has_value()) {
return std::nullopt;
}
entry.boundingBox_ =
boost::apply_visitor(BoundingBoxVisitor(), entry.geometry_.value());
} else {
entry.boundingBox_ =
Box(Point(entry.geoPoint_.value().getLng(),
entry.geoPoint_.value().getLat()),
Point(entry.geoPoint_.value().getLng() + 0.00000001,
entry.geoPoint_.value().getLat() + 0.00000001));
std::optional<rtreeEntry> SpatialJoinAlgorithms::getRtreeEntry(
const IdTable* idTable, const size_t row, const ColumnIndex col) {
rtreeEntry entry{row, std::nullopt, std::nullopt, std::nullopt};
entry.geoPoint_ = getPoint(idTable, row, col);

if (!entry.geoPoint_) {
entry.geometryIndex_ = getAnyGeometry(idTable, row, col);
if (!entry.geometryIndex_) {
return std::nullopt;
}
return entry;
};
entry.boundingBox_ = boost::apply_visitor(
BoundingBoxVisitor(), geometries_.at(entry.geometryIndex_.value()));
} else {
entry.boundingBox_ =
Box(Point(entry.geoPoint_.value().getLng(),
entry.geoPoint_.value().getLat()),
Point(entry.geoPoint_.value().getLng() + 0.00000001,
entry.geoPoint_.value().getLat() + 0.00000001));
}
return entry;
}

// ____________________________________________________________________________
std::vector<Box> SpatialJoinAlgorithms::getQueryBox(
const std::optional<rtreeEntry>& entry) const {
if (!entry.value().geoPoint_) {
auto midpoint = calculateMidpointOfBox(entry.value().boundingBox_.value());
return computeQueryBox(midpoint,
getMaxDistFromMidpointToAnyPointInsideTheBox(
entry.value().boundingBox_.value(), midpoint));
} else {
return computeQueryBox(Point(entry.value().geoPoint_.value().getLng(),
entry.value().geoPoint_.value().getLat()));
}
}

// ____________________________________________________________________________
Result SpatialJoinAlgorithms::BoundingBoxAlgorithm() {
const auto [idTableLeft, resultLeft, idTableRight, resultRight, leftJoinCol,
rightJoinCol, rightSelectedCols, numColumns, maxDist,
maxResults] = params_;
Expand Down Expand Up @@ -683,8 +702,8 @@
// skipped
continue;
}
rtree.insert(std::make_pair(entry.value().boundingBox_.value(),
std::move(entry.value())));
rtree.insert(std::pair(entry.value().boundingBox_.value(),
std::move(entry.value())));
}

// query rtree with the other child
Expand All @@ -699,26 +718,15 @@
// skipped
continue;
}
std::vector<Box> queryBox;
if (!entry.value().geoPoint_) {
auto midpoint =
calculateMidpointOfBox(entry.value().boundingBox_.value());
queryBox = computeBoundingBox(
midpoint, getMaxDistFromMidpointToAnyPointInsideTheBox(
entry.value().boundingBox_.value(), midpoint));
} else {
queryBox =
computeBoundingBox(Point(entry.value().geoPoint_.value().getLng(),
entry.value().geoPoint_.value().getLat()));
}
std::vector<Box> queryBox = getQueryBox(entry);

results.clear();

ql::ranges::for_each(queryBox, [&](const Box& bbox) {
rtree.query(bgi::intersects(bbox), std::back_inserter(results));
});

ql::ranges::for_each(results, [&](const Value& res) {
ql::ranges::for_each(results, [&](Value& res) {
size_t rowLeft = res.second.row_;
size_t rowRight = i;
if (!leftResSmaller) {
Expand Down
Loading
Loading