SPARQL 1.1: variable introduced by GRAPH is not affecting MINUS disjointness - #228
Merged
Merged
Conversation
rubensworks
approved these changes
Oct 3, 2025
afs
approved these changes
Oct 5, 2025
rubensworks
added a commit
to rubensworks/SPARQLAlgebra.js
that referenced
this pull request
Oct 15, 2025
Since our graph translation is not really part of the spec, there is a MINUS edge case we need to consider with GRAPH ?g. If left and right of MINUS have disjoint variables, the whole left solution sequence must be kept. If GRAPH is defined outside of an operator (e.g. MINUS), then the spec says that evaluation of the operators must be done as union over the evaluation of that operator within each graph separately, and that the variable of ?g must only be bound **after** that evaluation. As such, MINUS will not be aware of this variable ?g, and the disjoint case will apply. This code adds metadata to the operation so that engines can special-case this. See w3c/rdf-tests#228
joachimvh
pushed a commit
to rubensworks/SPARQLAlgebra.js
that referenced
this pull request
Oct 15, 2025
Since our graph translation is not really part of the spec, there is a MINUS edge case we need to consider with GRAPH ?g. If left and right of MINUS have disjoint variables, the whole left solution sequence must be kept. If GRAPH is defined outside of an operator (e.g. MINUS), then the spec says that evaluation of the operators must be done as union over the evaluation of that operator within each graph separately, and that the variable of ?g must only be bound **after** that evaluation. As such, MINUS will not be aware of this variable ?g, and the disjoint case will apply. This code adds metadata to the operation so that engines can special-case this. See w3c/rdf-tests#228
joachimvh
pushed a commit
to joachimvh/SPARQLAlgebra.js
that referenced
this pull request
Oct 15, 2025
Since our graph translation is not really part of the spec, there is a MINUS edge case we need to consider with GRAPH ?g. If left and right of MINUS have disjoint variables, the whole left solution sequence must be kept. If GRAPH is defined outside of an operator (e.g. MINUS), then the spec says that evaluation of the operators must be done as union over the evaluation of that operator within each graph separately, and that the variable of ?g must only be bound **after** that evaluation. As such, MINUS will not be aware of this variable ?g, and the disjoint case will apply. This code adds metadata to the operation so that engines can special-case this. See w3c/rdf-tests#228
rubensworks
added a commit
to comunica/comunica
that referenced
this pull request
Oct 15, 2025
Since our algebraic graph translation is not really part of the spec, there is a MINUS edge case we need to consider with GRAPH ?g. If left and right of MINUS have disjoint variables, the whole left solution sequence must be kept. If GRAPH is defined outside of an operator (e.g. MINUS), then the spec says that evaluation of the operators must be done as union over the evaluation of that operator within each graph separately, and that the variable of ?g must only be bound **after** that evaluation. As such, MINUS will not be aware of this variable ?g, and the disjoint case will apply. This is fixed by adding metadata to the operation so that engines can special-case this. See w3c/rdf-tests#228
rubensworks
added a commit
to comunica/comunica
that referenced
this pull request
Oct 15, 2025
Since our algebraic graph translation is not really part of the spec, there is a MINUS edge case we need to consider with GRAPH ?g. If left and right of MINUS have disjoint variables, the whole left solution sequence must be kept. If GRAPH is defined outside of an operator (e.g. MINUS), then the spec says that evaluation of the operators must be done as union over the evaluation of that operator within each graph separately, and that the variable of ?g must only be bound **after** that evaluation. As such, MINUS will not be aware of this variable ?g, and the disjoint case will apply. This is fixed by adding metadata to the operation so that engines can special-case this. See w3c/rdf-tests#228 joachimvh/SPARQLAlgebra.js#128
rubensworks
added a commit
to comunica/comunica
that referenced
this pull request
Oct 15, 2025
Since our algebraic graph translation is not really part of the spec, there is a MINUS edge case we need to consider with GRAPH ?g. If left and right of MINUS have disjoint variables, the whole left solution sequence must be kept. If GRAPH is defined outside of an operator (e.g. MINUS), then the spec says that evaluation of the operators must be done as union over the evaluation of that operator within each graph separately, and that the variable of ?g must only be bound **after** that evaluation. As such, MINUS will not be aware of this variable ?g, and the disjoint case will apply. This is fixed by adding metadata to the operation so that engines can special-case this. See w3c/rdf-tests#228 joachimvh/SPARQLAlgebra.js#128
This file contains hidden or 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
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.
variable scoping is already tested but it seems to me this case is easy to get wrong