Better separate SparqlTriple and SparqlTripleSimple types to avoid accidental mismatches

src/engine/CheckUsePatternTrick.cpp

@@ -60,11 +60,7 @@ bool isVariableContainedInGraphPatternOperation(
               return false;
             }
             return (triple.s_ == variable ||
-                    // Complex property paths are not allowed to contain
-                    // variables in SPARQL, so this check is sufficient.
-                    // TODO<joka921> Still make the interface of the
-                    // `PropertyPath` class typesafe.
-                    triple.p_.asString() == variable.name() ||
+                    triple.getPredicateVariable() == variable ||
                     triple.o_ == variable);
           });
     } else if constexpr (std::is_same_v<T, p::Values>) {
@@ -104,7 +100,7 @@ static void rewriteTriplesForPatternTrick(const PatternTrickTuple& subAndPred,
     auto matchingTriple = ql::ranges::find_if(
         triples, [&subAndPred, triplePosition](const SparqlTriple& t) {
           return std::invoke(triplePosition, t) == subAndPred.subject_ &&
-                 t.p_.isIri() && !isVariable(t.p_);
+                 t.getSimplePredicate().has_value();
         });
     if (matchingTriple == triples.end()) {
       return false;
@@ -218,25 +214,27 @@ std::optional<PatternTrickTuple> isTripleSuitableForPatternTrick(
 
   const auto patternTrickDataIfTripleIsPossible =
       [&]() -> std::optional<PatternTrickData> {
-    if ((triple.p_.iri_ == HAS_PREDICATE_PREDICATE) && isVariable(triple.s_) &&
-        isVariable(triple.o_) && triple.s_ != triple.o_) {
+    if (triple.getSimplePredicate() == HAS_PREDICATE_PREDICATE &&
+        triple.s_.isVariable() && triple.o_.isVariable() &&
+        triple.s_ != triple.o_) {
       Variable predicateVariable{triple.o_.getVariable()};
       return PatternTrickData{predicateVariable,
                               triple.s_.getVariable(),
                               {predicateVariable},
                               true};
-    } else if (isVariable(triple.s_) && isVariable(triple.p_) &&
-               isVariable(triple.o_)) {
+    } else if (auto variable = triple.getPredicateVariable();
+               triple.s_.isVariable() && variable.has_value() &&
+               triple.o_.isVariable()) {
+      auto predicateVariable = variable.value();
       // Check that the three variables are pairwise distinct.
-      std::vector<string> variables{triple.s_.getVariable().name(),
-                                    triple.o_.getVariable().name(),
-                                    triple.p_.asString()};
+      std::array variables{triple.s_.getVariable().name(),
+                           triple.o_.getVariable().name(),
+                           predicateVariable.name()};
       ql::ranges::sort(variables);
       if (std::unique(variables.begin(), variables.end()) != variables.end()) {
         return std::nullopt;
       }
 
-      Variable predicateVariable{triple.p_.getIri()};
       return PatternTrickData{predicateVariable,
                               triple.s_.getVariable(),
                               {predicateVariable, triple.o_.getVariable()},

src/engine/HasPredicateScan.cpp

@@ -34,8 +34,10 @@ static constexpr auto makeJoin =
       subtree->getVariableAndInfoByColumnIndex(subtreeColIndex).first;
   auto hasPatternScan = ad_utility::makeExecutionTree<IndexScan>(
       qec, Permutation::Enum::PSO,
-      SparqlTriple{subtreeVar, std::string{HAS_PATTERN_PREDICATE},
-                   objectVariable});
+      SparqlTripleSimple{
+          subtreeVar,
+          ad_utility::triple_component::Iri::fromIriref(HAS_PATTERN_PREDICATE),
+          objectVariable});
   auto joinedSubtree = ad_utility::makeExecutionTree<Join>(
       qec, std::move(subtree), std::move(hasPatternScan), subtreeColIndex, 0);
   auto column =
@@ -59,17 +61,17 @@ HasPredicateScan::HasPredicateScan(QueryExecutionContext* qec,
 // `ScanType`.
 static HasPredicateScan::ScanType getScanType(const SparqlTriple& triple) {
   using enum HasPredicateScan::ScanType;
-  AD_CONTRACT_CHECK(triple.p_.iri_ == HAS_PREDICATE_PREDICATE);
-  if (isVariable(triple.s_) && (isVariable(triple.o_))) {
+  AD_CONTRACT_CHECK(triple.getSimplePredicate() == HAS_PREDICATE_PREDICATE);
+  if (triple.s_.isVariable() && triple.o_.isVariable()) {
     if (triple.s_ == triple.o_) {
       throw std::runtime_error{
           "ql:has-predicate with same variable for subject and object not "
           "supported."};
     }
     return FULL_SCAN;
-  } else if (isVariable(triple.s_)) {
+  } else if (triple.s_.isVariable()) {
     return FREE_S;
-  } else if (isVariable(triple.o_)) {
+  } else if (triple.o_.isVariable()) {
     return FREE_O;
   } else {
     AD_FAIL();

src/engine/IndexScan.cpp

@@ -61,13 +61,6 @@ IndexScan::IndexScan(QueryExecutionContext* qec, Permutation::Enum permutation,
   }
 }
 
-// _____________________________________________________________________________
-IndexScan::IndexScan(QueryExecutionContext* qec, Permutation::Enum permutation,
-                     const SparqlTriple& triple, Graphs graphsToFilter,
-                     PrefilterIndexPair prefilter)
-    : IndexScan(qec, permutation, triple.getSimple(), std::move(graphsToFilter),
-                std::move(prefilter)) {}
-
 // _____________________________________________________________________________
 IndexScan::IndexScan(QueryExecutionContext* qec, Permutation::Enum permutation,
                      const TripleComponent& s, const TripleComponent& p,

src/engine/IndexScan.h

@@ -39,9 +39,6 @@ class IndexScan final : public Operation {
   std::vector<Variable> additionalVariables_;
 
  public:
-  IndexScan(QueryExecutionContext* qec, Permutation::Enum permutation,
-            const SparqlTriple& triple, Graphs graphsToFilter = std::nullopt,
-            PrefilterIndexPair prefilter = std::nullopt);
   IndexScan(QueryExecutionContext* qec, Permutation::Enum permutation,
             const SparqlTripleSimple& triple,
             Graphs graphsToFilter = std::nullopt,

src/engine/QueryPlanner.cpp

@@ -498,7 +498,7 @@
   vector<const SparqlTriple*> entityTriples;
   // Add one or more nodes for each triple.
   for (auto& t : pattern->_triples) {
-    if (t.p_.iri_ == CONTAINS_WORD_PREDICATE) {
+    if (t.getSimplePredicate() == CONTAINS_WORD_PREDICATE) {
       std::string buffer = t.o_.toString();
       std::string_view sv{buffer};
       // Add one node for each word
@@ -517,7 +517,7 @@
             tg);
         numNodesInTripleGraph++;
       }
-    } else if (t.p_.iri_ == CONTAINS_ENTITY_PREDICATE) {
+    } else if (t.getSimplePredicate() == CONTAINS_ENTITY_PREDICATE) {
       entityTriples.push_back(&t);
     } else {
       addNodeToTripleGraph(
@@ -608,9 +608,9 @@
     const AddedIndexScanFunction& addIndexScan) const {
   using enum Permutation::Enum;
 
-  if (isVariable(triple.s_)) {
+  if (triple.s_.isVariable()) {
     addIndexScan(POS);
-  } else if (isVariable(triple.p_)) {
+  } else if (triple.p_.isVariable()) {
     addIndexScan(SOP);
   } else {
     addIndexScan(PSO);
@@ -644,22 +644,22 @@
 
   using Tr = SparqlTripleSimple;
 
-  if (!isVariable(s)) {
+  if (!s.isVariable()) {
     if (p == o) {
       handleRepeatedVariables({{SPO}}, &Tr::o_);
     } else {
       addIndexScan(SPO);
       addIndexScan(SOP);
     }
-  } else if (!isVariable(p)) {
+  } else if (!p.isVariable()) {
     if (s == o) {
       handleRepeatedVariables({{PSO}}, &Tr::o_);
     } else {
       addIndexScan(PSO);
       addIndexScan(POS);
     }
   } else {
-    AD_CORRECTNESS_CHECK(!isVariable(o));
+    AD_CORRECTNESS_CHECK(!o.isVariable());
     if (s == p) {
       handleRepeatedVariables({{OPS}}, &Tr::s_);
     } else {
@@ -726,9 +726,9 @@
     const TripleGraph::Node& node, const AddedIndexScanFunction& addIndexScan,
     const AddFilter& addFilter) {
   auto triple = node.triple_.getSimple();
-  const size_t numVars = static_cast<size_t>(isVariable(triple.s_)) +
-                         static_cast<size_t>(isVariable(triple.p_)) +
-                         static_cast<size_t>(isVariable(triple.o_));
+  const size_t numVars = static_cast<size_t>(triple.s_.isVariable()) +
+                         static_cast<size_t>(triple.p_.isVariable()) +
+                         static_cast<size_t>(triple.o_.isVariable());
   if (numVars == 0) {
     // We could read this from any of the permutations.
     addIndexScan(Permutation::Enum::PSO);
@@ -783,13 +783,8 @@
       continue;
     }
 
-    // Property paths must have been handled previously.
-    AD_CORRECTNESS_CHECK(node.triple_.p_.operation_ ==
-                         PropertyPath::Operation::IRI);
-    // At this point, we know that the predicate is a simple IRI or a variable.
-
     if (_qec && !_qec->getIndex().hasAllPermutations() &&
-        isVariable(node.triple_.p_.iri_)) {
+        node.triple_.getPredicateVariable().has_value()) {
       AD_THROW(
           "The query contains a predicate variable, but only the PSO "
           "and POS permutations were loaded. Rerun the server without "
@@ -798,7 +793,17 @@
     }
 
     // Backward compatibility with spatial search predicates
-    const auto& input = node.triple_.p_.iri_;
+    const auto& input = std::visit(
+        ad_utility::OverloadCallOperator{
+            [](const PropertyPath& propertyPath) -> const std::string& {
+              AD_CORRECTNESS_CHECK(propertyPath.operation_ ==
+                                   PropertyPath::Operation::IRI);
+              return propertyPath.iri_;
+            },
+            [](const Variable& var) -> const std::string& {
+              return var.name();
+            }},
+        node.triple_.p_);
     if ((input.starts_with(MAX_DIST_IN_METERS) ||
          input.starts_with(NEAREST_NEIGHBORS)) &&
         input.ends_with('>')) {
@@ -818,7 +823,7 @@
       continue;
     }
 
-    if (node.triple_.p_.iri_ == HAS_PREDICATE_PREDICATE) {
+    if (input == HAS_PREDICATE_PREDICATE) {
       pushPlan(makeSubtreePlan<HasPredicateScan>(_qec, node.triple_));
       continue;
     }
@@ -882,7 +887,9 @@
     case PropertyPath::Operation::NEGATED:
       return seedFromNegated(left, path, right);
     case PropertyPath::Operation::IRI:
-      return seedFromIri(left, path, right);
+      return seedFromVarOrIri(
+          left, ad_utility::triple_component::Iri::fromIriref(path.iri_),
+          right);
     case PropertyPath::Operation::SEQUENCE:
       return seedFromSequence(left, path, right);
     case PropertyPath::Operation::ZERO_OR_MORE:
@@ -1024,8 +1031,7 @@
                                   const std::vector<string_view>& iris) {
     using namespace sparqlExpression;
     Variable variable = generateUniqueVarName();
-    ParsedQuery::GraphPattern pattern =
-        seedFromIri(left, PropertyPath::fromVariable(variable), right);
+    ParsedQuery::GraphPattern pattern = seedFromVarOrIri(left, variable, right);
     std::ostringstream descriptor;
     auto expression = makeNotEqualExpression(variable, iris.at(0));
     appendNotEqualString(descriptor, iris.at(0), variable);
@@ -1053,12 +1059,24 @@
 }
 
 // _____________________________________________________________________________
-ParsedQuery::GraphPattern QueryPlanner::seedFromIri(
-    const TripleComponent& left, const PropertyPath& path,
+ParsedQuery::GraphPattern QueryPlanner::seedFromVarOrIri(
+    const TripleComponent& left,
+    const ad_utility::sparql_types::VarOrIri& varOrIri,
     const TripleComponent& right) {
   ParsedQuery::GraphPattern p{};
   p::BasicGraphPattern basic;
-  basic._triples.push_back(SparqlTriple(left, path, right));
+  basic._triples.push_back(SparqlTriple(
+      left,
+      std::visit(
+          ad_utility::OverloadCallOperator{
+              [](const Variable& variable)
+                  -> ad_utility::sparql_types::VarOrPath { return variable; },
+              [](const ad_utility::triple_component::Iri& iri)
+                  -> ad_utility::sparql_types::VarOrPath {
+                return PropertyPath::fromIri(iri.toStringRepresentation());
+              }},
+          varOrIri),
+      right));
   p._graphPatterns.emplace_back(std::move(basic));
 
   return p;
@@ -1098,7 +1116,7 @@
   if (!textLimits.contains(cvar)) {
     textLimits[cvar] = parsedQuery::TextLimitMetaObject{{}, {}, 0};
   }
-  if (node.triple_.p_.iri_ == CONTAINS_ENTITY_PREDICATE) {
+  if (node.triple_.getSimplePredicate() == CONTAINS_ENTITY_PREDICATE) {
     if (node._variables.size() == 2) {
       // TODO<joka921>: This is not nice, refactor the whole TripleGraph class
       // to make these checks more explicitly.
@@ -1667,10 +1685,14 @@
 
 // _____________________________________________________________________________
 bool QueryPlanner::TripleGraph::isTextNode(size_t i) const {
-  return _nodeMap.count(i) > 0 &&
-         (_nodeMap.find(i)->second->triple_.p_.iri_ ==
-              CONTAINS_ENTITY_PREDICATE ||
-          _nodeMap.find(i)->second->triple_.p_.iri_ == CONTAINS_WORD_PREDICATE);
+  auto it = _nodeMap.find(i);
+  if (it == _nodeMap.end()) {
+    return false;
+  }
+  const auto& triple = it->second->triple_;
+  auto predicate = triple.getSimplePredicate();
+  return predicate == CONTAINS_ENTITY_PREDICATE ||
+         predicate == CONTAINS_WORD_PREDICATE;
 }
 
 // _____________________________________________________________________________
@@ -2716,25 +2738,26 @@
   // A basic graph patterns consists only of triples. First collect all
   // the bound variables.
   for (const SparqlTriple& t : v._triples) {
-    if (isVariable(t.s_)) {
+    if (t.s_.isVariable()) {
       boundVariables_.insert(t.s_.getVariable());
     }
-    if (isVariable(t.p_)) {
-      boundVariables_.insert(Variable{t.p_.iri_});
+    if (auto predicate = t.getPredicateVariable()) {
+      boundVariables_.insert(predicate.value());
     }
-    if (isVariable(t.o_)) {
+    if (t.o_.isVariable()) {
       boundVariables_.insert(t.o_.getVariable());
     }
   }
 
   // Then collect the triples. Transform each triple with a property path to
   // an equivalent form without property path (using `seedFromPropertyPath`).
   for (const auto& triple : v._triples) {
-    if (triple.p_.operation_ == PropertyPath::Operation::IRI) {
+    if (std::holds_alternative<Variable>(triple.p_) ||
+        triple.getSimplePredicate().has_value()) {
       candidateTriples_._triples.push_back(triple);
     } else {
-      auto children =
-          planner_.seedFromPropertyPath(triple.s_, triple.p_, triple.o_);
+      auto children = planner_.seedFromPropertyPath(
+          triple.s_, std::get<PropertyPath>(triple.p_), triple.o_);
       for (auto& child : children._graphPatterns) {
         std::visit([self = this](
                        auto& arg) { self->graphPatternOperationVisitor(arg); },

src/engine/QueryPlanner.h

@@ -15,6 +15,7 @@
 #include "parser/GraphPattern.h"
 #include "parser/GraphPatternOperation.h"
 #include "parser/ParsedQuery.h"
+#include "parser/data/Types.h"
 
 class QueryPlanner {
   using TextLimitMap =
@@ -57,13 +58,13 @@ class QueryPlanner {
       Node(size_t id, SparqlTriple t,
            std::optional<Variable> graphVariable = std::nullopt)
           : id_(id), triple_(std::move(t)) {
-        if (isVariable(triple_.s_)) {
+        if (triple_.s_.isVariable()) {
           _variables.insert(triple_.s_.getVariable());
         }
-        if (isVariable(triple_.p_)) {
-          _variables.insert(Variable{triple_.p_.iri_});
+        if (auto predicate = triple_.getPredicateVariable()) {
+          _variables.insert(predicate.value());
         }
-        if (isVariable(triple_.o_)) {
+        if (triple_.o_.isVariable()) {
           _variables.insert(triple_.o_.getVariable());
         }
         if (graphVariable.has_value()) {
@@ -318,9 +319,10 @@ class QueryPlanner {
   ParsedQuery::GraphPattern seedFromNegated(const TripleComponent& left,
                                             const PropertyPath& path,
                                             const TripleComponent& right);
-  static ParsedQuery::GraphPattern seedFromIri(const TripleComponent& left,
-                                               const PropertyPath& path,
-                                               const TripleComponent& right);
+  static ParsedQuery::GraphPattern seedFromVarOrIri(
+      const TripleComponent& left,
+      const ad_utility::sparql_types::VarOrIri& varOrIri,
+      const TripleComponent& right);
 
   Variable generateUniqueVarName();