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

JENA-1831: Fix SDB transform issue with using UNION in restrincting a query #681

Merged
merged 2 commits into from Jan 30, 2020

Conversation

grahamtriggs
Copy link
Contributor

Encountered a case using a UNION in a query to select e.g. objects where type is asserted as X, Y or Z (but not A, B, C, etc.)

Pull request prevents the transformer from copying restrictions out of a UNION so that the algebra is correct in this case.

Includes a test for a query that exhibits the problem. All previously existing transformer tests are unmodified and continue to pass, as do all the other unit tests in SDB.

https://issues.apache.org/jira/browse/JENA-1831

@afs afs changed the title Fix SDB transform issue with using UNION in restrincting a query JENA-1831: Fix SDB transform issue with using UNION in restrincting a query Jan 28, 2020
@afs
Copy link
Member

afs commented Jan 28, 2020

I'm thinking that anyone looking back on this in the future may want to understand it and the test case is the illustration.

Is there a simpler test case? (it does not need to be CONSTRUCT for example). Are all the WHERE clause parts necessary?

@grahamtriggs
Copy link
Contributor Author

In theory, it might be possible to cut the WHERE clause down to the following:

WHERE {
?publication a ?type .
OPTIONAL { ?publication http://localhost/access ?access . }
{
?publication a http://localhost/Report .
}
UNION
{
?publication a http://localhost/AcademicArticle .
}
}

Without the OPTIONAL element, it makes it similar to the existing UNION queries, and it transforms correctly.

It's only with the combination of OPTIONAL and UNION in this form that generates incorrectly transformed algebra without the patch applied.

@afs
Copy link
Member

afs commented Jan 29, 2020

I'll leave the decision to you. After all, the next person to read it may well be future-you!

@grahamtriggs
Copy link
Contributor Author

I've simplified the test slightly by removing a couple of unnecessary patterns, and cleaned it up by removing the escaped tabs in the query. Also added a comment to say why it is necessary.

Test has been checked to ensure that it fails with the previous version of the transformer, so is a valid test of the issue that is being fixed.

@afs afs merged commit 7571c53 into apache:master Jan 30, 2020
@afs
Copy link
Member

afs commented Jan 30, 2020

@grahamtriggs - thank you!

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.

None yet

2 participants