Andy, If I can work out how to look at logs when running tests (viz Log.warn in E_Regex) then I can make a test that shows duplicate log lines are dropped.
Do you think the changes we've been working on will land in 3.3.0? Chris On 12 April 2017 at 13:11, Chris Dollin <[email protected]> wrote: > On 12 April 2017 at 11:53, Andy Seaborne <[email protected]> wrote: > >> >> >> On 11/04/17 15:49, Chris Dollin wrote: >> >>> diff --git >>> a/jena-arq/src/main/java/org/apache/jena/sparql/engine/itera >>> tor/QueryIterSort.java >>> b/jena-arq/src/main/java/org/apache/jena/sparql/engine/itera >>> tor/QueryIterSort.java >>> index 173da34..04c2655 100644 >>> --- >>> a/jena-arq/src/main/java/org/apache/jena/sparql/engine/itera >>> tor/QueryIterSort.java >>> +++ >>> b/jena-arq/src/main/java/org/apache/jena/sparql/engine/itera >>> tor/QueryIterSort.java >>> @@ -92,6 +92,7 @@ public class QueryIterSort extends >>> QueryIterPlainWrapper { >>> // iterator in a try/finally block, and thus will call >>> // close() themselves. >>> catch (QueryCancelledException e) { >>> + QueryIterSort.this.close(); >>> >> >> Is this needed? The query is cancelled so QueryIterSort.requestCancel >> happens? >> > > Without it, the test fails, but of course that could be an error > in the test. > > I ran the failing test in the debugger with a breakpoint > at the QueryIterSort.close() and println()s at the catch above > and in requestCancel -- the former message was printed, the > latter not. > > I went back to the close() and looked at the call stack. All > the active methods from the test to the breakpoint were in > code that did not obviously catch exceptions. Interestingly > at line 113/114 of QueryIteratorBase we see > > // Handles exceptions > boolean r = hasNextBinding() ; > > which will let any QueryCancelledExceptions on upwards > without doing, say, a local cancel or close. > > In actual use the code at the top of the query execution > will itself catch any QueryCancelledException and then > close() the entire query, so there's a case for saying the > code is overkill and that the test can be dropped. > Belt and braces or clutter? > > Chris > > (Same applies to QueryIteratorTopN) > > -- > "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck > Affair/ > > Epimorphics Ltd, http://www.epimorphics.com > Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20 > 6PT > Epimorphics Ltd. is a limited company registered in England (number > 7016688) > -- "What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck Affair/ Epimorphics Ltd, http://www.epimorphics.com Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20 6PT Epimorphics Ltd. is a limited company registered in England (number 7016688)
