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

Fix: query over empty optional aggregate; added test and fix for issue #2229 #2448

Merged
merged 3 commits into from Jun 15, 2023

Conversation

WhiteGobo
Copy link
Contributor

@WhiteGobo WhiteGobo commented Jun 14, 2023

testing counting of optional nodes. Zero optional nodes may throw a NotBoundError
Added fix for NotBoundError, for this test.

Summary of changes

As stated in issue 2229 a sparql query fails, if you count over an aggregate of optional values and zero values where found.
This patch catches the resulting NotBoundError and enables with that the count over the empty value-list. Also added the reason for the catch as comment.
Also added a test, that makes such a query.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Checked that all tests and type checking passes.
  • If the change adds new features or changes the RDFLib public API:
  • If the change has a potential impact on users of this project:
    • Added or updated tests that fail without the change.
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

testing counting of optional nodes. Zero optional nodes may throw a
NotBoundError
Added fix for NotBoundError, for this test.
@aucampia aucampia self-assigned this Jun 15, 2023
@aucampia
Copy link
Member

pre-commit.ci autofix

@coveralls
Copy link

coveralls commented Jun 15, 2023

Coverage Status

coverage: 90.898% (+0.02%) from 90.882% when pulling d173c00 on WhiteGobo:fix_aggregate_raises_exception_2229 into 1e5f56b on RDFLib:main.

Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @WhiteGobo, this looks good to me. I changed the test to check the whole result though, not just one row, and I confirmed this with Jena's SPARQL CLI.

@aucampia aucampia merged commit 46ff6cf into RDFLib:main Jun 15, 2023
22 checks passed
@WhiteGobo WhiteGobo deleted the fix_aggregate_raises_exception_2229 branch July 4, 2023 06:28
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.

Serializing SPARQL Query Results with Aggregates over Variables from Optional Graph Pattern
3 participants