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

Allow selecting all variables as payload in spatial search SERVICE #1656

Merged
merged 8 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
36 changes: 34 additions & 2 deletions src/parser/SpatialQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@

#include "parser/SpatialQuery.h"

#include <type_traits>
#include <variant>

#include "engine/SpatialJoin.h"
#include "parser/MagicServiceIriConstants.h"
#include "parser/data/Variable.h"

namespace parsedQuery {

Expand Down Expand Up @@ -60,7 +64,25 @@ void SpatialQuery::addParameter(const SparqlTriple& triple) {
"<baseline>, <s2> or <boundingBox>.");
}
} else if (predString == "payload") {
payloadVariables_.push_back(getVariable("payload", object));
if (object.isVariable()) {
// Single selected variable

// If we have already selected all payload variables, we can ignore
// another explicitly selected variable.
if (std::holds_alternative<std::vector<Variable>>(payloadVariables_)) {
std::get<std::vector<Variable>>(payloadVariables_)
.push_back(getVariable("payload", object));
}
} else if (object.isIri() &&
extractParameterName(object, SPATIAL_SEARCH_IRI) == "all") {
// All variables selected
payloadVariables_ = PayloadAllVariables{};
} else {
throw SpatialSearchException(
"The argument to the <payload> parameter must be either a variable "
"to be selected or <all>.");
}

ullingerc marked this conversation as resolved.
Show resolved Hide resolved
} else {
throw SpatialSearchException(absl::StrCat(
"Unsupported argument ", predString,
Expand All @@ -84,6 +106,16 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const {
"Missing parameter <right> in spatial search.");
}

// Helper visitor to check if the payload variables are empty
auto emptyPayload = []<typename T>(const T& value) -> bool {
if constexpr (std::is_same_v<T, PayloadAllVariables>) {
return false;
} else {
static_assert(std::is_same_v<T, std::vector<Variable>>);
return value.empty();
}
};

// Only if the number of results is limited, it is mandatory that the right
// variable must be selected inside the service. If only the distance is
// limited, it may be declared inside or outside of the service.
Expand All @@ -95,7 +127,7 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const {
"spatialSearch: { [Config Triples] { <Something> <ThatSelects> ?right "
"} }.");
} else if (!ignoreMissingRightChild_ && !childGraphPattern_.has_value() &&
!payloadVariables_.empty()) {
!std::visit(emptyPayload, payloadVariables_)) {
throw SpatialSearchException(
"The right variable for the spatial search is declared outside the "
"SERVICE, but the <payload> parameter was set. Please move the "
Expand Down
3 changes: 2 additions & 1 deletion src/parser/SpatialQuery.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#pragma once

#include "engine/SpatialJoin.h"
ullingerc marked this conversation as resolved.
Show resolved Hide resolved
#include "parser/GraphPattern.h"
#include "parser/MagicServiceQuery.h"

Expand Down Expand Up @@ -39,7 +40,7 @@ struct SpatialQuery : MagicServiceQuery {
// this vector is required to be empty - the user may not specify the payload
// configuration parameter. It will then be automatically set to
// `PayloadAllVariables` to ensure appropriate semantics.
std::vector<Variable> payloadVariables_;
PayloadVariables payloadVariables_ = std::vector<Variable>{};

// Optional further argument: the join algorithm. If it is not given, the
// default algorithm is used implicitly.
Expand Down
71 changes: 70 additions & 1 deletion test/QueryPlannerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,59 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) {
std::vector<V>{V{"?a"}, V{"?a"}, V{"?b"}, V{"?a2"}}, S2,
scan("?x", "<p>", "?y"),
h::Join(scan("?a", "<p>", "?a2"), scan("?a2", "<p>", "?b"))));

// Selecting all payload variables using "all"
h::expect(
"PREFIX spatialSearch: <https://qlever.cs.uni-freiburg.de/spatialSearch/>"
"SELECT * WHERE {"
"?x <p> ?y."
"SERVICE spatialSearch: {"
"_:config spatialSearch:algorithm spatialSearch:s2 ;"
"spatialSearch:right ?b ;"
"spatialSearch:bindDistance ?dist ;"
"spatialSearch:numNearestNeighbors 5 . "
"_:config spatialSearch:left ?y ."
"_:config spatialSearch:payload <all> ."
"{ ?a <p> ?a2 . ?a2 <p> ?b } }}",
h::SpatialJoin(
-1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, PayloadAllVariables{}, S2,
scan("?x", "<p>", "?y"),
h::Join(scan("?a", "<p>", "?a2"), scan("?a2", "<p>", "?b"))));
h::expect(
"PREFIX spatialSearch: <https://qlever.cs.uni-freiburg.de/spatialSearch/>"
"SELECT * WHERE {"
"?x <p> ?y."
"SERVICE spatialSearch: {"
"_:config spatialSearch:algorithm spatialSearch:s2 ;"
"spatialSearch:right ?b ;"
"spatialSearch:bindDistance ?dist ;"
"spatialSearch:numNearestNeighbors 5 . "
"_:config spatialSearch:left ?y ."
"_:config spatialSearch:payload spatialSearch:all ."
"{ ?a <p> ?a2 . ?a2 <p> ?b } }}",
h::SpatialJoin(
-1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, PayloadAllVariables{}, S2,
scan("?x", "<p>", "?y"),
h::Join(scan("?a", "<p>", "?a2"), scan("?a2", "<p>", "?b"))));

// All and explicitly named ones just select all
h::expect(
"PREFIX spatialSearch: <https://qlever.cs.uni-freiburg.de/spatialSearch/>"
"SELECT * WHERE {"
"?x <p> ?y."
"SERVICE spatialSearch: {"
"_:config spatialSearch:algorithm spatialSearch:s2 ;"
"spatialSearch:right ?b ;"
"spatialSearch:bindDistance ?dist ;"
"spatialSearch:numNearestNeighbors 5 . "
"_:config spatialSearch:left ?y ."
"_:config spatialSearch:payload <all> ."
"_:config spatialSearch:payload ?a ."
"{ ?a <p> ?a2 . ?a2 <p> ?b } }}",
h::SpatialJoin(
-1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, PayloadAllVariables{}, S2,
scan("?x", "<p>", "?y"),
h::Join(scan("?a", "<p>", "?a2"), scan("?a2", "<p>", "?b"))));
}

TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) {
Expand Down Expand Up @@ -2105,7 +2158,23 @@ TEST(QueryPlanner, SpatialJoinIncorrectConfigValues) {
" { ?a <p> ?b . }"
"}}",
::testing::_),
::testing::ContainsRegex("<payload> has to be a variable"));
::testing::ContainsRegex("<payload> parameter must be either a variable "
"to be selected or <all>"));
AD_EXPECT_THROW_WITH_MESSAGE(
h::expect("PREFIX spatialSearch: "
"<https://qlever.cs.uni-freiburg.de/spatialSearch/>"
"SELECT * WHERE {"
"?x <p> ?y ."
"SERVICE spatialSearch: {"
"_:config spatialSearch:right ?b ;"
"spatialSearch:left ?y ;"
"spatialSearch:maxDistance 5 ;"
"spatialSearch:payload <http://some.iri.that.is.not.all> ."
" { ?a <p> ?b . }"
"}}",
::testing::_),
::testing::ContainsRegex("<payload> parameter must be either a variable "
"to be selected or <all>"));
AD_EXPECT_THROW_WITH_MESSAGE(
h::expect("PREFIX spatialSearch: "
"<https://qlever.cs.uni-freiburg.de/spatialSearch/>"
Expand Down
Loading