afs commented on code in PR #2882:
URL: https://github.com/apache/jena/pull/2882#discussion_r1874382764
##########
jena-arq/src/main/java/org/apache/jena/sparql/engine/main/StageGeneratorGeneric.java:
##########
@@ -49,12 +53,29 @@ public QueryIterator execute(BasicPattern pattern,
QueryIterator input, Executio
return execute(pattern, reorder, input, execCxt) ;
}
+ protected static <T> T closeOnException(Supplier<T> compute, Closeable
closeAction) {
+ try {
+ T result = compute.get();
+ return result;
+ } catch (Exception e) {
+ closeAction.close();
+ e.addSuppressed(new RuntimeException("Exception during
construction of basic pattern stage execution."));
+ throw e;
+ }
+ }
+
+ /**
+ * Attempts to construct an iterator that executes the input against the
pattern.
+ * If the construction fails, such as due to {@link
QueryCancelledException}, then the exception is passed on
+ * and the input iterator is closed.
+ */
protected QueryIterator execute(BasicPattern pattern,
ReorderTransformation reorder,
QueryIterator input, ExecutionContext
execCxt) {
Explain.explain(pattern, execCxt.getContext()) ;
- if ( ! input.hasNext() )
- return input ;
+ QueryIterator originalInput = input;
+ if ( closeOnException(() -> !originalInput.hasNext(), originalInput) )
+ return originalInput ;
Review Comment:
The `input` should be closed in a cancel. If it is not then something is
missing elsewhere.
See the issue #2881 for discussion about inverting the overall design to
have explicit declaration and management of resources that have clear-up.
##########
jena-arq/src/main/java/org/apache/jena/sparql/engine/main/StageGeneratorGeneric.java:
##########
@@ -65,7 +86,7 @@ protected QueryIterator execute(BasicPattern pattern,
ReorderTransformation reor
QueryIterPeek peek = QueryIterPeek.create(input, execCxt) ;
// And now use this one
input = peek ;
- Binding b = peek.peek() ;
+ Binding b = closeOnException(peek::peek, peek) ;
Review Comment:
It looks to me like `QueryIterPeek` should be updated to handle
cancellations rather than putting execution costs into the main path.
--
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]
