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

[GitHub] [jena] sszuev opened a new issue, #1374: a convenient way to execute a query against graph in a dedicated context.

sszuev opened a new issue, #1374:
URL: https://github.com/apache/jena/issues/1374

   ### Version
   
   4.6.0
   
   ### Suggestion
   
   If we want to execute a query in isolated context with unique set of independent functions and p-functions, 
   we will encounter some difficulties:
   
   - the method `org.apache.jena.sparql.util.Context#copy` performs only shallow copying, not deep
   - there is no straightforward way to pass the context while constructing `org.apache.jena.query.QueryExecution`
   
   So we have to spend some time to find the proper way.
   I think this can be simplified by adding two new helper-methods:
   
   - `Context#copyWithFunctionRegistries(Context)` - the method to create a copy of `Context` with copied function and p-function registries
   - `QueryExecutionFactory#create(Query, Graph, Context)` - the method to create `QueryExceution` for a `Graph` with the given context
   
   
   ### Are you interested in contributing a solution yourself?
   
   Yes


-- 
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: issues-unsubscribe@jena.apache.org.apache.org

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


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


[GitHub] [jena] afs closed issue #1374: a convenient way to copy arq-context including registries.

Posted by GitBox <gi...@apache.org>.
afs closed issue #1374: a convenient way to copy arq-context including registries.
URL: https://github.com/apache/jena/issues/1374


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] afs commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1155537503

   ContextUtils makes sense.
   
   If it were each registry, I'd put the code there but to have a function that all three at once is better.


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] afs commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1153177536

   > But replacing `QueryExecutionFactory.create(query, data.getGraph(), context)` (from PR) with the expression `QueryExecution.create().query(query).model(data).context(context).build()` causes test error (where query is `String`, not `Query`).
   
   Visual inspection only but this looks odd. The PR new `create(query, graph, context)` uses `DatasetGraphWrapper` to apply the context. But I don't see this going to the `QueryExecDatasetBuilder`.  Dataset contexts are usually added (`Context.mergeCopy`).
   


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] afs commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1153150788

   Where does `Query` cache functions? Do you mean in "E_Function"?
   
   `QueryExecution.create(...).context(cxt)...` will use that and only that context.
   
   `QueryExecutionFactory` provides common cases - and it had grown into a bit too large. The new-style builders are better for detailed setup.


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] sszuev commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
sszuev commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1153891201

   > > But replacing `QueryExecutionFactory.create(query, data.getGraph(), context)` (from PR) with the expression `QueryExecution.create().query(query).model(data).context(context).build()` causes test error (where query is `String`, not `Query`).
   > 
   > Visual inspection only but this looks odd. The PR new `create(query, graph, context)` uses `DatasetGraphWrapper` to apply the context. But I don't see this going to the `QueryExecDatasetBuilder`. Dataset contexts are usually added to the context being built for the query (`Context.mergeCopy`), not a replacement. `DatasetGraphWrapper(,Context)` hides the wrapped dataset's context.
   
   I think, `QueryExecutionFactory.create(query, data.getGraph(), context)` can be removed from PR, and new bug for `QueryExecution.create().query(query).model(data).context(context).build()` with tests attached can be started.
   
   @afs  ?
   


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] sszuev commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
sszuev commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1153164302

   > Where does `Query` cache functions? Do you mean in "E_Function"?
   
   Yes. `org.apache.jena.sparql.expr.E_Function`
   
   > `QueryExecution.create(...).context(cxt)...` will use that and only that context.
    
   Ok, I see. 
   
   But replacing `QueryExecutionFactory.create(query, data.getGraph(), context)` (from PR) with the expression `QueryExecution.create().query(query).model(data).context(context).build()` causes test error (where query is `String`, not `Query`). 
   Here https://github.com/sszuev/jena/blob/main/jena-arq/src/test/java/org/apache/jena/query/TestQueryExecutionFactory.java#L89
   
   Just found this, didn't dig yet.
   
   > QueryExecutionFactory provides common cases - and it had grown into a bit too large. The new-style builders are better for detailed setup.
   
   Ok.
   
   


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] afs commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1153122216

   Hi @sszuev ,
   
   A copy of the context will share the original registries - the way to modify that is to modify the copy of the context. I'd put that code in the PropertyFunctionRegistry, not "teach" context about a specific setting - moie discussion for the PR - the copy operations look useful.
   
   The best control over a query setup is via the new (4.3.x) builders which allow a change of context: `QueryExec` for graph level, `QueryExecution.create` for model level (the associated builder also some legacy compatibility about setting timeouts).
   
   ```
           Graph graph = GraphFactory.createDefaultGraph();
   
           PropertyFunctionRegistry pfr = new PropertyFunctionRegistry();
           PropertyFunction pf = new version();
           pfr.put("urn:test:version", (x)->pf);
   
           Context cxt = ARQ.getContext().copy();
           PropertyFunctionRegistry.set(cxt, pfr);
   
           String qs = """
                   SELECT * { ?x <urn:test:version> ?y }
                   """;
           RowSet rowSet = QueryExec.graph(graph)
                   .context(cxt)
                   .query(qs)
                   .select();
           RowSetOps.out(rowSet);
   ```
   Unrelated: there is a problem with some system provided property functions. There is a hardwired translation in the code.
   ```
           Graph graph = GraphFactory.createDefaultGraph();
           Context cxt = ARQ.getContext().copy();    
           PropertyFunctionRegistry pfr = new PropertyFunctionRegistry();
           PropertyFunctionRegistry.set(cxt, pfr);
           String qs = """
                   PREFIX afn:     <http://jena.apache.org/ARQ/function#>
                   PREFIX apf:     <http://jena.apache.org/ARQ/property#>
                   SELECT * { ?x apf:version ?y }
                   """;
           RowSet rowSet = QueryExec.graph(graph)
                   .context(cxt)
                   .query(qs)
                   .select();
           RowSetOps.out(rowSet);
   ```
   doesn't work - `apf:version` is still a property function because `afn` and `apf` are hardwired in the registry code to map to "java:" URI which then will always happen. That's a bug.
   


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] afs commented on issue #1374: a convenient way to copy arq-context including registries.

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1179097575

   Closed by #1375.


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] sszuev commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
sszuev commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1153134326

   There is also a problem (or feature) with the `org.apache.jena.query.Query` itself: it caches functions in thread-unsafe manner. 
   This was unexpected behavior for me, and for a while I didn't realize that my code was working incorrectly.
   
   This one is also a reason (although weak) to have obvious way to create ready to use `QueryExecution` from scratch with specified context, from some well-known place.  
   But yes, we always can do the same things in our client-projects instead of patching Jena.


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] afs commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
afs commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1154094313

   Seems reasonable.


-- 
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: issues-unsubscribe@jena.apache.org

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


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


[GitHub] [jena] sszuev commented on issue #1374: a convenient way to execute a query against graph in a dedicated context.

Posted by GitBox <gi...@apache.org>.
sszuev commented on issue #1374:
URL: https://github.com/apache/jena/issues/1374#issuecomment-1154122367

   > A copy of the context will share the original registries - the way to modify that is to modify the copy of the context. I'd put that code in the PropertyFunctionRegistry, not "teach" context about a specific setting - moie discussion for the PR - the copy operations look usefu
   
   What is appropriate place to such functionality ?
   `org.apache.jena.sparql.util.Contexts` ?
   `org.apache.jena.sparql.util;ContextUtils` ?
   
   In the PR, this is static helper, instance of `Context` itself does not refer to any particular symbol.
   There are already static helpers like `fromDataset(DatasetGraph)`.
   I don't quite understand why `import org.apache.jena.sparql.core.DatasetGraph` is ok, but import `org.apache.jena.sparql.function.FunctionRegistry` is not? 
   If we move it to some other place it would be harder to find it.
   
   


-- 
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: issues-unsubscribe@jena.apache.org

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


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