From a0b39d2e6d7c702e5b57adc9d4d90d65b08bad6c Mon Sep 17 00:00:00 2001 From: ullingerc Date: Wed, 4 Dec 2024 10:53:51 +0100 Subject: [PATCH 1/8] Allow selecting all variables as payload in spatial search SERVICE --- src/parser/SpatialQuery.cpp | 36 ++++++++++++++++++++++-- src/parser/SpatialQuery.h | 3 +- test/QueryPlannerTest.cpp | 56 ++++++++++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/parser/SpatialQuery.cpp b/src/parser/SpatialQuery.cpp index 2fefe31f86..cffebc96fa 100644 --- a/src/parser/SpatialQuery.cpp +++ b/src/parser/SpatialQuery.cpp @@ -4,8 +4,12 @@ #include "parser/SpatialQuery.h" +#include +#include + #include "engine/SpatialJoin.h" #include "parser/MagicServiceIriConstants.h" +#include "parser/data/Variable.h" namespace parsedQuery { @@ -60,7 +64,25 @@ 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. + if (std::holds_alternative>(payloadVariables_)) { + std::get>(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 parameter must be either a variable " + "to be selected or ."); + } + } else { throw SpatialSearchException(absl::StrCat( "Unsupported argument ", predString, @@ -84,6 +106,16 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const { "Missing parameter in spatial search."); } + // Helper visitor to check if the payload variables are empty + auto emptyPayload = [](const T& value) -> bool { + if constexpr (std::is_same_v) { + return false; + } else { + static_assert(std::is_same_v>); + 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. @@ -95,7 +127,7 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const { "spatialSearch: { [Config Triples] { ?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 parameter was set. Please move the " diff --git a/src/parser/SpatialQuery.h b/src/parser/SpatialQuery.h index b78c296f30..145cd2dc6b 100644 --- a/src/parser/SpatialQuery.h +++ b/src/parser/SpatialQuery.h @@ -4,6 +4,7 @@ #pragma once +#include "engine/SpatialJoin.h" #include "parser/GraphPattern.h" #include "parser/MagicServiceQuery.h" @@ -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_ = std::vector{}; // 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..947e5d6ff6 100644 --- a/test/QueryPlannerTest.cpp +++ b/test/QueryPlannerTest.cpp @@ -1769,6 +1769,59 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { 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"}, PayloadAllVariables{}, 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"}, PayloadAllVariables{}, 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"}, PayloadAllVariables{}, S2, + scan("?x", "

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

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

", "?b")))); } TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) { @@ -2105,7 +2158,8 @@ 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: " "" From 000fb9c66e8c4bbec92549f41595e6d3ba5af265 Mon Sep 17 00:00:00 2001 From: ullingerc Date: Wed, 4 Dec 2024 18:51:45 +0100 Subject: [PATCH 2/8] add one more test to get coverage to 100% --- test/QueryPlannerTest.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/QueryPlannerTest.cpp b/test/QueryPlannerTest.cpp index 947e5d6ff6..608d28da0e 100644 --- a/test/QueryPlannerTest.cpp +++ b/test/QueryPlannerTest.cpp @@ -2160,6 +2160,21 @@ TEST(QueryPlanner, SpatialJoinIncorrectConfigValues) { ::testing::_), ::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: " "" From fcf4cf5898d609de081ac5b76b3300499aeda1f5 Mon Sep 17 00:00:00 2001 From: ullingerc Date: Thu, 5 Dec 2024 10:44:35 +0100 Subject: [PATCH 3/8] Separate PayloadVariables into own class and module, add tests and test printer --- src/engine/CMakeLists.txt | 2 +- src/engine/PayloadVariables.cpp | 81 +++++++++++++++++++++++++ src/engine/PayloadVariables.h | 57 +++++++++++++++++ src/engine/SpatialJoin.cpp | 75 ++++++++++------------- src/engine/SpatialJoin.h | 15 +---- src/parser/SpatialQuery.cpp | 22 ++----- src/parser/SpatialQuery.h | 2 +- test/QueryPlannerTest.cpp | 76 +++++++++++------------ test/engine/CMakeLists.txt | 1 + test/engine/PayloadVariablesTest.cpp | 65 ++++++++++++++++++++ test/engine/SpatialJoinTest.cpp | 5 +- test/printers/PayloadVariablePrinters.h | 20 ++++++ 12 files changed, 306 insertions(+), 115 deletions(-) create mode 100644 src/engine/PayloadVariables.cpp create mode 100644 src/engine/PayloadVariables.h create mode 100644 test/engine/PayloadVariablesTest.cpp create mode 100644 test/printers/PayloadVariablePrinters.h diff --git a/src/engine/CMakeLists.txt b/src/engine/CMakeLists.txt index cbfb3344c3..232f3e1d2f 100644 --- a/src/engine/CMakeLists.txt +++ b/src/engine/CMakeLists.txt @@ -10,7 +10,7 @@ add_library(engine Union.cpp MultiColumnJoin.cpp TransitivePathBase.cpp TransitivePathHashMap.cpp TransitivePathBinSearch.cpp Service.cpp Values.cpp Bind.cpp Minus.cpp RuntimeInformation.cpp CheckUsePatternTrick.cpp - VariableToColumnMap.cpp ExportQueryExecutionTrees.cpp + VariableToColumnMap.cpp ExportQueryExecutionTrees.cpp PayloadVariables.cpp CartesianProductJoin.cpp TextIndexScanForWord.cpp TextIndexScanForEntity.cpp TextLimit.cpp LazyGroupBy.cpp GroupByHashMapOptimization.cpp SpatialJoin.cpp CountConnectedSubgraphs.cpp SpatialJoinAlgorithms.cpp PathSearch.cpp ExecuteUpdate.cpp) diff --git a/src/engine/PayloadVariables.cpp b/src/engine/PayloadVariables.cpp new file mode 100644 index 0000000000..0f9165e24a --- /dev/null +++ b/src/engine/PayloadVariables.cpp @@ -0,0 +1,81 @@ +// Copyright 2024, University of Freiburg, +// Chair of Algorithms and Data Structures. +// Author: Christoph Ullinger + +#include "engine/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 { + // Helper visitor to check if the payload variables has been set to all + auto allVisitor = [](const T&) -> bool { + if constexpr (std::is_same_v) { + return true; + } else { + static_assert(std::is_same_v>); + return false; + } + }; + + return std::visit(allVisitor, variables_); +}; + +// ____________________________________________________________________________ +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) -> 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/engine/PayloadVariables.h b/src/engine/PayloadVariables.h new file mode 100644 index 0000000000..b4a16f1caa --- /dev/null +++ b/src/engine/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. + 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/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..0f82efff0c 100644 --- a/src/engine/SpatialJoin.h +++ b/src/engine/SpatialJoin.h @@ -10,6 +10,7 @@ #include #include "engine/Operation.h" +#include "engine/PayloadVariables.h" #include "global/Id.h" #include "parser/data/Variable.h" @@ -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/SpatialQuery.cpp b/src/parser/SpatialQuery.cpp index cffebc96fa..a58a78df19 100644 --- a/src/parser/SpatialQuery.cpp +++ b/src/parser/SpatialQuery.cpp @@ -7,6 +7,7 @@ #include #include +#include "engine/PayloadVariables.h" #include "engine/SpatialJoin.h" #include "parser/MagicServiceIriConstants.h" #include "parser/data/Variable.h" @@ -69,14 +70,11 @@ void SpatialQuery::addParameter(const SparqlTriple& triple) { // If we have already selected all payload variables, we can ignore // another explicitly selected variable. - if (std::holds_alternative>(payloadVariables_)) { - std::get>(payloadVariables_) - .push_back(getVariable("payload", object)); - } + payloadVariables_.addVariable(getVariable("payload", object)); } else if (object.isIri() && extractParameterName(object, SPATIAL_SEARCH_IRI) == "all") { // All variables selected - payloadVariables_ = PayloadAllVariables{}; + payloadVariables_.setToAll(); } else { throw SpatialSearchException( "The argument to the parameter must be either a variable " @@ -106,16 +104,6 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const { "Missing parameter in spatial search."); } - // Helper visitor to check if the payload variables are empty - auto emptyPayload = [](const T& value) -> bool { - if constexpr (std::is_same_v) { - return false; - } else { - static_assert(std::is_same_v>); - 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. @@ -127,7 +115,7 @@ SpatialJoinConfiguration SpatialQuery::toSpatialJoinConfiguration() const { "spatialSearch: { [Config Triples] { ?right " "} }."); } else if (!ignoreMissingRightChild_ && !childGraphPattern_.has_value() && - !std::visit(emptyPayload, payloadVariables_)) { + !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 " @@ -144,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 145cd2dc6b..f6460280ca 100644 --- a/src/parser/SpatialQuery.h +++ b/src/parser/SpatialQuery.h @@ -40,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. - PayloadVariables payloadVariables_ = std::vector{}; + 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 608d28da0e..8b05f89577 100644 --- a/test/QueryPlannerTest.cpp +++ b/test/QueryPlannerTest.cpp @@ -5,7 +5,9 @@ #include +#include "./printers/PayloadVariablePrinters.h" #include "QueryPlannerTestHelpers.h" +#include "engine/PayloadVariables.h" #include "engine/QueryPlanner.h" #include "engine/SpatialJoin.h" #include "parser/GraphPatternOperation.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,7 +1769,7 @@ 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")))); @@ -1784,7 +1787,7 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { "_:config spatialSearch:payload ." "{ ?a

?a2 . ?a2

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

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

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

", "?b")))); h::expect( @@ -1800,7 +1803,7 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { "_:config spatialSearch:payload spatialSearch:all ." "{ ?a

?a2 . ?a2

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

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

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

", "?b")))); @@ -1819,7 +1822,7 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { "_:config spatialSearch:payload ?a ." "{ ?a

?a2 . ?a2

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

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

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

", "?b")))); } @@ -1844,7 +1847,7 @@ TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) { "spatialSearch:maxDistance 1 . " " } }", h::SpatialJoin(1, -1, 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("PREFIX spatialSearch: " @@ -1888,6 +1891,7 @@ TEST(QueryPlanner, SpatialJoinMultipleServiceSharedLeft) { auto scan = h::IndexScanFromStrings; using V = Variable; auto S2 = SpatialJoinAlgorithm::S2_GEOMETRY; + using PV = PayloadVariables; h::expect( "SELECT * WHERE {" @@ -1901,20 +1905,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 {" @@ -1942,18 +1942,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) { @@ -2231,7 +2231,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 {" @@ -2240,7 +2240,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 @@ -2293,7 +2293,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 {" @@ -2303,7 +2303,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/CMakeLists.txt b/test/engine/CMakeLists.txt index 47704485d6..4a949d51be 100644 --- a/test/engine/CMakeLists.txt +++ b/test/engine/CMakeLists.txt @@ -11,3 +11,4 @@ addLinkAndDiscoverTest(CountConnectedSubgraphsTest) addLinkAndDiscoverTest(BindTest engine) addLinkAndRunAsSingleTest(SpatialJoinAlgorithmsTest engine) addLinkAndDiscoverTestSerial(QueryExecutionTreeTest engine) +addLinkAndDiscoverTest(PayloadVariablesTest engine) diff --git a/test/engine/PayloadVariablesTest.cpp b/test/engine/PayloadVariablesTest.cpp new file mode 100644 index 0000000000..bce11d44f0 --- /dev/null +++ b/test/engine/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 "engine/PayloadVariables.h" +#include "gmock/gmock.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/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/printers/PayloadVariablePrinters.h b/test/printers/PayloadVariablePrinters.h new file mode 100644 index 0000000000..b4d32bc96c --- /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 "engine/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{" "}; + } + } +} From 7eaf7cd9e1ecc4337ca80eeeef686bf67dac3425 Mon Sep 17 00:00:00 2001 From: ullingerc Date: Thu, 5 Dec 2024 13:46:20 +0100 Subject: [PATCH 4/8] apply feedback --- src/engine/PayloadVariables.cpp | 17 ++++------------- src/engine/PayloadVariables.h | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/engine/PayloadVariables.cpp b/src/engine/PayloadVariables.cpp index 0f9165e24a..c0c23ba024 100644 --- a/src/engine/PayloadVariables.cpp +++ b/src/engine/PayloadVariables.cpp @@ -50,24 +50,15 @@ bool PayloadVariables::empty() const { // ____________________________________________________________________________ bool PayloadVariables::isAll() const { - // Helper visitor to check if the payload variables has been set to all - auto allVisitor = [](const T&) -> bool { - if constexpr (std::is_same_v) { - return true; - } else { - static_assert(std::is_same_v>); - return false; - } - }; - - return std::visit(allVisitor, variables_); + return std::holds_alternative(variables_); }; // ____________________________________________________________________________ -std::vector PayloadVariables::getVariables() const { +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) -> std::vector { + auto getVarVisitor = + [](const T& value) -> const std::vector& { if constexpr (std::is_same_v>) { return value; } else { diff --git a/src/engine/PayloadVariables.h b/src/engine/PayloadVariables.h index b4a16f1caa..c69214be6b 100644 --- a/src/engine/PayloadVariables.h +++ b/src/engine/PayloadVariables.h @@ -44,7 +44,7 @@ class PayloadVariables { bool isAll() const; // Returns a vector of variables if all has not been set. Otherwise throws. - std::vector getVariables() const; + const std::vector& getVariables() const; // For testing: equality operator bool operator==(const PayloadVariables& other) const { From 03b1ea41e108694fd79cb04d8c36a89857ea6e26 Mon Sep 17 00:00:00 2001 From: ullingerc Date: Thu, 5 Dec 2024 13:48:39 +0100 Subject: [PATCH 5/8] add edge case test and explanatory comments --- test/QueryPlannerTest.cpp | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/test/QueryPlannerTest.cpp b/test/QueryPlannerTest.cpp index 8b05f89577..9b94b43ce0 100644 --- a/test/QueryPlannerTest.cpp +++ b/test/QueryPlannerTest.cpp @@ -1828,13 +1828,12 @@ TEST(QueryPlanner, SpatialJoinServicePayloadVars) { } 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 {" @@ -1847,8 +1846,31 @@ TEST(QueryPlanner, SpatialJoinServiceMaxDistOutside) { "spatialSearch:maxDistance 1 . " " } }", h::SpatialJoin(1, -1, V{"?y"}, V{"?b"}, std::nullopt, + // 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: " "" @@ -1866,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: " "" From 33e3c7b82642d3905302dd59009a642e9d1ed1d3 Mon Sep 17 00:00:00 2001 From: ullingerc Date: Thu, 5 Dec 2024 13:50:01 +0100 Subject: [PATCH 6/8] smaller include --- src/parser/SpatialQuery.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/SpatialQuery.h b/src/parser/SpatialQuery.h index f6460280ca..09479f4066 100644 --- a/src/parser/SpatialQuery.h +++ b/src/parser/SpatialQuery.h @@ -4,7 +4,7 @@ #pragma once -#include "engine/SpatialJoin.h" +#include "engine/PayloadVariables.h" #include "parser/GraphPattern.h" #include "parser/MagicServiceQuery.h" From 218b1098df209adc4d744de87bbfcb768d9c0f9e Mon Sep 17 00:00:00 2001 From: ullingerc Date: Thu, 5 Dec 2024 16:20:25 +0100 Subject: [PATCH 7/8] move payloadvariables to parser --- src/engine/CMakeLists.txt | 2 +- src/engine/SpatialJoin.h | 2 +- src/parser/CMakeLists.txt | 1 + src/{engine => parser}/PayloadVariables.cpp | 2 +- src/{engine => parser}/PayloadVariables.h | 0 src/parser/SpatialQuery.cpp | 2 +- src/parser/SpatialQuery.h | 2 +- test/QueryPlannerTest.cpp | 2 +- test/engine/PayloadVariablesTest.cpp | 2 +- test/printers/PayloadVariablePrinters.h | 2 +- 10 files changed, 9 insertions(+), 8 deletions(-) rename src/{engine => parser}/PayloadVariables.cpp (98%) rename src/{engine => parser}/PayloadVariables.h (100%) diff --git a/src/engine/CMakeLists.txt b/src/engine/CMakeLists.txt index 232f3e1d2f..cbfb3344c3 100644 --- a/src/engine/CMakeLists.txt +++ b/src/engine/CMakeLists.txt @@ -10,7 +10,7 @@ add_library(engine Union.cpp MultiColumnJoin.cpp TransitivePathBase.cpp TransitivePathHashMap.cpp TransitivePathBinSearch.cpp Service.cpp Values.cpp Bind.cpp Minus.cpp RuntimeInformation.cpp CheckUsePatternTrick.cpp - VariableToColumnMap.cpp ExportQueryExecutionTrees.cpp PayloadVariables.cpp + VariableToColumnMap.cpp ExportQueryExecutionTrees.cpp CartesianProductJoin.cpp TextIndexScanForWord.cpp TextIndexScanForEntity.cpp TextLimit.cpp LazyGroupBy.cpp GroupByHashMapOptimization.cpp SpatialJoin.cpp CountConnectedSubgraphs.cpp SpatialJoinAlgorithms.cpp PathSearch.cpp ExecuteUpdate.cpp) diff --git a/src/engine/SpatialJoin.h b/src/engine/SpatialJoin.h index 0f82efff0c..af76c49a81 100644 --- a/src/engine/SpatialJoin.h +++ b/src/engine/SpatialJoin.h @@ -10,8 +10,8 @@ #include #include "engine/Operation.h" -#include "engine/PayloadVariables.h" #include "global/Id.h" +#include "parser/PayloadVariables.h" #include "parser/data/Variable.h" // A nearest neighbor search with optionally a maximum distance. 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/engine/PayloadVariables.cpp b/src/parser/PayloadVariables.cpp similarity index 98% rename from src/engine/PayloadVariables.cpp rename to src/parser/PayloadVariables.cpp index c0c23ba024..a785e23c21 100644 --- a/src/engine/PayloadVariables.cpp +++ b/src/parser/PayloadVariables.cpp @@ -2,7 +2,7 @@ // Chair of Algorithms and Data Structures. // Author: Christoph Ullinger -#include "engine/PayloadVariables.h" +#include "parser/PayloadVariables.h" // ____________________________________________________________________________ PayloadVariables::PayloadVariables(std::vector variables) diff --git a/src/engine/PayloadVariables.h b/src/parser/PayloadVariables.h similarity index 100% rename from src/engine/PayloadVariables.h rename to src/parser/PayloadVariables.h diff --git a/src/parser/SpatialQuery.cpp b/src/parser/SpatialQuery.cpp index a58a78df19..5112db3a20 100644 --- a/src/parser/SpatialQuery.cpp +++ b/src/parser/SpatialQuery.cpp @@ -7,9 +7,9 @@ #include #include -#include "engine/PayloadVariables.h" #include "engine/SpatialJoin.h" #include "parser/MagicServiceIriConstants.h" +#include "parser/PayloadVariables.h" #include "parser/data/Variable.h" namespace parsedQuery { diff --git a/src/parser/SpatialQuery.h b/src/parser/SpatialQuery.h index 09479f4066..f472a53806 100644 --- a/src/parser/SpatialQuery.h +++ b/src/parser/SpatialQuery.h @@ -4,9 +4,9 @@ #pragma once -#include "engine/PayloadVariables.h" #include "parser/GraphPattern.h" #include "parser/MagicServiceQuery.h" +#include "parser/PayloadVariables.h" namespace parsedQuery { diff --git a/test/QueryPlannerTest.cpp b/test/QueryPlannerTest.cpp index 9b94b43ce0..c9a104bdcb 100644 --- a/test/QueryPlannerTest.cpp +++ b/test/QueryPlannerTest.cpp @@ -7,11 +7,11 @@ #include "./printers/PayloadVariablePrinters.h" #include "QueryPlannerTestHelpers.h" -#include "engine/PayloadVariables.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" diff --git a/test/engine/PayloadVariablesTest.cpp b/test/engine/PayloadVariablesTest.cpp index bce11d44f0..9a0f2c6c31 100644 --- a/test/engine/PayloadVariablesTest.cpp +++ b/test/engine/PayloadVariablesTest.cpp @@ -6,8 +6,8 @@ #include "../printers/PayloadVariablePrinters.h" #include "../util/GTestHelpers.h" -#include "engine/PayloadVariables.h" #include "gmock/gmock.h" +#include "parser/PayloadVariables.h" #include "parser/data/Variable.h" namespace { // anonymous namespace to avoid linker problems diff --git a/test/printers/PayloadVariablePrinters.h b/test/printers/PayloadVariablePrinters.h index b4d32bc96c..efcb9ffb26 100644 --- a/test/printers/PayloadVariablePrinters.h +++ b/test/printers/PayloadVariablePrinters.h @@ -4,7 +4,7 @@ #pragma once #include "./VariablePrinters.h" -#include "engine/PayloadVariables.h" +#include "parser/PayloadVariables.h" // _____________________________________________________________ inline void PrintTo(const PayloadVariables& pv, std::ostream* os) { From 50fee8ccc9e6dbc68081dffec5668c7b6affcac4 Mon Sep 17 00:00:00 2001 From: ullingerc Date: Thu, 5 Dec 2024 16:29:32 +0100 Subject: [PATCH 8/8] also move the unit test --- test/engine/CMakeLists.txt | 1 - test/parser/CMakeLists.txt | 1 + test/{engine => parser}/PayloadVariablesTest.cpp | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename test/{engine => parser}/PayloadVariablesTest.cpp (100%) diff --git a/test/engine/CMakeLists.txt b/test/engine/CMakeLists.txt index 4a949d51be..47704485d6 100644 --- a/test/engine/CMakeLists.txt +++ b/test/engine/CMakeLists.txt @@ -11,4 +11,3 @@ addLinkAndDiscoverTest(CountConnectedSubgraphsTest) addLinkAndDiscoverTest(BindTest engine) addLinkAndRunAsSingleTest(SpatialJoinAlgorithmsTest engine) addLinkAndDiscoverTestSerial(QueryExecutionTreeTest engine) -addLinkAndDiscoverTest(PayloadVariablesTest engine) 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/engine/PayloadVariablesTest.cpp b/test/parser/PayloadVariablesTest.cpp similarity index 100% rename from test/engine/PayloadVariablesTest.cpp rename to test/parser/PayloadVariablesTest.cpp