Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jonathan24680
Jonathan24680 committed Nov 21, 2024
1 parent 7d8b98b commit 89491a1
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 66 deletions.
29 changes: 15 additions & 14 deletions src/engine/SpatialJoinAlgorithms.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <s2/util/units/length-units.h>

#include <cmath>

#include "util/GeoSparqlHelpers.h"

using namespace BoostGeometryNamespace;
Expand Down Expand Up @@ -232,7 +233,8 @@ std::vector<Box> SpatialJoinAlgorithms::computeBoundingBox(
const Point& startPoint) {
const auto [idTableLeft, resultLeft, idTableRight, resultRight, leftJoinCol,
rightJoinCol, numColumns, maxDist, maxResults] = params_;
AD_CORRECTNESS_CHECK(maxDist.has_value(), "Max distance must have a value for this operation");
AD_CORRECTNESS_CHECK(maxDist.has_value(),
"Max distance must have a value for this operation");
// haversine function
auto haversine = [](double theta) { return (1 - std::cos(theta)) / 2; };

Expand Down Expand Up @@ -313,7 +315,8 @@ std::vector<Box> SpatialJoinAlgorithms::computeBoundingBoxForLargeDistances(
const Point& startPoint) const {
const auto [idTableLeft, resultLeft, idTableRight, resultRight, leftJoinCol,
rightJoinCol, numColumns, maxDist, maxResults] = params_;
AD_CORRECTNESS_CHECK(maxDist.has_value(), "Max distance must have a value for this operation");
AD_CORRECTNESS_CHECK(maxDist.has_value(),
"Max distance must have a value for this operation");

// point on the opposite side of the globe
Point antiPoint(startPoint.get<0>() + 180, startPoint.get<1>() * -1);
Expand Down Expand Up @@ -429,16 +432,16 @@ std::array<bool, 2> SpatialJoinAlgorithms::isAPoleTouched(
// ____________________________________________________________________________
Result SpatialJoinAlgorithms::BoundingBoxAlgorithm() {
bool alreadyWarned = false;

auto printWarning = [&]() {
if (!alreadyWarned) {
AD_LOG_WARN
<< "expected a point here, but no point is given. Skipping this point and future 'non points' of this query"
<< std::endl;
AD_LOG_WARN << "expected a point here, but no point is given. Skipping "
"this point and future 'non points' of this query"
<< std::endl;
alreadyWarned = true;
}
};

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

View check run for this annotation

Codecov / codecov/patch

src/engine/SpatialJoinAlgorithms.cpp#L438-L443 

Added lines #L438 - L443 were not covered by tests

const auto [idTableLeft, resultLeft, idTableRight, resultRight, leftJoinCol,
rightJoinCol, numColumns, maxDist, maxResults] = params_;
IdTable result{numColumns, qec_->getAllocator()};
Expand All @@ -455,12 +458,10 @@ Result SpatialJoinAlgorithms::BoundingBoxAlgorithm() {
std::swap(smallerResJoinCol, otherResJoinCol);
}

// bgi::rtree<Value, bgi::quadratic<16>> rtree;
bgi::rtree<Value, bgi::quadratic<16>, bgi::indexable<Value>, bgi::equal_to<Value>, ad_utility::AllocatorWithLimit<Value>>
rtree(bgi::quadratic<16>{}, bgi::indexable<Value>{}, bgi::equal_to<Value>{}, qec_->getAllocator());
// bgi::rtree<Value, bgi::quadratic<16>, ad_utility::AllocatorWithLimit<Value>> rtree{qec_->getAllocator()};
//bgi::rtree<Value, bgi::quadratic<16>> dummyRtree;
//bgi::rtree<Value, bgi::quadratic<16>, ad_utility::AllocatorWithLimit<Value>> rtree(dummyRtree, qec_->getAllocator());
bgi::rtree<Value, bgi::quadratic<16>, bgi::indexable<Value>,
bgi::equal_to<Value>, ad_utility::AllocatorWithLimit<Value>>
rtree(bgi::quadratic<16>{}, bgi::indexable<Value>{},
bgi::equal_to<Value>{}, qec_->getAllocator());
for (size_t i = 0; i < smallerResult->numRows(); i++) {
// get point of row i
auto geopoint = getPoint(smallerResult, i, smallerResJoinCol);
Expand All @@ -476,7 +477,7 @@ Result SpatialJoinAlgorithms::BoundingBoxAlgorithm() {
rtree.insert(std::make_pair(std::move(p), i));
}
std::vector<Value, ad_utility::AllocatorWithLimit<Value>> results{
qec_->getAllocator()};
qec_->getAllocator()};
for (size_t i = 0; i < otherResult->numRows(); i++) {
auto geopoint1 = getPoint(otherResult, i, otherResJoinCol);

Expand Down
145 changes: 93 additions & 52 deletions test/engine/SpatialJoinAlgorithmsTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,32 +217,37 @@ class SpatialJoinParamTest
columnNames);
}

void testDiffSizeIdTables(std::string specialPredicate, bool addLeftChildFirst,
void testDiffSizeIdTables(
std::string specialPredicate, bool addLeftChildFirst,
std::vector<std::vector<std::string>> expectedOutput,
std::vector<std::string> columnNames, bool bigChildLeft) {
auto qec = buildTestQEC();
auto numTriples = qec->getIndex().numTriples().normal;
ASSERT_EQ(numTriples, 15);
// ====================== build small input =================================
TripleComponent point1{Variable{"?point1"}};
TripleComponent subject{ad_utility::triple_component::Iri::fromIriref("<geometry1>")};
auto smallChild = ad_utility::makeExecutionTree<IndexScan>(
qec, Permutation::Enum::PSO, SparqlTriple{subject, std::string{"<asWKT>"}, point1});
// ====================== build big input ==================================
auto bigChild = buildIndexScan(qec, {"?obj2", std::string{"<asWKT>"}, "?point2"});

auto firstChild = bigChildLeft ? bigChild : smallChild;
auto secondChild = bigChildLeft ? smallChild : bigChild;
auto firstVariable =
auto qec = buildTestQEC();
auto numTriples = qec->getIndex().numTriples().normal;
ASSERT_EQ(numTriples, 15);
// ====================== build small input
// =================================
TripleComponent point1{Variable{"?point1"}};
TripleComponent subject{
ad_utility::triple_component::Iri::fromIriref("<geometry1>")};
auto smallChild = ad_utility::makeExecutionTree<IndexScan>(
qec, Permutation::Enum::PSO,
SparqlTriple{subject, std::string{"<asWKT>"}, point1});
// ====================== build big input ==================================
auto bigChild =
buildIndexScan(qec, {"?obj2", std::string{"<asWKT>"}, "?point2"});

auto firstChild = bigChildLeft ? bigChild : smallChild;
auto secondChild = bigChildLeft ? smallChild : bigChild;
auto firstVariable =
bigChildLeft ? TripleComponent{Variable{"?point2"}} : point1;
auto secondVariable =
bigChildLeft ? point1 : TripleComponent{Variable{"?point2"}};
createAndTestSpatialJoin(

createAndTestSpatialJoin(
qec, SparqlTriple{firstVariable, specialPredicate, secondVariable},
firstChild, secondChild, addLeftChildFirst, expectedOutput,
columnNames);
}
}

protected:
bool useBaselineAlgorithm_;
Expand Down Expand Up @@ -592,25 +597,29 @@ std::vector<std::vector<std::string>> expectedMaxDist10000000_rows_diff{
mergeToRow(Eif, sLib, expectedDistEifLib)};

std::vector<std::vector<std::string>> expectedMaxDist1_rows_diffIDTable{
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf)
};
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf)};

std::vector<std::vector<std::string>> expectedMaxDist5000_rows_diffIDTable{
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf),
mergeToRow({sTF.at(1)}, sMun, expectedDistUniMun)
};
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf),
mergeToRow({sTF.at(1)}, sMun, expectedDistUniMun)};

std::vector<std::vector<std::string>> expectedMaxDist500000_rows_diffIDTable{
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf)
};
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf),
mergeToRow({sTF.at(1)}, sMun, expectedDistUniMun),
mergeToRow({sTF.at(1)}, sEif, expectedDistUniEif)};

std::vector<std::vector<std::string>> expectedMaxDist1000000_rows_diffIDTable{
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf)
};
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf),
mergeToRow({sTF.at(1)}, sMun, expectedDistUniMun),
mergeToRow({sTF.at(1)}, sEif, expectedDistUniEif),
mergeToRow({sTF.at(1)}, sEye, expectedDistUniEye)};

std::vector<std::vector<std::string>> expectedMaxDist10000000_rows_diffIDTable{
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf)
};
mergeToRow({sTF.at(1)}, sTF, expectedDistSelf),
mergeToRow({sTF.at(1)}, sMun, expectedDistUniMun),
mergeToRow({sTF.at(1)}, sEif, expectedDistUniEif),
mergeToRow({sTF.at(1)}, sEye, expectedDistUniEye),
mergeToRow({sTF.at(1)}, sLib, expectedDistUniLib)};

std::vector<std::vector<std::string>> expectedNearestNeighbors1{
mergeToRow(TF, TF, expectedDistSelf),
Expand Down Expand Up @@ -828,30 +837,62 @@ TEST_P(SpatialJoinParamTest, computeResultSmallDatasetDifferentSizeChildren) {
}

TEST_P(SpatialJoinParamTest, diffSizeIdTables) {
std::vector<std::string> columnNames{"?point1",
"?obj2",
"?point2",
std::vector<std::string> columnNames{"?point1", "?obj2", "?point2",
"?distOfTheTwoObjectsAddedInternally"};
/*testDiffSizeIdTables("<max-distance-in-meters:1>", true, expectedMaxDist1_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:1>", true, expectedMaxDist1_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:1>", false, expectedMaxDist1_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:1>", false, expectedMaxDist1_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:5000>", true, expectedMaxDist5000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:5000>", true, expectedMaxDist5000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:5000>", false, expectedMaxDist5000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:5000>", false, expectedMaxDist5000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:500000>", true, expectedMaxDist500000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:500000>", true, expectedMaxDist500000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:500000>", false, expectedMaxDist500000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:500000>", false, expectedMaxDist500000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", true, expectedMaxDist1000000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", true, expectedMaxDist1000000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", false, expectedMaxDist1000000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", false, expectedMaxDist1000000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", true, expectedMaxDist10000000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", true, expectedMaxDist10000000_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", false, expectedMaxDist10000000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", false, expectedMaxDist10000000_rows_diffIDTable, columnNames, false);*/
testDiffSizeIdTables("<max-distance-in-meters:1>", true,
expectedMaxDist1_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:1>", true,
expectedMaxDist1_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:1>", false,
expectedMaxDist1_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:1>", false,
expectedMaxDist1_rows_diffIDTable, columnNames, false);
testDiffSizeIdTables("<max-distance-in-meters:5000>", true,
expectedMaxDist5000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:5000>", true,
expectedMaxDist5000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:5000>", false,
expectedMaxDist5000_rows_diffIDTable, columnNames, true);
testDiffSizeIdTables("<max-distance-in-meters:5000>", false,
expectedMaxDist5000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:500000>", true,
expectedMaxDist500000_rows_diffIDTable, columnNames,
true);
testDiffSizeIdTables("<max-distance-in-meters:500000>", true,
expectedMaxDist500000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:500000>", false,
expectedMaxDist500000_rows_diffIDTable, columnNames,
true);
testDiffSizeIdTables("<max-distance-in-meters:500000>", false,
expectedMaxDist500000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", true,
expectedMaxDist1000000_rows_diffIDTable, columnNames,
true);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", true,
expectedMaxDist1000000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", false,
expectedMaxDist1000000_rows_diffIDTable, columnNames,
true);
testDiffSizeIdTables("<max-distance-in-meters:1000000>", false,
expectedMaxDist1000000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", true,
expectedMaxDist10000000_rows_diffIDTable, columnNames,
true);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", true,
expectedMaxDist10000000_rows_diffIDTable, columnNames,
false);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", false,
expectedMaxDist10000000_rows_diffIDTable, columnNames,
true);
testDiffSizeIdTables("<max-distance-in-meters:10000000>", false,
expectedMaxDist10000000_rows_diffIDTable, columnNames,
false);
}

INSTANTIATE_TEST_SUITE_P(
Expand Down

0 comments on commit 89491a1

Please sign in to comment.