[
https://issues.apache.org/jira/browse/JENA-1215?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15396269#comment-15396269
]
Andy Seaborne edited comment on JENA-1215 at 7/28/16 3:34 PM:
--------------------------------------------------------------
See the nearby discussion {{AutoClosable}} for {{StmtIterators}} in
https://github.com/apache/jena/pull/133.
Adding {{AutoCloseable}} is a change that affects many users. The pattern is
that {{QueryExecution}} is closeable - adding that the inner {{ResultSet}}
should be as well is a burden. The doubly nested try-resource or doubled up
single try-resource is a change in preferred style.
Maybe if this were a green field API maybe it would be the way to go.
We need to avoid making work in user support. Even warnings appearing is a
cost. Such changes must be undertaken carefully.
Note that passing {{ResultSets}} around after the {{QueryExecution}} is closed
is not supported. The app needs to materialize the results and that
{{ResultSet}} is not attached to the {{QueryExecution}}.
When transactions are involved, the result set need materializing as well.
Adding {{Closeable}} to {{ResultSet}} is more practical but it encourages bad
practice. Teh {{QueryExecution}} is the unit to close.
There ways to get the are alternatives:
1. Return a {{QueryExecution}} object, not a {{ResultSet}}.
2. Return a pair of {{(QueryExecution, ResultSet}}. Not java's strong suit.
3. Extend the capabilities of a {{ResultSet}} to create a ResultSetCloseable:
{noformat}
public class ResultSetCloseable extends ResultSetWrapper implements Closeable {
private QueryExecution qexec ;
ResultSetCloseable(ResultSet rs, QueryExecution qexec) {
super(rs) ;
this.qexec = qexec ;
}
@Override
public void close() {
qexec.close() ;
}
}
{noformat}
where {{ResutlSetWrapper}} is the usual style. Call {{IO.close}} to explicit
close any {{ResultSet}} capable of that or (maybe) make that AutoCloseable as
well.
Add code to the path of {{hasNext}} and {{next}} to make self-closing when the
iterator runs out.
See also {{QueryIteratorCloseable}}.
was (Author: andy.seaborne):
See the nearby discussion {{AutoClosable}} for {{StmtIterators}} in
https://github.com/apache/jena/pull/133.
Adding {{AutoCloseable}} is a change that affects many users. The pattern is
that {{QueryExecution}} is closeable - adding that the inner {{ResultSet}}
should be as well is a burden. The doubly nested try-resource or doubled up
single try-resource is a change in preferred style.
Maybe if this were a green field API maybe it would be the way to go.
We need to avoid making work in user support. Even warnings appearing is a
cost. Such changes must be undertaken carefully.
Note that passing {{ResultSets}} around after the {{QueryExecution}} is closed
is not supported. The app needs to materialize the results and that
{{ResultSet}} is not attached to the {{QueryExecution}}.
When transactions are involved, the result set need materializing as well.
Adding {{Closeable}} to {{ResultSet}} is more practical but it encourages bad
practice. Teh {{QueryExecution}} is the unit to close.
There ways to get the are alternatives:
1. Return a {{QueryExecution}} object, not a {{ResultSet}}.
2. Return a pair of {{(QueryExecution, ResultSet}}. Not java's strong suit.
3. Extend the capabilities of a {{ResultSet}} to create a ResultSetCloseable:
{noformat}
public class ResultSetCloseable extends ResultSetWrapper implements Closeable {
private QueryExecution qexec ;
ResultSetCloseable(ResultSet rs, QueryExecution qexec) {
super(rs) ;
this.qexec = qexec ;
}
@Override
public void close() {
qexec.close() ;
}
}
{norformat}
where {{ResutlSetWrapper}} is the usual style. Call {{IO.close}} to explicit
close any {{ResultSet}} capable of that or (maybe) make that AutoCloseable as
well.
Add code to the path of {{hasNext}} and {{next}} to make self-closing when the
iterator runs out.
See also {{QueryIteratorCloseable}}.
> Make ResultSets closeable
> -------------------------
>
> Key: JENA-1215
> URL: https://issues.apache.org/jira/browse/JENA-1215
> Project: Apache Jena
> Issue Type: Improvement
> Components: ARQ
> Affects Versions: Jena 3.1.1
> Environment: any
> Reporter: Paul Houle
>
> Currently the QueryExecution has to be closed after you are done working with
> a ResultSet. This means that you can't write a function that returns a
> ResultSet and expect the QueryExecution to be properly closed.
> This problem can be fixed like so.
> (1) Make the ResultSet interface extend ClosableIterator and probably
> AutoCloseable (and fall back to just adding close() in case plan A causes
> something awful to happen)
> (2) Put a reference to the QueryExecution (probably just a reference to any
> Autoclosable) into the StreamResultSet
> (3) Have the StreamResultSet close() implementations delegate to that, or
> otherwise do nothing
> (4) Have the ResultSetStream close itself when it gets to the end.
> (5) Make sure close on the relevant QueryExecution element is idempotent
> (doesn't crash on a double close) otherwise behavior (4) followed by a later
> QE.close() could cause trouble
> (6) Make close() a no-op on ResultSetMem(), there is no point in punishing
> users for using it or not using it.
> (7) ResultSetMem does not autoclose, put it in the javadocs that a
> ResultSetRewindable does not self-close, but a ResultSet which is not
> Rewindable does close when it hits the end
> (8) Think about what exactly to do for a ResultSetPeekable which is not
> Rewindable so we don't get blindsided by a corner condition.
> (9) No isClosed() method on ResultSet because it would call attention to any
> fuzziness involved in (6)
> I volunteer to do this.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
