Skip to content

Commit

Permalink
Fix query planner issue with two spatial joins sharing variables
Browse files Browse the repository at this point in the history
  • Loading branch information
ullingerc committed Nov 22, 2024
1 parent 9453e6a commit 39fd03d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 13 deletions.
31 changes: 22 additions & 9 deletions src/engine/QueryPlanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1827,6 +1827,13 @@ std::vector<QueryPlanner::SubtreePlan> QueryPlanner::createJoinCandidates(
return candidates;
}

// If both sides are spatial joins, return immediately to prevent a regular
// join on the variables, which would lead to the spatial join never having
// children.
if (checkSpatialJoin(a, b) == std::pair<bool, bool>{true, true}) {
return candidates;
}

if (a.type == SubtreePlan::MINUS) {
AD_THROW(
"MINUS can only appear after"
Expand Down Expand Up @@ -1897,25 +1904,31 @@ std::vector<QueryPlanner::SubtreePlan> QueryPlanner::createJoinCandidates(
}

// _____________________________________________________________________________
auto QueryPlanner::createSpatialJoin(
const SubtreePlan& a, const SubtreePlan& b,
const std::vector<std::array<ColumnIndex, 2>>& jcs)
-> std::optional<SubtreePlan> {
std::pair<bool, bool> QueryPlanner::checkSpatialJoin(const SubtreePlan& a,
const SubtreePlan& b) {
auto aIsSpatialJoin =
std::dynamic_pointer_cast<const SpatialJoin>(a._qet->getRootOperation());
auto bIsSpatialJoin =
std::dynamic_pointer_cast<const SpatialJoin>(b._qet->getRootOperation());

auto aIs = static_cast<bool>(aIsSpatialJoin);
auto bIs = static_cast<bool>(bIsSpatialJoin);
return std::pair<bool, bool>{static_cast<bool>(aIsSpatialJoin),
static_cast<bool>(bIsSpatialJoin)};
}

// _____________________________________________________________________________
auto QueryPlanner::createSpatialJoin(
const SubtreePlan& a, const SubtreePlan& b,
const std::vector<std::array<ColumnIndex, 2>>& jcs)
-> std::optional<SubtreePlan> {
auto [aIs, bIs] = checkSpatialJoin(a, b);

// Exactly one of the inputs must be a SpatialJoin.
if (aIs == bIs) {
return std::nullopt;
}

const SubtreePlan& spatialSubtreePlan = aIsSpatialJoin ? a : b;
const SubtreePlan& otherSubtreePlan = aIsSpatialJoin ? b : a;
const SubtreePlan& spatialSubtreePlan = aIs ? a : b;
const SubtreePlan& otherSubtreePlan = aIs ? b : a;

std::shared_ptr<Operation> op = spatialSubtreePlan._qet->getRootOperation();
auto spatialJoin = static_cast<SpatialJoin*>(op.get());
Expand All @@ -1929,7 +1942,7 @@ auto QueryPlanner::createSpatialJoin(
"Currently, if both sides of a SpatialJoin are variables, then the"
"SpatialJoin must be the only connection between these variables");
}
ColumnIndex ind = aIsSpatialJoin ? jcs[0][1] : jcs[0][0];
ColumnIndex ind = aIs ? jcs[0][1] : jcs[0][0];
const Variable& var =
otherSubtreePlan._qet->getVariableAndInfoByColumnIndex(ind).first;
std::cout << spatialJoin->onlyForTestingGetTask().first << " "
Expand Down
4 changes: 4 additions & 0 deletions src/engine/QueryPlanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ class QueryPlanner {
const SubtreePlan& a, const SubtreePlan& b,
const std::vector<std::array<ColumnIndex, 2>>& jcs);

// Helper to check if subtree plans are spatial join
[[nodiscard]] static std::pair<bool, bool> checkSpatialJoin(
const SubtreePlan& a, const SubtreePlan& b);

// if one of the inputs is a spatial join which is compatible with the other
// input, then add that other input to the spatial join as a child instead of
// creating a normal join.
Expand Down
11 changes: 7 additions & 4 deletions src/engine/SpatialJoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,14 @@ std::optional<size_t> SpatialJoin::getMaxResults() const {

// ____________________________________________________________________________
std::vector<QueryExecutionTree*> SpatialJoin::getChildren() {
if (!(childLeft_ && childRight_)) {
AD_THROW("SpatialJoin needs two children, but at least one is missing");
std::vector<QueryExecutionTree*> result;
if (childLeft_) {
result.push_back(childLeft_.get());
}

return {childLeft_.get(), childRight_.get()};
if (childRight_) {
result.push_back(childRight_.get());
}
return result;
}

// ____________________________________________________________________________
Expand Down

0 comments on commit 39fd03d

Please sign in to comment.