Insert ql:langtag triple when inserting triple with an object with a language tag (#2674)

For each triple with an object with a language tag, QLever stores two additional so-called "internal" triples. For example, for `wd:Q873 rdfs:label "Meryl Streep"@en`, QLever also stores `wd:Q873 @en@rdfs:label "Meryl Streep"@en` and `"Meryl Streep"@en ql:langtag ql:@en`. These triples are used for the efficient processing of queries involving a language filter like `FILTER (LANG(?label) = "en")`. Specifically, the internal `@en@rdfs:label` triple is used when the predicate of the graph pattern introducing `?label` is fixed, while the internal `ql:langtag` triple is used when the predicate is a variable.

Since #2561 and #2461, the `@lang@...` triples are properly taking care of when processing an update operation. However, the corresponding `ql:langtag` triple was not updated so far, thus leading to potentially incorrect results for queries involving a language filter for an object introduced by a graph pattern with a variable predicate.

With this change, the corresponding `ql:langtag` is added when inserting a triple with an object with a language tag. However, when deleting such a triple, the corresponding `ql:langtag` triple is **not** deleted (because this would require keeping track of how many subjects require that triple). This does not harm correctness. It does potentially increase the number of delta triples beyond what is required.

Author: RobinTF

src/engine/SpatialJoinParser.cpp

@@ -13,7 +13,7 @@ namespace ad_utility::detail::parallel_wkt_parser {
 // _____________________________________________________________________________
 WKTParser::WKTParser(sj::Sweeper* sweeper, size_t numThreads,
                      bool usePrefiltering,
-                     const std::optional<util::geo::DBox>& prefilterLatLngBox,
+                     const std::optional<::util::geo::DBox>& prefilterLatLngBox,
                      const Index& index)
     : sj::WKTParserBase<SpatialJoinParseJob>(sweeper, numThreads),
       _numSkipped(numThreads),
@@ -66,22 +66,22 @@ void WKTParser::processQueue(size_t t) {
         parseCounter++;
       } else if (dt == Datatype::GeoPoint) {
         const auto& p = job.valueId.getGeoPoint();
-        const util::geo::DPoint utilPoint{p.getLng(), p.getLat()};
+        const ::util::geo::DPoint utilPoint{p.getLng(), p.getLat()};
 
         // If point is not contained in the prefilter box, we can skip it
         // immediately instead of feeding it to the parser.
         if (_prefilterLatLngBox.has_value() &&
-            !util::geo::intersects(_prefilterLatLngBox.value(), utilPoint)) {
+            !::util::geo::intersects(_prefilterLatLngBox.value(), utilPoint)) {
           prefilterCounter++;
           continue;
         }
         // parse point directly
         auto mercPoint = latLngToWebMerc(utilPoint);
 
-        util::geo::I32Point addPoint{
+        ::util::geo::I32Point addPoint{
             static_cast<int32_t>(mercPoint.getX() * PREC),
             static_cast<int32_t>(mercPoint.getY() * PREC)};
-        _bboxes[t] = util::geo::extendBox(
+        _bboxes[t] = ::util::geo::extendBox(
             _sweeper->add(addPoint, std::to_string(job.line), job.side, w),
             _bboxes[t]);
         parseCounter++;

src/engine/SpatialJoinParser.h

@@ -40,7 +40,7 @@ inline bool operator==(const SpatialJoinParseJob& a,
 class WKTParser : public sj::WKTParserBase<SpatialJoinParseJob> {
  public:
   WKTParser(sj::Sweeper* sweeper, size_t numThreads, bool usePrefiltering,
-            const std::optional<util::geo::DBox>& prefilterLatLngBox,
+            const std::optional<::util::geo::DBox>& prefilterLatLngBox,
             const Index& index);
 
   // Enqueue a new row from the input table (given the `ValueId` of the
@@ -69,7 +69,7 @@ class WKTParser : public sj::WKTParserBase<SpatialJoinParseJob> {
 
   // Configure prefiltering geometries by bounding box.
   bool _usePrefiltering;
-  std::optional<util::geo::DBox> _prefilterLatLngBox;
+  std::optional<::util::geo::DBox> _prefilterLatLngBox;
 
   // A reference to QLever's index is needed to access precomputed geometry
   // bounding boxes and to resolve `ValueId`s into WKT literals.

src/index/DeltaTriples.cpp

@@ -20,7 +20,6 @@
 #include "index/Index.h"
 #include "index/IndexImpl.h"
 #include "index/LocatedTriples.h"
-#include "util/LruCache.h"
 #include "util/Serializer/TripleSerializer.h"
 
 // ____________________________________________________________________________
@@ -167,16 +166,22 @@ DeltaTriplesCount DeltaTriples::getCounts() const {
 }
 
 // _____________________________________________________________________________
-DeltaTriples::Triples DeltaTriples::makeInternalTriples(
-    const Triples& triples) {
+DeltaTriples::Triples DeltaTriples::makeInternalTriples(const Triples& triples,
+                                                        bool insertion) {
   // NOTE: If this logic is ever changed, you need to also change the code
   // in `IndexBuilderTypes.h`, the function `getIdMapLambdas` specifically,
   // which adds the same extra triples for language tags to the internal triples
   // on the initial index build.
   Triples internalTriples;
-  constexpr size_t predicateCacheSize = 50;
-  ad_utility::util::LRUCache<Id::T, ad_utility::triple_component::Iri>
-      predicateCache{predicateCacheSize};
+  // Initialize on first use.
+  if (languagePredicate_.isUndefined()) {
+    languagePredicate_ =
+        TripleComponent{
+            ad_utility::triple_component::Iri::fromIriref(LANGUAGE_PREDICATE)}
+            .toValueId(index_.getVocab(), localVocab_,
+                       index_.encodedIriManager());
+  }
+  ad_utility::HashSet<Id> addedObjects;
   for (const auto& triple : triples) {
     const auto& ids = triple.ids();
     Id objectId = ids.at(2);
@@ -188,28 +193,45 @@ DeltaTriples::Triples DeltaTriples::makeInternalTriples(
       continue;
     }
     const auto& predicate =
-        predicateCache.getOrCompute(ids.at(1).getBits(), [this](Id::T bits) {
+        predicateCache_.getOrCompute(ids.at(1).getBits(), [this](Id::T bits) {
           auto optionalPredicate = ExportQueryExecutionTrees::idToLiteralOrIri(
               index_, Id::fromBits(bits), localVocab_, true);
           AD_CORRECTNESS_CHECK(optionalPredicate.has_value());
           AD_CORRECTNESS_CHECK(optionalPredicate.value().isIri());
           return std::move(optionalPredicate.value().getIri());
         });
-    auto specialPredicate = ad_utility::convertToLanguageTaggedPredicate(
-        predicate,
-        asStringViewUnsafe(optionalLiteralOrIri.value().getLanguageTag()));
+    auto langtag =
+        asStringViewUnsafe(optionalLiteralOrIri.value().getLanguageTag());
+    auto specialPredicate =
+        ad_utility::convertToLanguageTaggedPredicate(predicate, langtag);
     Id specialId = TripleComponent{std::move(specialPredicate)}.toValueId(
         index_.getVocab(), localVocab_, index_.encodedIriManager());
-    // NOTE: We currently only add one of the language triples, specifically
-    // `<subject> @language@<predicate> "object"@language` because it is
-    // directly tied to a regular triple, so insertion and removal work exactly
-    // the same. `<object> ql:langtag <@language>` on the other hand needs
-    // either some sort of reference counting or we have to keep it
-    // indefinitely, even if the object is removed. This means that some queries
-    // will return no results for entries with language tags that were inserted
-    // via an UPDATE operation.
+    // Extra triple `<subject> @language@<predicate> "object"@language`.
     internalTriples.push_back(
-        IdTriple<0>{std::array{ids.at(0), specialId, ids.at(2), ids.at(3)}});
+        IdTriple<0>{std::array{ids.at(0), specialId, objectId, ids.at(3)}});
+    // If we have already added the triple for this object with its langtag we
+    // can't add it a second time.
+    if (addedObjects.contains(objectId)) {
+      continue;
+    }
+    Id langtagId =
+        languageTagCache_.getOrCompute(langtag, [this](const std::string& tag) {
+          return TripleComponent{ad_utility::convertLangtagToEntityUri(tag)}
+              .toValueId(index_.getVocab(), localVocab_,
+                         index_.encodedIriManager());
+        });
+
+    // Because we don't track the exact counts of existing objects, we just
+    // conservatively add these internal triples on insertion, and never remove
+    // them. This is inefficient, but never wrong because queries that use these
+    // internal triples will always join these internal triples with a regular
+    // index scan.
+    if (insertion) {
+      // Extra triple `"object"@language ql:langtag <@language>`.
+      internalTriples.push_back(IdTriple<0>{
+          std::array{objectId, languagePredicate_, langtagId, ids.at(3)}});
+      addedObjects.emplace(objectId);
+    }
   }
   // Because of the special predicates, we need to re-sort the triples.
   ql::ranges::sort(internalTriples);
@@ -221,7 +243,7 @@ void DeltaTriples::insertTriples(CancellationHandle cancellationHandle,
                                  Triples triples,
                                  ad_utility::timer::TimeTracer& tracer) {
   tracer.beginTrace("makeInternalTriples");
-  auto internalTriples = makeInternalTriples(triples);
+  auto internalTriples = makeInternalTriples(triples, true);
   tracer.endTrace("makeInternalTriples");
   tracer.beginTrace("externalPermutation");
   modifyTriplesImpl<false, true>(cancellationHandle, std::move(triples),
@@ -240,7 +262,7 @@ void DeltaTriples::deleteTriples(CancellationHandle cancellationHandle,
                                  Triples triples,
                                  ad_utility::timer::TimeTracer& tracer) {
   tracer.beginTrace("makeInternalTriples");
-  auto internalTriples = makeInternalTriples(triples);
+  auto internalTriples = makeInternalTriples(triples, false);
   tracer.endTrace("makeInternalTriples");
   tracer.beginTrace("externalPermutation");
   modifyTriplesImpl<false, false>(cancellationHandle, std::move(triples),
@@ -344,11 +366,7 @@ void DeltaTriples::modifyTriplesImpl(CancellationHandle cancellationHandle,
   tracer.beginTrace("rewriteLocalVocabEntries");
   rewriteLocalVocabEntriesAndBlankNodes(triples);
   tracer.endTrace("rewriteLocalVocabEntries");
-  // TODO<joka921> Once the migration is finished, check whether we can remove
-  // the `ifndef` here again.
-#ifndef QLEVER_CPP_17
   AD_EXPENSIVE_CHECK(ql::ranges::is_sorted(triples));
-#endif
   AD_EXPENSIVE_CHECK(std::unique(triples.begin(), triples.end()) ==
                      triples.end());
   tracer.beginTrace("removeExistingTriples");
@@ -417,6 +435,9 @@ DeltaTriplesCount operator-(const DeltaTriplesCount& lhs,
 DeltaTriples::DeltaTriples(const Index& index)
     : DeltaTriples(index.getImpl()) {}
 
+// ____________________________________________________________________________
+DeltaTriples::DeltaTriples(const IndexImpl& index) : index_{index} {}
+
 // ____________________________________________________________________________
 DeltaTriplesManager::DeltaTriplesManager(const IndexImpl& index)
     : deltaTriples_{index},

src/index/DeltaTriples.h

@@ -19,6 +19,7 @@
 #include "index/IndexBuilderTypes.h"
 #include "index/LocatedTriples.h"
 #include "index/Permutation.h"
+#include "util/LruCache.h"
 #include "util/Synchronized.h"
 #include "util/TimeTracer.h"
 
@@ -128,6 +129,23 @@ class DeltaTriples {
   // See the documentation of `setPersist()` below.
   std::optional<std::string> filenameForPersisting_;
 
+  // Store the id of the `ql:langtag` predicate to avoid repeated disk lookups.
+  // This is initialized on first use.
+  Id languagePredicate_ = Id::makeUndefined();
+
+  // Store commonly used language tags of the form `<@lang>` to avoid repeated
+  // disk lookups.
+  static constexpr size_t languageTagCacheSize_ = 1000;
+  ad_utility::util::LRUCache<std::string, Id> languageTagCache_{
+      languageTagCacheSize_};
+
+  // Cache commonly used predicates and their IRI representation between calls
+  // of `makeInternalTriples`. For example in wikidata `wdt:P31`, or `wdt:P279`
+  // are frequently used, so we try to avoid an expensive lookup from disk.
+  static constexpr size_t predicateCacheSize_ = 1000;
+  ad_utility::util::LRUCache<Id::T, ad_utility::triple_component::Iri>
+      predicateCache_{predicateCacheSize_};
+
   // Assert that the Permutation Enum values have the expected int values.
   // This is used to store and lookup items that exist for permutation in an
   // array.
@@ -167,7 +185,7 @@ class DeltaTriples {
  public:
   // Construct for given index.
   explicit DeltaTriples(const Index& index);
-  explicit DeltaTriples(const IndexImpl& index) : index_{index} {}
+  explicit DeltaTriples(const IndexImpl& index);
 
   // Disable accidental copying.
   DeltaTriples(const DeltaTriples&) = delete;
@@ -213,8 +231,11 @@ class DeltaTriples {
   // bunch of triples to be inserted into the internal permutation to make
   // things like efficient language filters work. This currently performs a
   // lookup from disk to check the language tag, but in the future this may be
-  // implemented more efficiently.
-  Triples makeInternalTriples(const Triples& triples);
+  // implemented more efficiently. If `insertion` is false, this indicates that
+  // the triples are meant for deletion. In that case no triples are returned
+  // that may be unsafe to delete. In particular this refers to triples of the
+  // form `<object> ql:langtag <@language>`.
+  Triples makeInternalTriples(const Triples& triples, bool insertion);
 
   // Insert triples.
   void insertTriples(CancellationHandle cancellationHandle, Triples triples,

src/rdfTypes/GeometryInfoHelpersImpl.h

@@ -46,7 +46,7 @@ using ParsedWkt =
                  MultiPoint<CoordType>, MultiLine<CoordType>,
                  MultiPolygon<CoordType>, Collection<CoordType>>;
 using ParseResult = std::pair<WKTType, std::optional<ParsedWkt>>;
-using DAnyGeometry = util::geo::AnyGeometry<CoordType>;
+using DAnyGeometry = AnyGeometry<CoordType>;
 
 template <typename T>
 CPP_concept WktSingleGeometryType =
@@ -245,19 +245,16 @@ inline std::optional<std::string_view> wktTypeToIri(uint8_t type) {
 
 // Reverse projection applied by `sj::WKTParser`: convert coordinates from web
 // mercator int32 to normal lat-long double coordinates.
-inline util::geo::DPoint projectInt32WebMercToDoubleLatLng(
-    const util::geo::I32Point& p) {
-  return util::geo::webMercToLatLng<double>(
-      static_cast<double>(p.getX()) / PREC,
-      static_cast<double>(p.getY()) / PREC);
-};
+inline DPoint projectInt32WebMercToDoubleLatLng(const I32Point& p) {
+  return webMercToLatLng<double>(static_cast<double>(p.getX()) / PREC,
+                                 static_cast<double>(p.getY()) / PREC);
+}
 
 // Same as above, but for a bounding box.
-inline util::geo::DBox projectInt32WebMercToDoubleLatLng(
-    const util::geo::I32Box& box) {
+inline DBox projectInt32WebMercToDoubleLatLng(const I32Box& box) {
   return {projectInt32WebMercToDoubleLatLng(box.getLowerLeft()),
           projectInt32WebMercToDoubleLatLng(box.getUpperRight())};
-};
+}
 
 // Counts the number of geometries in a geometry collection.
 inline uint32_t countChildGeometries(const ParsedWkt& geom) {
@@ -657,7 +654,7 @@ struct MetricDistanceVisitor {
   // Delegate the actual distance computation to `pb_util`.
   CPP_template(typename T, typename U)(requires IsPairOfUtilGeoms<T, U>) double
   operator()(const T& a, const U& b) const {
-    return util::geo::webMercMeterDist<T, U>(a, b);
+    return webMercMeterDist<T, U>(a, b);
   }
 
   // Handle optional geometries that may be contained in a `ParseResult`.

src/util/LruCache.h

@@ -35,9 +35,9 @@ class LRUCache {
   // found. Otherwise, compute the value using `computeFunction` and store it in
   // the cache. If the cache is already at maximum capacity, evict the least
   // recently used element.
-  CPP_template(typename Func)(
+  CPP_template(typename Key, typename Func)(
       requires ad_utility::InvocableWithConvertibleReturnType<
-          Func, V, const K&>) const V& getOrCompute(const K& key,
+          Func, V, const K&>) const V& getOrCompute(Key&& key,
                                                     Func computeFunction) {
     auto it = cache_.find(key);
     if (it != cache_.end()) {
@@ -56,9 +56,10 @@ class LRUCache {
       lruKey = key;
     } else {
       // Push new element if not full
-      keys_.push_front(key);
+      keys_.push_front(K{key});
     }
-    auto result = cache_.try_emplace(key, computeFunction(key), keys_.begin());
+    auto result = cache_.try_emplace(
+        AD_FWD(key), computeFunction(keys_.front()), keys_.begin());
     AD_CORRECTNESS_CHECK(result.second);
     return result.first->second.first;
   }