Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle scoping issues in SPARQL #2171

Closed
abrokenjester opened this issue May 5, 2020 · 0 comments · Fixed by #2172
Closed

Handle scoping issues in SPARQL #2171

abrokenjester opened this issue May 5, 2020 · 0 comments · Fixed by #2172
Assignees
Labels
🐞 bug issue is a bug
Milestone

Comments

@abrokenjester
Copy link
Contributor

Related to #1405 and #2119

Currently, the SPARQL parser and engine use a check whether something was parsed as a group graph pattern to determine if a variable scope change is needed (which influences the chosen join strategy).

This is a bit of a workaround approach and there are many exceptions. Filter conditions as well as named graph patterns are parsed as group patterns, but do not necessarily mean a scope change. In addition, the entire where clause is by definition a group pattern, but a values clause defined outside the where clause is intended to be scoped to that where-clause. We are continually bumping into other corner cases and adapting the workaround as we go.

To fix the issue more robustly, we should introduce a isScopeChange field in the SPARQL AST tree, which defaults to false. In the case of ASTGraphPatternGroups we can inspect the node parent to determine if the group is part of a named graph or a filter condition before returning true or false. The external values clause will need to be treated as a special case in the TupleExprBuilder (in fact it already is, we need to modify it slightly).

Fix and regression tests incoming.

@abrokenjester abrokenjester added the 🐞 bug issue is a bug label May 5, 2020
@abrokenjester abrokenjester added this to the 3.1.5 milestone May 5, 2020
@abrokenjester abrokenjester self-assigned this May 5, 2020
abrokenjester added a commit that referenced this issue May 5, 2020
- TupleExprBuilder uses this to build algebra tree with correct settings
- GraphPatternGroupable renamed to VariableScopeChange
- easier to distinguish corner cases at correct abstraction level
- added regression test for handling of values clause and named graphs
abrokenjester added a commit that referenced this issue May 5, 2020
* GH-2171 AST nodes now determine if node is scope change

- TupleExprBuilder uses this to build algebra tree with correct settings
- GraphPatternGroupable renamed to VariableScopeChange
- easier to distinguish corner cases at correct abstraction level
- added regression test for handling of values clause and named graphs

* bump CI

* GH-2171 use new interface / method for tracking scope changes in algebra
@abrokenjester abrokenjester reopened this May 5, 2020
abrokenjester added a commit that referenced this issue May 6, 2020
…change (#2178)

* GH-2171 group pattern as direct child of where clause is not a scope change

* GH-2171 adapted test expectations to deal with improved scope handling

* GH-2171 replaced use of setGraphPatternGroup with setVariableScopeChange
@abrokenjester abrokenjester modified the milestones: 3.1.5, 3.2.0, 3.2.1 May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant