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-4997: Enable SparqlBuilder to create VALUES clauses #5002

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

fkleedorfer
Copy link
Contributor

@fkleedorfer fkleedorfer commented Jun 1, 2024

GitHub issue resolved: #4997

Briefly describe the changes proposed in this PR:

Adds a builder for VALUES clauses and integrates them with GraphPattern and Select


PR Author Checklist (see the contributor guidelines for more details):

  • my pull request is self-contained
  • I've added tests for the changes I made
  • I've applied code formatting (you can use mvn process-resources to format from the command line)
  • I've squashed my commits where necessary
  • every commit message starts with the issue number (GH-xxxx) followed by a meaningful description of the change

@fkleedorfer fkleedorfer force-pushed the GH-4997 branch 3 times, most recently from 5bbbf37 to cd2b417 Compare June 1, 2024 21:08
@fkleedorfer
Copy link
Contributor Author

Happy to rebase that onto develop and change the target branch to develop, if needed.

@hmottestad
Copy link
Contributor

Happy to rebase that onto develop and change the target branch to develop, if needed.

Yeah, this needs to target the develop branch.

Btw. Have you considered supporting VALUES inside UNION clauses? It's a common scenario for getting better performance since the scoping of the UNION means that any VALUES clauses outside the UNION clause will be evaluated after the UNION clauses. Duplicating the VALUES clause inside each branch of the UNION clause fixes the scoping issue so the VALUES clause can be used when executing the joins inside each UNION clause.

@fkleedorfer
Copy link
Contributor Author

You can add it to any GraphPattern, so you can also add it to a Union pattern

@fkleedorfer
Copy link
Contributor Author

fkleedorfer commented Jun 8, 2024

... But we might be able to make that more explicit if you like
EDIT: looked into that - there does not seem to be a good way to make it more explicit. Just add the values clause to your graph pattern.

It is generally a weakness of the sparqlbuilder package that you have to know how to produce the bits you need. The API has little 'fluency'. This here is a case in point.

@fkleedorfer fkleedorfer force-pushed the GH-4997 branch 2 times, most recently from 2f58b72 to d58a6c0 Compare June 9, 2024 15:10
@fkleedorfer fkleedorfer changed the base branch from main to develop June 9, 2024 15:10
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.

VALUES for SparqlBuilder
2 participants