GH-2883: Fix for an NPE in OpAsQuery. #2884
Conversation
...n/java/org/apache/jena/sparql/syntax/syntaxtransform/ExprTransformApplyElementTransform.java
Outdated
Show resolved
Hide resolved
| @@ -69,7 +69,7 @@ public static Query transform(Query query, ElementTransform transform, ExprTrans | |||
| Query q2 = QueryTransformOps.shallowCopy(query); | |||
| // Mutate the q2 structures which are already allocated and no other code can access yet. | |||
|
|
|||
| mutateByQueryType(q2, transform, exprTransform); | |||
| mutateByQueryType(q2, exprTransform); | |||
There was a problem hiding this comment.
Removed unused argument.
Aklakan
force-pushed
the
2024-12-08_op-as-query-fix
branch
2 times, most recently
from
0a6a054 to
994ace2
Compare
| @@ -482,6 +482,21 @@ public void testMinus02() { | |||
| "SELECT * { GRAPH ?g { { ?x ?y ?z FILTER(EXISTS { ?s ?p ?o }) } ?x ?y ?z } }", | |||
| syntaxARQ); } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
These EXISTS tests are about the ARQ extensions.
IIRC It's not about EXISTS, it's about subquery select expressions.
If that's right please use this test (insert at around L463):
@Test
public void testSubQuery01() {
test_roundTripQuery("SELECT ?z { SELECT ?x { } }");
}The outer SELECT ?z stops the outer SELECT getting removed because SELECT * is a no-op.
There was a problem hiding this comment.
Your query does not fail for me. See my note on the PR discussion.
Aklakan
force-pushed
the
2024-12-08_op-as-query-fix
branch
from
994ace2 to
e18ab07
Compare
| @@ -329,6 +329,11 @@ public void testSubQuery3() { | |||
| test_roundTripQuery(query) ; | |||
| } | |||
|
|
|||
| @Test | |||
| public void testSubQuery4() { | |||
There was a problem hiding this comment.
I added this test case for completeness - but it did not fail.
|
My change is in here: public Expr transform(ExprFunctionOp funcOp, ExprList args, Op opArg)
{
Element el2 = ElementTransformer.transform(funcOp.getElement(), transform, this);
}
I reduced the query to this: SELECT ?x {
SELECT ?x { # <-- NPE disappears when removing this sub select.
?x a ?y
FILTER EXISTS { SELECT * { ?y a ?z } }
}
}I silghtly updated the test cases. |
|
I converted to draft for now because it seems there are some more issues related to how ExprFunctionOp are handled - I added another example query that breaks to #2883 (comment) |
Aklakan
force-pushed
the
2024-12-08_op-as-query-fix
branch
3 times, most recently
from
109d372 to
3f660cd
Compare
|
I updated the code that handles ExprFunctionOp that when the Element is absent then it will be derived from the algebra. I also added additional test cases. |
Aklakan
force-pushed
the
2024-12-08_op-as-query-fix
branch
2 times, most recently
from
077c539 to
10a1360
Compare
Aklakan
force-pushed
the
2024-12-08_op-as-query-fix
branch
from
10a1360 to
261af51
Compare
| // If the element is null then an attempt is made to obtain it from the algebra. | ||
| // A given element takes precedence over the algebra. | ||
| Element el1 = funcOp.getElement(); | ||
| if (el1 == null) { | ||
| Op op = funcOp.getGraphPattern(); | ||
| el1 = op == null ? null : OpAsQuery.asElement(op); | ||
| } | ||
| // ElementTransformer will warn should it happen that null is passed to it. | ||
| Element el2 = ElementTransformer.transform(el1, transform, this); |
There was a problem hiding this comment.
This is now the updated actual fix - together with the introduction of OpAsQuery.asElement.
GitHub issue resolved #2883
Pull request Description: Changed a line to pass on an ExprTransform rather than null.
Notes for possible further improvements outside the scope of this PR:
ElementTransformerneeds to create an identity ExprTransform rather than passing null into the machinery.QueryTransformOps.transformVarExprListin my IDE warns that thechangedflag is not used. Perhaps this needlessly creates clones of queries w.r.t. identity transforms.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.
See the Apache Jena "Contributing" guide.