Fixes GH-2013 by traversing parents until sourceString is found #2014
Conversation
There was a problem hiding this comment.
Good find and fix, thanks @damyan-ognyanov ! One minor editorial remark, other than that LGTM.
...r/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/ast/ASTServiceGraphPattern.java
Outdated
Show resolved
Hide resolved
| sourceString = ((ASTUpdateSequence) theParent).getSourceString(); | ||
| } | ||
| } | ||
| return sourceString; |
There was a problem hiding this comment.
I was wondering if maybe there could be a null check here, so that any exception is thrown when looking for the source string instead of when using the resulting string. Maybe return Objects.requireNonNull(sourceString);
Unless of course there is a case where returning null from getSourceString() is acceptable.
There was a problem hiding this comment.
Thought of that for a while, but decided that if we did not found the sourceString at this point, something else was really messed up, so throwing a custom exception early, would not help much here since we do not have a resonable alternative to continue processing update further - a NPE will still be thrown in the code that uses it ...
|
Thanks @damyan-ognyanov . One more thing: could you please squash your commits? Oh and also - we have revised our branch naming convention a little while back. For future fixes, could you please use the new convention? See https://rdf4j.org/documentation/developer/workflow/#workflow: it's basically Sorry for all the red tape requests - I promise this should be all of it :) |
…test case Signed-off-by: damyan.ognyanov <damyan.ognyanov@ontotext.com>
damyan-ognyanov
force-pushed
the
issues/#2013-NPE-service-multiple-update-expressions
branch
from
bb8c475
to
4023a7a
Compare
|
@jeenbroekstra squashed commits but renaming it to follow the naming convention is a bit tricky since we already have this pull request refering to it... |
|
That's all good, thanks, the branch naming thing was more for future reference. |
abrokenjester
deleted the
issues/#2013-NPE-service-multiple-update-expressions
branch
Fixes GH-2013 by traversing parents until sourceString is found
Signed-off-by: damyan.ognyanov damyan.ognyanov@ontotext.com
GitHub issue resolved: #2013
ASTServiceGraphPatternthat traverse the node parents untl asourceStringis found