You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2022/06/19 15:19:32 UTC

[GitHub] [jena] Aklakan opened a new pull request, #1392: GH-1391 Support for context-local QueryEngineRegistries

Aklakan opened a new pull request, #1392:
URL: https://github.com/apache/jena/pull/1392

   Pull request Description:
   While the function, property function and service executor registries can be set per context, this is not possible with QueryEngineRegistry. So far engines thus have to be registered globally which can be brittle when a specific engine is wanted to evaluate a query.
   
   The default query engine always calls DatasetGraph.listGraphNodes for a pattern `GRAPH ?g { ?s ?p ?o }` - even if ?s, ?p or ?o were concrete and an index could be used instead.
   
   The main use case is thus to allow for specifically using the quad-based engine (QueryEngineQuad + OpExecutorQuad) so that evaluation can be handled in the DatasetGraphs.find() method.
   With a single global registry it is hard to guarantee that the desired engine will be chosen in the presence of other query engine factory plugins.
   
   This PR makes it possible to override the global QueryEngineRegistry with a context-local one.
   
   A test case for this use case is provided.
   
   
   ----
   
    - [x] Tests are included.
    - [x] Commits have been squashed to remove intermediate development commit messages.
    - [x] Key commit messages start with the issue number (GH-xxxx or JENA-xxxx)
   
   By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the [Contributor's Agreement](https://www.apache.org/licenses/contributor-agreements.html).
   
   ----
   
   See the [Apache Jena "Contributing" guide](https://github.com/apache/jena/blob/main/CONTRIBUTING.md).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on pull request #1392: GH-1391 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
Aklakan commented on PR #1392:
URL: https://github.com/apache/jena/pull/1392#issuecomment-1161393574

   - I just realized the method `getOrDefault` should be renamed to `chooseRegistry` - the same concept already exists with e.g.
   `public static PropertyFunctionRegistry chooseRegistry(Context context)` (will update later today)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] Aklakan commented on pull request #1392: GH-1407 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
Aklakan commented on PR #1392:
URL: https://github.com/apache/jena/pull/1392#issuecomment-1173693871

   > Instead of using `chooseRegistry` in `QueryExecutionFactory`, `QueryExecDatasetBuilder` and `QueryEngineFactoryWrapper`, I think we can give a simpler view by replacing `QueryEngineRegistry.get().find` with `QueryEngineRegistry.findFactory` (currently, the definition of `findFactory`) and changing `findFactory` to be `chooseRegistry(context).find(...)`.
   
   Good point, makes it simpler.
    
   > `QueryEngineFactoryWrapper` can be left alone. `QueryExecutionFactory` and `QueryExecDatasetBuilder` needs teh change `get().find` to `findFactory`.
   
   Done and squashed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1392: GH-1391 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
afs commented on PR #1392:
URL: https://github.com/apache/jena/pull/1392#issuecomment-1165734203

   The existing way to have a different query engine is to add a dataset-graph wrapper, and add an factory to the lookup chain.
   That is very old and not really that useful.
   
   Probably, `DatasetGraph` should have an `exec(op, context)` but that's not a change to make lightly. So this PR is a useful addition and exploration of the ideal design.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs closed pull request #1392: GH-1391 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
afs closed pull request #1392: GH-1391 Support for context-local QueryEngineRegistries
URL: https://github.com/apache/jena/pull/1392


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1392: GH-1391 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
afs commented on PR #1392:
URL: https://github.com/apache/jena/pull/1392#issuecomment-1165726321

   (sorry - misclick)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs merged pull request #1392: GH-1407 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
afs merged PR #1392:
URL: https://github.com/apache/jena/pull/1392


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1392: GH-1391 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
afs commented on PR #1392:
URL: https://github.com/apache/jena/pull/1392#issuecomment-1165662761

   GH-1391 isn't the issue for this PR nor is it this PR.
   
   Could you create an issue (or press send on a draft!) and label the commit please?
   
   All this makes looking back over git history in several months/years time easier.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org


[GitHub] [jena] afs commented on pull request #1392: GH-1391 Support for context-local QueryEngineRegistries

Posted by GitBox <gi...@apache.org>.
afs commented on PR #1392:
URL: https://github.com/apache/jena/pull/1392#issuecomment-1165732517

   Instead of using `chooseRegistry` in `QueryExecutionFactory`, `QueryExecDatasetBuilder` and `QueryEngineFactoryWrapper`, I think we can give a simpler view by replacing `QueryEngineRegistry.get().find` with `QueryEngineRegistry.findFactory` (currently, the definition of `findFactory`) and changing `findFactory` to be `chooseRegistry(context).find(...)`.
   
   `QueryEngineFactoryWrapper` can be left alone.
   `QueryExecutionFactory` and `QueryExecDatasetBuilder` needs teh change `get().find` to `findFactory`.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org