On 02/08/16 17:01, Rob Vesse wrote:
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.
Returning (table unit) would be possible.
The main thing is as a way to declare the variables and how they are
used by the OpExt. The effective Op is not used for execution at all.
Andy
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
>
