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

GH-1405 fixed scope handling in join iterator #1616

Merged
merged 3 commits into from
Oct 22, 2019

Conversation

abrokenjester
Copy link
Contributor

GitHub issue resolved: #1405

Briefly describe the changes proposed in this PR:

  • use merge-join for group patterns to handle scope, with special case
    exception for FILTER (NOT) EXISTS

use merge-join for group patterns to handle scope, with special case
exception for FILTER (NOT) EXISTS
@abrokenjester
Copy link
Contributor Author

The fix seems to cause a regression in the federation sail, got two failing tests there.

@abrokenjester
Copy link
Contributor Author

@aschwarte10 is this something you might be able to shed some light on? I've got two failing tests in the (old) federation sail due a change I made in the JoinIterator algorithm, put I can't put my finger on why. Is there an implicit assumption somewhere in the federated evaluation strategy that left argument results automatically get injected into right argument evaluation?

@abrokenjester
Copy link
Contributor Author

abrokenjester commented Oct 20, 2019

This still has a number of failing compliance tests (these aren't run on PR verify) and will need more time. I think I may need to ensure we don't use merge-joining at all when the arg is a filter condition (not just when it's a filter not exists) which makes sense if you think about it: the filter needs injected variable bindings to evaluate its condition.

@abrokenjester abrokenjester removed the WIP label Oct 20, 2019
@abrokenjester
Copy link
Contributor Author

abrokenjester commented Oct 20, 2019

Verified locally that this now passes all compliance tests. Be advised that due to the way the tests are set up, you will need to run a full mvn clean install on the compliance modules, because otherwise the federated sparql tests use an old snapshot of the rdf4j-server and consequently fail.

Another point in favor of getting rid of using embedded rdf4j servers for (most of) our test suites, and just moving to a full mock/stub approach for this kind of integration testing.

@abrokenjester abrokenjester merged commit 91cb1cd into master Oct 22, 2019
@abrokenjester abrokenjester deleted the issues/GH-1405-var-scope branch October 22, 2019 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SPARQL: Variables from outside group are visible inside group
2 participants