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 under normal conditions be 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]

Reply via email to