Potentially yes However this was the only source of errors when I upgraded our code to the latest builds. I will try and concoct some additional test cases against our code to see if I can hit any of those other places. We don’t use join classification in our optimiser so we are unlikely to ever hit that case. We do actually use the variable finder and once this case was fixed that did not elicit any further issues.
There is a general question here about the use of OpExt, if the assumption is that it simply abstracts some specialism of an algebra then we will fail that assumption. We use it to add additional features to the query language Beyond what vanilla ARQ it Is able to support and so there is no effective operator. Rob On 02/08/2016 16:19, "Andy Seaborne" <[email protected]> wrote: Rob, Several places in the code access effectiveOp during optimization depend and on there being an equivalent non-null op (e.g. variable finding and classification). Aren't they vulnerable? Andy On 02/08/16 11:25, [email protected] wrote: > Repository: jena > Updated Branches: > refs/heads/master 61368dfa0 -> c21db9408 > > > Avoid possible NPE in OpVisitor > > The default for visit(OpExt) assumed an effectiveOp() is always > available which may not be the case and can cause a NPE. Check that the > effectiveOp() is non-null and only visits in that case. > > > Project: http://git-wip-us.apache.org/repos/asf/jena/repo > Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/c21db940 > Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/c21db940 > Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/c21db940 > > Branch: refs/heads/master > Commit: c21db9408a231fbb98173061d048931a8e71dac4 > Parents: 61368df > Author: Rob Vesse <[email protected]> > Authored: Tue Aug 2 11:24:26 2016 +0100 > Committer: Rob Vesse <[email protected]> > Committed: Tue Aug 2 11:24:26 2016 +0100 > > ---------------------------------------------------------------------- > .../src/main/java/org/apache/jena/sparql/algebra/OpVisitor.java | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/jena/blob/c21db940/jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpVisitor.java > ---------------------------------------------------------------------- > diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpVisitor.java b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpVisitor.java > index f947eca..ef3e952 100644 > --- a/jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpVisitor.java > +++ b/jena-arq/src/main/java/org/apache/jena/sparql/algebra/OpVisitor.java > @@ -57,7 +57,9 @@ public interface OpVisitor > public void visit(OpDisjunction opDisjunction) ; > > public default void visit(OpExt opExt) { > - opExt.effectiveOp().visit(this); > + Op effective = opExt.effectiveOp(); > + if (effective != null) > + effective.visit(this); > } > > // OpModifier >
