Aklakan commented on code in PR #2882:
URL: https://github.com/apache/jena/pull/2882#discussion_r1874414501
##########
jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterPeek.java:
##########
@@ -25,27 +25,34 @@
public class QueryIterPeek extends QueryIter1
{
- private Binding binding = null ;
+ private Binding binding = null ;
private boolean closed = false ;
-
+
public static QueryIterPeek create(QueryIterator iterator,
ExecutionContext cxt)
{
if ( iterator instanceof QueryIterPeek)
return (QueryIterPeek)iterator ;
return new QueryIterPeek(iterator, cxt) ;
}
-
+
private QueryIterPeek(QueryIterator iterator, ExecutionContext cxt)
{
super(iterator, cxt) ;
}
/** Returns the next binding without moving on. Returns "null" for no
such element. */
- public Binding peek()
+ public Binding peek()
{
if ( closed ) return null ;
- if ( ! hasNextBinding() )
- return null ;
+ try {
+ if ( ! hasNextBinding() )
+ return null ;
+ } catch (Exception e) {
Review Comment:
Perhaps having the try-catch block in `StageGeneratorGeneric.execute` is
slightly better. Essentially, a method that returns a QueryIterator should
close its own already allocated resources early in case of a failure. It feels
not totally clean having peek() deal with an exception but not hasNextBinding /
moveToNext.
The thing is, that even if an iterator's hasNextBinding / moveToNext raise
an exception, then this iterator will usually connected to the 'root' iterator.
The query execution will catch an exception and close the root iterator which
will propagate to its children and only then a check for dangling resources is
done.
The problem is that StageGeneratorGeneric creates an QueryIterPeek, then
calls peek which fails, and then the already registered iterator is left
dangling - so its not really an issue of peek().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]