diff --git a/src/engine/SpatialJoin.cpp b/src/engine/SpatialJoin.cpp index d4837435e3..18f5831a88 100644 --- a/src/engine/SpatialJoin.cpp +++ b/src/engine/SpatialJoin.cpp @@ -175,30 +175,24 @@ string SpatialJoin::getDescriptor() const { // ____________________________________________________________________________ size_t SpatialJoin::getResultWidth() const { if (childLeft_ && childRight_) { - // Helper visitor lambda to determine the size of the right child after - // selecting only the join column and explicitly requested payload columns - auto sizeRight = [this](const T& value) { - if constexpr (std::is_same_v) { - return childRight_->getResultWidth(); - } else { - static_assert(std::is_same_v>); - - // We convert to a set here, because we allow multiple occurrences of - // variables in payloadVariables_ - absl::flat_hash_set pvSet{value.begin(), value.end()}; - - // The payloadVariables_ may contain the right join variable - return pvSet.size() + (pvSet.contains(config_.right_) ? 0 : 1); - } - }; - // don't subtract anything because of a common join column. In the case // of the spatial join, the join column is different for both sides (e.g. // objects with a distance of at most 500m to each other, then each object // might contain different positions, which should be kept). // For the right join table we only use the selected columns. - auto widthChildren = childLeft_->getResultWidth() + - std::visit(sizeRight, config_.payloadVariables_); + size_t sizeRight; + if (config_.payloadVariables_.isAll()) { + sizeRight = childRight_->getResultWidth(); + } else { + // We convert to a set here, because we allow multiple occurrences of + // variables in payloadVariables_ + std::vector pv = config_.payloadVariables_.getVariables(); + absl::flat_hash_set pvSet{pv.begin(), pv.end()}; + + // The payloadVariables_ may contain the right join variable + return pvSet.size() + (pvSet.contains(config_.right_) ? 0 : 1); + } + auto widthChildren = childLeft_->getResultWidth() + sizeRight; if (config_.distanceVariable_.has_value()) { return widthChildren + 1; @@ -324,34 +318,29 @@ VariableToColumnMap SpatialJoin::getVarColMapPayloadVars() const { "Child right must be added before payload variable to " "column map can be computed."); - auto payloadVariablesVisitor = [this](const T& value) { - const auto& varColMap = childRight_->getVariableColumns(); - VariableToColumnMap newVarColMap; + const auto& varColMap = childRight_->getVariableColumns(); + VariableToColumnMap newVarColMap; - if constexpr (std::is_same_v) { - newVarColMap = VariableToColumnMap{varColMap}; - } else { - static_assert(std::is_same_v>); - for (const auto& var : value) { - if (varColMap.contains(var)) { - newVarColMap.insert_or_assign(var, varColMap.at(var)); - } else { - addWarningOrThrow( - absl::StrCat("Variable '", var.name(), - "' selected as payload to spatial join but not " - "present in right child.")); - } + if (config_.payloadVariables_.isAll()) { + newVarColMap = VariableToColumnMap{varColMap}; + } else { + for (const auto& var : config_.payloadVariables_.getVariables()) { + if (varColMap.contains(var)) { + newVarColMap.insert_or_assign(var, varColMap.at(var)); + } else { + addWarningOrThrow( + absl::StrCat("Variable '", var.name(), + "' selected as payload to spatial join but not " + "present in right child.")); } - AD_CONTRACT_CHECK( - varColMap.contains(config_.right_), - "Right variable is not defined in right child of spatial join."); - newVarColMap.insert_or_assign(config_.right_, - varColMap.at(config_.right_)); } - return newVarColMap; - }; + AD_CONTRACT_CHECK( + varColMap.contains(config_.right_), + "Right variable is not defined in right child of spatial join."); + newVarColMap.insert_or_assign(config_.right_, varColMap.at(config_.right_)); + } - return std::visit(payloadVariablesVisitor, config_.payloadVariables_); + return newVarColMap; } // ____________________________________________________________________________ diff --git a/src/engine/SpatialJoin.h b/src/engine/SpatialJoin.h index 1e1f3cf719..af76c49a81 100644 --- a/src/engine/SpatialJoin.h +++ b/src/engine/SpatialJoin.h @@ -11,6 +11,7 @@ #include "engine/Operation.h" #include "global/Id.h" +#include "parser/PayloadVariables.h" #include "parser/data/Variable.h" // A nearest neighbor search with optionally a maximum distance. @@ -27,18 +28,6 @@ struct MaxDistanceConfig { // Configuration to restrict the results provided by the SpatialJoin using SpatialJoinTask = std::variant; -// Represents the selection of all variables of the right join table as payload -struct PayloadAllVariables : std::monostate { - bool operator==([[maybe_unused]] const std::vector& other) const { - return false; - }; -}; - -// Configuration to select which columns of the right join table should be -// part of the result. -using PayloadVariables = - std::variant>; - // Selection of a SpatialJoin algorithm enum class SpatialJoinAlgorithm { BASELINE, S2_GEOMETRY, BOUNDING_BOX }; const SpatialJoinAlgorithm SPATIAL_JOIN_DEFAULT_ALGORITHM = @@ -60,7 +49,7 @@ struct SpatialJoinConfiguration { // If given a vector of variables, the selected variables will be part of the // result table - the join column will automatically be part of the result. // You may use PayloadAllVariables to select all columns of the right table. - PayloadVariables payloadVariables_ = PayloadAllVariables{}; + PayloadVariables payloadVariables_ = PayloadVariables::all(); // Choice of algorithm. Both algorithms have equal results, but different // runtime characteristics. diff --git a/src/parser/CMakeLists.txt b/src/parser/CMakeLists.txt index 15facff065..cb6da1e6ed 100644 --- a/src/parser/CMakeLists.txt +++ b/src/parser/CMakeLists.txt @@ -20,6 +20,7 @@ add_library(parser MagicServiceQuery.cpp PathQuery.cpp SpatialQuery.cpp + PayloadVariables.cpp PropertyPath.cpp data/SparqlFilter.cpp SelectClause.cpp diff --git a/src/parser/PayloadVariables.cpp b/src/parser/PayloadVariables.cpp new file mode 100644 index 0000000000..a785e23c21 --- /dev/null +++ b/src/parser/PayloadVariables.cpp @@ -0,0 +1,72 @@ +// Copyright 2024, University of Freiburg, +// Chair of Algorithms and Data Structures. +// Author: Christoph Ullinger + +#include "parser/PayloadVariables.h" + +// ____________________________________________________________________________ +PayloadVariables::PayloadVariables(std::vector variables) + : variables_{std::move(variables)} {}; + +// ____________________________________________________________________________ +PayloadVariables PayloadVariables::all() { + PayloadVariables pv{}; + pv.setToAll(); + return pv; +}; + +// ____________________________________________________________________________ +void PayloadVariables::addVariable(const Variable& variable) { + // Helper visitor to check if the payload variables has not been set to all + // and a variable can be added. If yes, add it. + auto addVarVisitor = [&](T& value) { + if constexpr (std::is_same_v>) { + value.push_back(variable); + } + }; + + std::visit(addVarVisitor, variables_); +}; + +// ____________________________________________________________________________ +void PayloadVariables::setToAll() { + variables_ = detail::PayloadAllVariables{}; +}; + +// ____________________________________________________________________________ +bool PayloadVariables::empty() const { + // Helper visitor to check if the payload variables are empty + auto emptyVisitor = [](const T& value) -> bool { + if constexpr (std::is_same_v) { + return false; + } else { + static_assert(std::is_same_v>); + return value.empty(); + } + }; + + return std::visit(emptyVisitor, variables_); +}; + +// ____________________________________________________________________________ +bool PayloadVariables::isAll() const { + return std::holds_alternative(variables_); +}; + +// ____________________________________________________________________________ +const std::vector& PayloadVariables::getVariables() const { + // Helper visitor to check if the payload variables has been set to all: then + // throw, otherwise return the vector + auto getVarVisitor = + [](const T& value) -> const std::vector& { + if constexpr (std::is_same_v>) { + return value; + } else { + AD_THROW( + "getVariables may only be called on a non-all PayloadVariables " + "object."); + } + }; + + return std::visit(getVarVisitor, variables_); +}; diff --git a/src/parser/PayloadVariables.h b/src/parser/PayloadVariables.h new file mode 100644 index 0000000000..c69214be6b --- /dev/null +++ b/src/parser/PayloadVariables.h @@ -0,0 +1,57 @@ +// Copyright 2024, University of Freiburg, +// Chair of Algorithms and Data Structures. +// Author: Christoph Ullinger + +#pragma once + +#include + +#include "parser/data/Variable.h" + +namespace detail { +// Represents the selection of all variables as payload +struct PayloadAllVariables : std::monostate { + bool operator==([[maybe_unused]] const std::vector& other) const { + return false; + }; +}; +} // namespace detail + +// This class represents a list of variables to be included in the result of +// an operation. This is currently used in the spatial search. +class PayloadVariables { + public: + // Construct an empty payload variables object + PayloadVariables() = default; + + // Construct a payload variables object from a vector of variables + PayloadVariables(std::vector variables); + + // Construct a payload variables object that is set to all + static PayloadVariables all(); + + // Add a variable to the payload variables or do nothing if all variables are + // already selected + void addVariable(const Variable& variable); + + // Select all variables. + void setToAll(); + + // Returns whether the payload variables object is empty. + bool empty() const; + + // Returns whether all variables have been selected. + bool isAll() const; + + // Returns a vector of variables if all has not been set. Otherwise throws. + const std::vector& getVariables() const; + + // For testing: equality operator + bool operator==(const PayloadVariables& other) const { + return variables_ == other.variables_; + }; + + private: + std::variant> variables_ = + std::vector{}; +}; diff --git a/src/parser/SpatialQuery.cpp b/src/parser/SpatialQuery.cpp index 2fefe31f86..5112db3a20 100644 --- a/src/parser/SpatialQuery.cpp +++ b/src/parser/SpatialQuery.cpp @@ -4,8 +4,13 @@ #include "parser/SpatialQuery.h" +#include +#include + #include "engine/SpatialJoin.h" #include "parser/MagicServiceIriConstants.h" +#include "parser/PayloadVariables.h" +#include "parser/data/Variable.h" namespace parsedQuery { @@ -60,7 +65,22 @@ void SpatialQuery::addParameter(const SparqlTriple& triple) { ", or ."); } } 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. + payloadVariables_.addVariable(getVariable("payload", object)); + } else if (object.isIri() && + extractParameterName(object, SPATIAL_SEARCH_IRI) == "all") { + // All variables selected + payloadVariables_.setToAll(); + } else { + throw SpatialSearchException( + "The argument to the parameter must be either a variable " + "to be selected or ."); + } + } else { throw SpatialSearchException(absl::StrCat( "Unsupported argument ", predString, @@ -95,7 +115,7 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const { "spatialSearch: { [Config Triples] { ?right " "} }."); } else if (!ignoreMissingRightChild_ && !childGraphPattern_.has_value() && - !payloadVariables_.empty()) { + !payloadVariables_.isAll() && !payloadVariables_.empty()) { throw SpatialSearchException( "The right variable for the spatial search is declared outside the " "SERVICE, but the parameter was set. Please move the " @@ -112,7 +132,7 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const { // Payload variables PayloadVariables pv; if (!childGraphPattern_.has_value()) { - pv = PayloadAllVariables{}; + pv = PayloadVariables::all(); } else { pv = payloadVariables_; } diff --git a/src/parser/SpatialQuery.h b/src/parser/SpatialQuery.h index b78c296f30..f472a53806 100644 --- a/src/parser/SpatialQuery.h +++ b/src/parser/SpatialQuery.h @@ -6,6 +6,7 @@ #include "parser/GraphPattern.h" #include "parser/MagicServiceQuery.h" +#include "parser/PayloadVariables.h" namespace parsedQuery { @@ -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 payloadVariables_; + PayloadVariables payloadVariables_; // Optional further argument: the join algorithm. If it is not given, the // default algorithm is used implicitly. diff --git a/test/QueryPlannerTest.cpp b/test/QueryPlannerTest.cpp index 926f9a40e0..c9a104bdcb 100644 --- a/test/QueryPlannerTest.cpp +++ b/test/QueryPlannerTest.cpp @@ -5,11 +5,13 @@ #include +#include "./printers/PayloadVariablePrinters.h" #include "QueryPlannerTestHelpers.h" #include "engine/QueryPlanner.h" #include "engine/SpatialJoin.h" #include "parser/GraphPatternOperation.h" #include "parser/MagicServiceQuery.h" +#include "parser/PayloadVariables.h" #include "parser/SparqlParser.h" #include "parser/SpatialQuery.h" #include "parser/data/Variable.h" @@ -1632,7 +1634,7 @@ TEST(QueryPlanner, SpatialJoinService) { auto S2 = SpatialJoinAlgorithm::S2_GEOMETRY; auto Basel = SpatialJoinAlgorithm::BASELINE; auto BBox = SpatialJoinAlgorithm::BOUNDING_BOX; - std::vector emptyPayload{}; + PayloadVariables emptyPayload{}; // Simple base cases h::expect( @@ -1717,6 +1719,7 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { auto scan = h::IndexScanFromStrings; using V = Variable; auto S2 = SpatialJoinAlgorithm::S2_GEOMETRY; + using PV = PayloadVariables; h::expect( "PREFIX spatialSearch: " @@ -1731,7 +1734,7 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { "_:config spatialSearch:payload ?a ." "{ ?a

?b } }}", h::SpatialJoin(-1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, - std::vector{V{"?a"}}, S2, scan("?x", "

", "?y"), + PV{std::vector{V{"?a"}}}, S2, scan("?x", "

", "?y"), scan("?a", "

", "?b"))); h::expect( "PREFIX spatialSearch: " @@ -1747,7 +1750,7 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { "{ ?a

?a2 . ?a2

?b } }}", h::SpatialJoin( -1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, - std::vector{V{"?a"}, V{"?a2"}}, S2, scan("?x", "

", "?y"), + PV{std::vector{V{"?a"}, V{"?a2"}}}, S2, scan("?x", "

", "?y"), h::Join(scan("?a", "

", "?a2"), scan("?a2", "

", "?b")))); // Right variable and duplicates are possible (silently deduplicated during @@ -1766,19 +1769,71 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { "{ ?a

?a2 . ?a2

?b } }}", h::SpatialJoin( -1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, - std::vector{V{"?a"}, V{"?a"}, V{"?b"}, V{"?a2"}}, S2, + PV{std::vector{V{"?a"}, V{"?a"}, V{"?b"}, V{"?a2"}}}, S2, + scan("?x", "

", "?y"), + h::Join(scan("?a", "

", "?a2"), scan("?a2", "

", "?b")))); + + // Selecting all payload variables using "all" + h::expect( + "PREFIX spatialSearch: " + "SELECT * WHERE {" + "?x

?y." + "SERVICE spatialSearch: {" + "_:config spatialSearch:algorithm spatialSearch:s2 ;" + "spatialSearch:right ?b ;" + "spatialSearch:bindDistance ?dist ;" + "spatialSearch:numNearestNeighbors 5 . " + "_:config spatialSearch:left ?y ." + "_:config spatialSearch:payload ." + "{ ?a

?a2 . ?a2

?b } }}", + h::SpatialJoin( + -1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, PayloadVariables::all(), S2, + scan("?x", "

", "?y"), + h::Join(scan("?a", "

", "?a2"), scan("?a2", "

", "?b")))); + h::expect( + "PREFIX spatialSearch: " + "SELECT * WHERE {" + "?x

?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

?a2 . ?a2

?b } }}", + h::SpatialJoin( + -1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, PayloadVariables::all(), S2, + scan("?x", "

", "?y"), + h::Join(scan("?a", "

", "?a2"), scan("?a2", "

", "?b")))); + + // All and explicitly named ones just select all + h::expect( + "PREFIX spatialSearch: " + "SELECT * WHERE {" + "?x

?y." + "SERVICE spatialSearch: {" + "_:config spatialSearch:algorithm spatialSearch:s2 ;" + "spatialSearch:right ?b ;" + "spatialSearch:bindDistance ?dist ;" + "spatialSearch:numNearestNeighbors 5 . " + "_:config spatialSearch:left ?y ." + "_:config spatialSearch:payload ." + "_:config spatialSearch:payload ?a ." + "{ ?a

?a2 . ?a2

?b } }}", + h::SpatialJoin( + -1, 5, V{"?y"}, V{"?b"}, V{"?dist"}, PayloadVariables::all(), S2, scan("?x", "

", "?y"), h::Join(scan("?a", "

", "?a2"), scan("?a2", "

", "?b")))); } TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) { - // If only maxDistance is used but not numNearestNeighbors, the right variable - // must not come from inside the SERVICE - auto scan = h::IndexScanFromStrings; using V = Variable; auto S2 = SpatialJoinAlgorithm::S2_GEOMETRY; + // If only maxDistance is used but not numNearestNeighbors, the right variable + // must not come from inside the SERVICE h::expect( "PREFIX spatialSearch: " "SELECT * WHERE {" @@ -1791,8 +1846,31 @@ TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) { "spatialSearch:maxDistance 1 . " " } }", h::SpatialJoin(1, -1, V{"?y"}, V{"?b"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), + // Payload variables have the default all instead of empty + // in this case + PayloadVariables::all(), S2, scan("?x", "

", "?y"), + scan("?a", "

", "?b"))); + + // If the user explicitly states that they want all payload variables (which + // is enforced and the default anyway), this should also work + h::expect( + "PREFIX spatialSearch: " + "SELECT * WHERE {" + "?a

?b ." + "?x

?y ." + "SERVICE spatialSearch: {" + "_:config spatialSearch:algorithm spatialSearch:s2 ;" + "spatialSearch:left ?y ;" + "spatialSearch:right ?b ;" + "spatialSearch:maxDistance 1 ; " + "spatialSearch:payload spatialSearch:all ." + " } }", + h::SpatialJoin(1, -1, V{"?y"}, V{"?b"}, std::nullopt, + PayloadVariables::all(), S2, scan("?x", "

", "?y"), scan("?a", "

", "?b"))); + + // Nearest neighbors search requires the right child to be defined inside the + // service AD_EXPECT_THROW_WITH_MESSAGE( h::expect("PREFIX spatialSearch: " "" @@ -1810,6 +1888,11 @@ TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) { ::testing::ContainsRegex( "must have its right " "variable declared inside the service using a graph pattern")); + + // The user may not select specific payload variables if the right join table + // is declared outside because this would mess up the query semantics and may + // not have deterministic results on different inputs because of query planner + // decisions AD_EXPECT_THROW_WITH_MESSAGE( h::expect("PREFIX spatialSearch: " "" @@ -1835,6 +1918,7 @@ TEST(QueryPlanner, SpatialJoinMultipleServiceSharedLeft) { auto scan = h::IndexScanFromStrings; using V = Variable; auto S2 = SpatialJoinAlgorithm::S2_GEOMETRY; + using PV = PayloadVariables; h::expect( "SELECT * WHERE {" @@ -1848,20 +1932,16 @@ TEST(QueryPlanner, SpatialJoinMultipleServiceSharedLeft) { // children one way or the other depending on cost estimates. Both // versions are semantically correct. ::testing::AnyOf( - h::SpatialJoin( - 100, -1, V{"?y"}, V{"?b"}, std::nullopt, PayloadAllVariables{}, - S2, - h::SpatialJoin(500, -1, V{"?y"}, V{"?c"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), - scan("?ac", "", "?c")), - scan("?ab", "", "?b")), - h::SpatialJoin( - 500, -1, V{"?y"}, V{"?c"}, std::nullopt, PayloadAllVariables{}, - S2, - h::SpatialJoin(100, -1, V{"?y"}, V{"?b"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), - scan("?ab", "", "?b")), - scan("?ac", "", "?c")))); + h::SpatialJoin(100, -1, V{"?y"}, V{"?b"}, std::nullopt, PV::all(), S2, + h::SpatialJoin(500, -1, V{"?y"}, V{"?c"}, std::nullopt, + PV::all(), S2, scan("?x", "

", "?y"), + scan("?ac", "", "?c")), + scan("?ab", "", "?b")), + h::SpatialJoin(500, -1, V{"?y"}, V{"?c"}, std::nullopt, PV::all(), S2, + h::SpatialJoin(100, -1, V{"?y"}, V{"?b"}, std::nullopt, + PV::all(), S2, scan("?x", "

", "?y"), + scan("?ab", "", "?b")), + scan("?ac", "", "?c")))); h::expect( "PREFIX spatialSearch: " "SELECT * WHERE {" @@ -1889,18 +1969,18 @@ TEST(QueryPlanner, SpatialJoinMultipleServiceSharedLeft) { // children one way or the other depending on cost estimates. Both // versions are semantically correct. ::testing::AnyOf( - h::SpatialJoin( - 500, 5, V{"?y"}, V{"?c"}, V{"?dc"}, std::vector{V{"?ac"}}, S2, - h::SpatialJoin(-1, 5, V{"?y"}, V{"?b"}, V{"?db"}, - std::vector{}, S2, scan("?x", "

", "?y"), - scan("?ab", "", "?b")), - scan("?ac", "", "?c")), - h::SpatialJoin( - -1, 5, V{"?y"}, V{"?b"}, V{"?db"}, std::vector{}, S2, - h::SpatialJoin( - 500, 5, V{"?y"}, V{"?c"}, V{"?dc"}, std::vector{V{"?ac"}}, - S2, scan("?x", "

", "?y"), scan("?ac", "", "?c")), - scan("?ab", "", "?b")))); + h::SpatialJoin(500, 5, V{"?y"}, V{"?c"}, V{"?dc"}, + PV{std::vector{V{"?ac"}}}, S2, + h::SpatialJoin(-1, 5, V{"?y"}, V{"?b"}, V{"?db"}, PV{}, + S2, scan("?x", "

", "?y"), + scan("?ab", "", "?b")), + scan("?ac", "", "?c")), + h::SpatialJoin(-1, 5, V{"?y"}, V{"?b"}, V{"?db"}, PV{}, S2, + h::SpatialJoin(500, 5, V{"?y"}, V{"?c"}, V{"?dc"}, + PV{std::vector{V{"?ac"}}}, S2, + scan("?x", "

", "?y"), + scan("?ac", "", "?c")), + scan("?ab", "", "?b")))); } TEST(QueryPlanner, SpatialJoinMissingConfig) { @@ -2105,7 +2185,23 @@ TEST(QueryPlanner, SpatialJoinIncorrectConfigValues) { " { ?a

?b . }" "}}", ::testing::_), - ::testing::ContainsRegex(" has to be a variable")); + ::testing::ContainsRegex(" parameter must be either a variable " + "to be selected or ")); + AD_EXPECT_THROW_WITH_MESSAGE( + h::expect("PREFIX spatialSearch: " + "" + "SELECT * WHERE {" + "?x

?y ." + "SERVICE spatialSearch: {" + "_:config spatialSearch:right ?b ;" + "spatialSearch:left ?y ;" + "spatialSearch:maxDistance 5 ;" + "spatialSearch:payload ." + " { ?a

?b . }" + "}}", + ::testing::_), + ::testing::ContainsRegex(" parameter must be either a variable " + "to be selected or ")); AD_EXPECT_THROW_WITH_MESSAGE( h::expect("PREFIX spatialSearch: " "" @@ -2162,7 +2258,7 @@ TEST(QueryPlanner, SpatialJoinLegacyPredicateSupport) { "?x

?y ." " }", h::SpatialJoin(1, -1, V{"?y"}, V{"?b"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), + PayloadVariables::all(), S2, scan("?x", "

", "?y"), scan("?a", "

", "?b"))); h::expect( "SELECT * WHERE {" @@ -2171,7 +2267,7 @@ TEST(QueryPlanner, SpatialJoinLegacyPredicateSupport) { "?x

?y ." " }", h::SpatialJoin(5000, -1, V{"?y"}, V{"?b"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), + PayloadVariables::all(), S2, scan("?x", "

", "?y"), scan("?a", "

", "?b"))); // Test that invalid triples throw an error @@ -2224,7 +2320,7 @@ TEST(QueryPlanner, SpatialJoinLegacyPredicateSupport) { h::QetWithWarnings( {"special predicate is deprecated"}, h::SpatialJoin(500, 2, V{"?y"}, V{"?b"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), + PayloadVariables::all(), S2, scan("?x", "

", "?y"), scan("?a", "

", "?b")))); h::expect( "SELECT ?x ?y WHERE {" @@ -2234,7 +2330,7 @@ TEST(QueryPlanner, SpatialJoinLegacyPredicateSupport) { h::QetWithWarnings( {"special predicate is deprecated"}, h::SpatialJoin(-1, 20, V{"?y"}, V{"?b"}, std::nullopt, - PayloadAllVariables{}, S2, scan("?x", "

", "?y"), + PayloadVariables::all(), S2, scan("?x", "

", "?y"), scan("?a", "

", "?b")))); AD_EXPECT_THROW_WITH_MESSAGE(h::expect("SELECT ?x ?y WHERE {" diff --git a/test/engine/SpatialJoinTest.cpp b/test/engine/SpatialJoinTest.cpp index 455ecf8b16..ed49d54e86 100644 --- a/test/engine/SpatialJoinTest.cpp +++ b/test/engine/SpatialJoinTest.cpp @@ -14,6 +14,7 @@ #include #include +#include "../printers/PayloadVariablePrinters.h" #include "../printers/VariablePrinters.h" #include "../printers/VariableToColumnMapPrinters.h" #include "../util/GTestHelpers.h" @@ -261,7 +262,7 @@ class SpatialJoinVarColParamTest std::shared_ptr makeSpatialJoin( QueryExecutionContext* qec, VarColTestSuiteParam parameters, - bool addDist = true, PayloadVariables pv = PayloadAllVariables{}) { + bool addDist = true, PayloadVariables pv = PayloadVariables::all()) { auto [leftSideBigChild, rightSideBigChild, addLeftChildFirst, testVarToColMap] = parameters; auto leftChild = getChild(qec, leftSideBigChild, "1"); @@ -456,7 +457,7 @@ class SpatialJoinVarColParamTest // Also test the PayloadAllVariables version if (withGeo && withObj && (withName || !rightSideBigChild)) { - computeAndCompareVarToColMaps(addDist, PayloadAllVariables{}, + computeAndCompareVarToColMaps(addDist, PayloadVariables::all(), exp); } diff --git a/test/parser/CMakeLists.txt b/test/parser/CMakeLists.txt index ac21f4b453..a694ed3f37 100644 --- a/test/parser/CMakeLists.txt +++ b/test/parser/CMakeLists.txt @@ -1,3 +1,4 @@ add_subdirectory(data) addLinkAndDiscoverTest(ParallelBufferTest parser) addLinkAndDiscoverTest(LiteralOrIriTest engine) +addLinkAndDiscoverTest(PayloadVariablesTest engine) diff --git a/test/parser/PayloadVariablesTest.cpp b/test/parser/PayloadVariablesTest.cpp new file mode 100644 index 0000000000..9a0f2c6c31 --- /dev/null +++ b/test/parser/PayloadVariablesTest.cpp @@ -0,0 +1,65 @@ +// Copyright 2024, University of Freiburg, +// Chair of Algorithms and Data Structures. +// Author: Christoph Ullinger + +#include + +#include "../printers/PayloadVariablePrinters.h" +#include "../util/GTestHelpers.h" +#include "gmock/gmock.h" +#include "parser/PayloadVariables.h" +#include "parser/data/Variable.h" + +namespace { // anonymous namespace to avoid linker problems + +TEST(PayloadVariablesTest, PayloadVariables) { + PayloadVariables pv1; + ASSERT_TRUE(pv1.empty()); + ASSERT_FALSE(pv1.isAll()); + ASSERT_EQ(pv1.getVariables(), std::vector{}); + + PayloadVariables pv2{std::vector{}}; + ASSERT_EQ(pv1, pv2); + ASSERT_TRUE(pv2.empty()); + ASSERT_FALSE(pv2.isAll()); + + pv2.setToAll(); + ASSERT_TRUE(pv2.isAll()); + ASSERT_FALSE(pv2.empty()); + ASSERT_NE(pv1, pv2); + + pv2.addVariable(Variable{"?a"}); + ASSERT_TRUE(pv2.isAll()); + ASSERT_FALSE(pv2.empty()); + AD_EXPECT_THROW_WITH_MESSAGE( + pv2.getVariables(), + ::testing::HasSubstr( + "getVariables may only be called on a non-all PayloadVariables " + "object")); + + auto pv3 = PayloadVariables::all(); + ASSERT_EQ(pv2, pv3); + ASSERT_TRUE(pv3.isAll()); + ASSERT_FALSE(pv3.empty()); + + PayloadVariables pv4{std::vector{Variable{"?a"}, Variable{"?b"}}}; + ASSERT_FALSE(pv4.isAll()); + ASSERT_FALSE(pv4.empty()); + ASSERT_NE(pv3, pv4); + ASSERT_NE(pv1, pv4); + std::vector expect4{Variable{"?a"}, Variable{"?b"}}; + ASSERT_EQ(pv4.getVariables(), expect4); + + PayloadVariables pv5; + ASSERT_EQ(pv5.getVariables(), std::vector{}); + pv5.addVariable(Variable{"?var"}); + std::vector expect5_step1{Variable{"?var"}}; + ASSERT_EQ(pv5.getVariables(), expect5_step1); + pv5.addVariable(Variable{"?var2"}); + std::vector expect5_step2{Variable{"?var"}, Variable{"?var2"}}; + ASSERT_EQ(pv5.getVariables(), expect5_step2); + ASSERT_FALSE(pv5.isAll()); + ASSERT_FALSE(pv5.empty()); +}; + +} // namespace diff --git a/test/printers/PayloadVariablePrinters.h b/test/printers/PayloadVariablePrinters.h new file mode 100644 index 0000000000..efcb9ffb26 --- /dev/null +++ b/test/printers/PayloadVariablePrinters.h @@ -0,0 +1,20 @@ +// Copyright 2024, University of Freiburg, +// Chair of Algorithms and Data Structures. +// Author: Christoph Ullinger + +#pragma once +#include "./VariablePrinters.h" +#include "parser/PayloadVariables.h" + +// _____________________________________________________________ +inline void PrintTo(const PayloadVariables& pv, std::ostream* os) { + auto& s = *os; + if (pv.isAll()) { + s << std::string{"ALL"}; + } else { + for (const auto& v : pv.getVariables()) { + PrintTo(v, os); + s << std::string{" "}; + } + } +}