OK, after editing a bunch of tab/space inconsistencies,
the attached iter.diff takes master/cancel_sort and inserts
the tests. (Too make sure, I made the diff, undid the changes,
applied the diff -- hooray, doesn't barf -- and checked that
it looked like the changes had been applied.)
If an alternative approach would be better, let me know.
have a good Easter
Chris
On 13 April 2017 at 16:34, Chris Dollin <[email protected]>
wrote:
> On 13 April 2017 at 14:57, Chris Dollin <[email protected]>
> wrote:
>
>> On 13 April 2017 at 11:43, Andy Seaborne <[email protected]> wrote:
>>
>>>
>>>
>>>
> It would be quicker if you could provide the content in unbroken form.
>
>
>
>> Yes.
>>
>
> There will be a delay, I seem to produce diff files that can't
> be applied. Something to do with spaces and tabs.
>
> Chris
>
> --
> "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)
diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
index 173da34..a489aa1 100644
---
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterSort.java
+++
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/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();
close();
throw e;
}
diff --git
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterTopN.java
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterTopN.java
index a9e2f65..b66e036 100644
---
a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterTopN.java
+++
b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIterTopN.java
@@ -29,6 +29,7 @@ import java.util.PriorityQueue ;
import org.apache.jena.atlas.iterator.Iter ;
import org.apache.jena.atlas.iterator.IteratorDelayedInitialization ;
import org.apache.jena.query.Query ;
+import org.apache.jena.query.QueryCancelledException;
import org.apache.jena.query.QueryExecException ;
import org.apache.jena.query.SortCondition ;
import org.apache.jena.sparql.engine.ExecutionContext ;
@@ -93,22 +94,29 @@ public class QueryIterTopN extends QueryIterPlainWrapper
return new IteratorDelayedInitialization<Binding>() {
@Override
protected Iterator<Binding> initializeIterator() {
- while ( qIter.hasNext() ) {
- Binding binding = qIter.next() ;
- if ( heap.size() < limit )
- add(binding) ;
- else {
- Binding currentMaxLeastN = heap.peek() ;
- if ( comparator.compare(binding, currentMaxLeastN) < 0
)
- add(binding) ;
- }
- }
- qIter.close() ;
- Binding[] y = heap.toArray(new Binding[]{}) ;
- heap = null ;
- Arrays.sort(y, comparator) ;
- return asList(y).iterator() ;
- }
+ try {
+ while ( qIter.hasNext() ) {
+ Binding binding = qIter.next() ;
+ if ( heap.size() < limit )
+ add(binding) ;
+ else {
+ Binding currentMaxLeastN = heap.peek() ;
+ if ( comparator.compare(binding,
currentMaxLeastN) < 0 )
+ add(binding) ;
+ }
+ }
+ qIter.close() ;
+ Binding[] y = heap.toArray(new Binding[]{}) ;
+ heap = null ;
+ Arrays.sort(y, comparator) ;
+ return asList(y).iterator() ;
+ }
+ catch (QueryCancelledException e) {
+ QueryIterTopN.this.close();
+ this.close();
+ throw e;
+ }
+ }
} ;
}
diff --git
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
index dd1b08c..c4ec1ce 100644
---
a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
+++
b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestQueryIterSort.java
@@ -18,9 +18,7 @@
package org.apache.jena.sparql.engine.iterator;
-import static org.junit.Assert.assertEquals ;
-import static org.junit.Assert.assertNotNull ;
-import static org.junit.Assert.assertTrue ;
+import static org.junit.Assert.*;
import java.util.ArrayList ;
import java.util.Iterator ;
@@ -135,6 +133,37 @@ public class TestQueryIterSort {
assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)) ;
}
+ @Test public void testCloseClosesSourceIterator() {
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null,
(DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ qis.close();
+ assertTrue("source iterator should have been closed",
iterator.isClosed());
+ }
+
+ @Test public void testExhaustionClosesSourceIterator() {
+ iterator.setCallback(new Callback()
+ { @Override public void call() { /* do nothing */ }
+ });
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph)
null, (DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ while (qis.hasNext()) qis.next();
+ assertTrue("source iterator should have been closed",
iterator.isClosed());
+ }
+
+ @Test public void testCancelClosesSourceIterator() {
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph)
null, (DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterSort qis = new QueryIterSort(iterator, comparator, ec);
+ try {
+ while (qis.hasNext()) qis.next();
+ fail("query should have been cancelled by trigger");
+ } catch (QueryCancelledException q) {
+ assertTrue("source iterator should have been closed",
iterator.isClosed());
+ }
+ }
+
@Test
public void testCleanAfterExhaustion()
{
@@ -248,6 +277,32 @@ public class TestQueryIterSort {
assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)) ;
}
+ @Test public void testTopNCloseClosesSource() {
+
+ long numItems = 3;
+ boolean distinct = false;
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null,
(DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterTopN tn = new QueryIterTopN(iterator, comparator, numItems,
distinct, ec);
+ tn.close();
+ assertTrue(iterator.isClosed());
+ }
+
+ @Test public void testTopNExhaustionClosesSource() {
+
+ Callback nullCB = new Callback() { @Override public void call() {} };
+ iterator.setCallback(nullCB);
+
+ long numItems = 3;
+ boolean distinct = false;
+ Context context = new Context() ;
+ ExecutionContext ec = new ExecutionContext(context, (Graph) null,
(DatasetGraph) null, (OpExecutorFactory) null);
+ QueryIterTopN tn = new QueryIterTopN(iterator, comparator, numItems,
distinct, ec);
+ while (tn.hasNext()) tn.next();
+ assertTrue(iterator.isClosed());
+ }
+
+
private Binding randomBinding(Var[] vars)
{
BindingMap binding = BindingFactory.create();
@@ -279,13 +334,14 @@ public class TestQueryIterSort {
}
- private class CallbackIterator implements QueryIterator
+ private static class CallbackIterator implements QueryIterator
{
int elementsReturned = 0 ;
Callback callback ;
int trigger ;
Iterator<Binding> delegate ;
boolean canceled = false ;
+ boolean closed = false ;
public CallbackIterator(Iterator<Binding> delegate, int trigger,
Callback callback)
{
@@ -302,7 +358,10 @@ public class TestQueryIterSort {
@Override
public boolean hasNext()
{
- return delegate.hasNext() ;
+ // self-closing
+ boolean has = delegate.hasNext() ;
+ if (has == false) closed = true;
+ return has ;
}
@Override
@@ -326,6 +385,10 @@ public class TestQueryIterSort {
return elementsReturned ;
}
+ public boolean isClosed() {
+ return closed ;
+ }
+
public boolean isCanceled() {
return canceled ;
}
@@ -341,7 +404,7 @@ public class TestQueryIterSort {
public void cancel() { canceled = true ; }
@Override
- public void close() { }
+ public void close() { closed = true ; }
@Override
public void output(IndentedWriter out, SerializationContext sCxt) {
throw new ARQNotImplemented() ; }
@@ -354,7 +417,7 @@ public class TestQueryIterSort {
}
- private interface Callback
+ public interface Callback
{
public void call() ;
}
