GH-4859 Add instead of remove QueryRoot for explaining optimized Sail queries #4862
Conversation
| @@ -148,4 +152,21 @@ public void testPrepareBooleanQuery_bypassed() { | |||
| verify(sailConnection).evaluate(eq(expr), any(), any(), anyBoolean()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testExplainQuery() { | |||
There was a problem hiding this comment.
I struggled a bit with the use of Mockito here.
| Explanation explanation = mock(Explanation.class); | ||
|
|
||
| when(sailConnection.prepareQuery(any(), any(), any(), any())).thenReturn(Optional.of(expr)); | ||
| when(sailConnection.explain(any(), any(TupleExpr.class), any(), any(), anyBoolean(), anyInt())) |
There was a problem hiding this comment.
I wanted to write
when(sailConnection.explain(any(), eq(expr), any(), any(), anyBoolean(), anyInt()))here. Note the eq(expr).
| TupleQuery query = subject.prepareTupleQuery("SELECT * WHERE { ?s ?p ?o }"); | ||
| assertThat(query.explain(Explanation.Level.Unoptimized)).isEqualTo(explanation); | ||
|
|
||
| verify(sailConnection).explain(eq(Explanation.Level.Unoptimized), any(QueryRoot.class), any(), any(), |
There was a problem hiding this comment.
And here I wanted to use
verify(sailConnection).explain(eq(Explanation.Level.Unoptimized), eq(new QueryRoot(expr)), any(), any(),
anyBoolean(), anyInt());but that didn't work.
There was a problem hiding this comment.
Should be fine as it is.
|
I see that you've targeted develop. That will make this part of 5.0.0 (or the next milestone build). If you need it sooner you can branch & target the |
…ptimized Sail queries
frensjan
force-pushed
the
GH-4859-explain-with-query-root
branch
from
510f14a to
cc3782c
Compare
|
I've changed the base to target main instead. Thanks @hmottestad. As an aside, is there a rough idea on when 5.0 will land? |
|
Had to close and reopen this to trigger the CI. |
I'm still working on integrating support for JSON-LD 1.1. Once that is complete I'll publish the final milestone build and allow for a few weeks to let users test it out and make sure things are functional. I'll have a bug fix release out in a week or so I reckon, it'll include your fix here. |
Cool! Can't wait to get al the deprecation warnings out of our code base 😉 |
GitHub issue resolved: #4859
Briefly describe the changes proposed in this PR:
This change adds instead of removes
QueryRootinSailQuery#explainto support explanations of queries of which the rootTupleExpris replaced.PR Author Checklist (see the contributor guidelines for more details):
mvn process-resourcesto format from the command line)