Located triples with graph information #1572
Conversation
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1572 +/- ##
==========================================
+ Coverage 88.98% 88.99% +0.01%
==========================================
Files 368 368
Lines 33825 33853 +28
Branches 3827 3827
==========================================
+ Hits 30098 30127 +29
Misses 2473 2473
+ Partials 1254 1253 -1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
…cated-triple-graph-id # Conflicts: # src/index/LocatedTriples.h
Signed-off-by: Johannes Kalmbach <[email protected]>
…cated-triple-graph-id
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
…nto located-triple-graph-id
src/index/LocatedTriples.cpp
Outdated
| // is `2` and `includeGraphColumn` is `true`, the function returns | ||
| // `std::tie(row[0], row[1], row[2])`. | ||
| template <size_t numIndexColumns, bool includeGraphColumn> | ||
| requires(numIndexColumns >= 1 && numIndexColumns <= IdTriple<0>::NumCols) |
There was a problem hiding this comment.
Shouldn't the second condition somehow depend on row and not on IdTriple<0>::NumCols?
There was a problem hiding this comment.
It should be a hardcoded 3 as it was before, since the graph column is now handled explicitly.
Signed-off-by: Johannes Kalmbach <[email protected]>
# Conflicts: # test/CompressedRelationsTest.cpp # test/IndexMetaDataTest.cpp
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
# Conflicts: # src/index/IndexFormatVersion.h # test/CompressedRelationsTest.cpp # test/IndexMetaDataTest.cpp
| auto merged = locatedTriplesPerBlock.mergeTriples(1, block, 2, true); | ||
| EXPECT_THAT(merged, testing::ElementsAreArray(resultExpected)); |
There was a problem hiding this comment.
How about also testing here (and in the following) the call, where the last argument is false. A simple helper lambda that strips the last column from resultExpected should do the job (and it can be reused for the following cases).
There was a problem hiding this comment.
If I understand you correctly, then this doesn't work. The result changes as soon as we drop the graph column, as triples that don't match because of the GRAPH now suddenly match etc.
We already (preexisting, so not shown in the diff) have exhaustive tests for the case where the last argument is false.
There was a problem hiding this comment.
You are right, thanks for the clarification
Signed-off-by: Johannes Kalmbach <[email protected]>
Signed-off-by: Johannes Kalmbach <[email protected]>
Conformance check passed ✅No test result changes. |
hannahbast
changed the title
hannahbast
changed the title
|
Triples from a SPARQL 1.1 Update operation also belong to a graph, but so far, our located triples did not contain graph information. Now each located triple holds an array of four
Ids, where the first three denote the subject, predicate, and object (in the order of the permutation in which the triple is located), and the fourth denotes the graph.