fix: queryGraph selection for query and update
#2546
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`Store.query` and `Store.update` have a parameter `queryGraph`, which presumably specifies the initial [active graph](https://www.w3.org/TR/sparql11-query/#sparqlDataset). When SPARQL is evaluated against a `Dataset`, the initial active graph should be the default graph [[ref](https://www.w3.org/TR/sparql11-query/#sparqlAlgebraEval)], however, when calling `rdflib.graph.Graph.query` or `rdflib.graph.Graph.update`, they were passing `rdflib.graph.Dataset.identifier` as the value of `queryGraph` to `Store.query` and `Store.update`. The passed value would not have been the identifier of the default graph of the Dataset, and would instead have been an identifier that has really no meaning whatsoever (see <RDFLib#2537>). This change fixes the selection of the initial active graph by passing `ConjunctiveGraph.default_context.identifier` if the `Graph.query` or `Graph.update` method is called on a `ConjunctiveGraph` and `ConjunctiveGraph.default_union` is `False`. Related issues: - Fixes <RDFLib#2538>.
aucampia
force-pushed
the
aucampia/20230826T1203-fix-store-query
branch
from
92b4181
to
9b9686e
Compare
|
After this oxigraph/oxrdflib#28 should hopefully not be a thing. |
aucampia
added
the
ready to merge
aucampia
commented
Aug 27, 2023
aucampia
commented
Aug 27, 2023
aucampia
commented
Aug 27, 2023
|
The other option is to make |
Maybe starting with this PR is best, we can change to having |
Tpt
approved these changes
Aug 29, 2023
Thank you for having a look |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Store.queryandStore.updatehave a parameterqueryGraph, whichpresumably specifies the initial active
graph.
When SPARQL is evaluated against a
Dataset, the initial active graphshould be the default graph
[ref],
however, when calling
rdflib.graph.Graph.queryorrdflib.graph.Graph.update, they were passingrdflib.graph.Dataset.identifieras the value ofqueryGraphtoStore.queryandStore.update. The passed value would not have beenthe identifier of the default graph of the Dataset, and would instead
have been an identifier that has really no meaning whatsoever (see
#2537).
This change fixes the selection of the initial active graph by passing
ConjunctiveGraph.default_context.identifierif theGraph.queryorGraph.updatemethod is called on aConjunctiveGraphandConjunctiveGraph.default_unionisFalse.Related issues:
Graph.{query,update}should use the default graph as the value for thequeryGraphwhen callingStore.{query,update}#2538.Checklist
the same change.
so maintainers can fix minor issues and keep your PR up to date.