This is an automated email from the ASF dual-hosted git repository. andy pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/jena.git
commit 6fe851695936bc8d33f802bf6a9b4bc7fa19c031 Author: Andy Seaborne <[email protected]> AuthorDate: Tue Feb 17 16:09:42 2026 +0000 GH-3755: Put in static functions to create sort iterators --- .../jena/sparql/engine/iterator/QueryIterSort.java | 32 +++++++------ .../jena/sparql/engine/iterator/QueryIterTopN.java | 18 ++++--- .../sparql/engine/iterator/QueryIteratorBase.java | 5 +- .../apache/jena/sparql/engine/main/OpExecutor.java | 6 +-- .../jena/sparql/engine/ref/EvaluatorSimple.java | 2 +- .../sparql/engine/iterator/TestQueryIterSort.java | 56 +++++++++++----------- .../iterator/TestSortedDataBagCancellation.java | 6 ++- 7 files changed, 68 insertions(+), 57 deletions(-) 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 f0fa64261c..1a21f1b5bf 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 @@ -47,32 +47,36 @@ import org.apache.jena.sparql.system.SerializationFactoryFinder; */ public class QueryIterSort extends QueryIterPlainWrapper { - private final QueryIterator embeddedIterator; - /*package*/ final SortedDataBag<Binding> db; + private final QueryIterator inputIterator; + /*package*/ final SortedDataBag<Binding> dataBag; - public QueryIterSort(QueryIterator qIter, List<SortCondition> conditions, ExecutionContext context) { - this(qIter, new BindingComparator(conditions, context), context); + public static QueryIterator create(QueryIterator qIter, List<SortCondition> conditions, ExecutionContext context) { + return create(qIter, new BindingComparator(conditions, context), context); } - public QueryIterSort(QueryIterator qIter, Comparator<Binding> comparator, ExecutionContext context) { + public static QueryIterator create(QueryIterator qIter, Comparator<Binding> comparator, ExecutionContext context) { + return new QueryIterSort(qIter, comparator, context); + } + + private QueryIterSort(QueryIterator qIter, Comparator<Binding> comparator, ExecutionContext context) { super(null, context); - this.embeddedIterator = qIter; + this.inputIterator = qIter; ThresholdPolicy<Binding> policy = ThresholdPolicyFactory.policyFromContext(context.getContext()); - this.db = BagFactory.newSortedBag(policy, SerializationFactoryFinder.bindingSerializationFactory(), comparator); + this.dataBag = BagFactory.newSortedBag(policy, SerializationFactoryFinder.bindingSerializationFactory(), comparator); this.setIterator(new SortedBindingIterator(qIter)); } @Override public void requestCancel() { - this.db.cancel(); - this.embeddedIterator.cancel(); + this.dataBag.cancel(); + this.inputIterator.cancel(); super.requestCancel(); } @Override protected void closeIterator() { - this.db.close(); - this.embeddedIterator.close(); + this.dataBag.close(); + this.inputIterator.close(); super.closeIterator(); } @@ -86,8 +90,8 @@ public class QueryIterSort extends QueryIterPlainWrapper { @Override protected Iterator<Binding> initializeIterator() { try { - db.addAll(qIter); - return db.iterator(); + dataBag.addAll(qIter); + return dataBag.iterator(); } // Should we catch other exceptions too? Theoretically // the user should be using this @@ -102,7 +106,7 @@ public class QueryIterSort extends QueryIterPlainWrapper { @Override public void close() { - db.close(); + dataBag.close(); } } 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 95f5b7f82b..a161c03350 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 @@ -48,18 +48,22 @@ public class QueryIterTopN extends QueryIterPlainWrapper * To keep another element, it must be less than the max so far. * This leaves the least N in the heap. */ - private final QueryIterator embeddedIterator; // Keep a record of the underlying source for .cancel. + private final QueryIterator inputIterator; // Keep a record of the unsorted underlying source for .cancel. private PriorityQueue<Binding> heap; private long limit; private final boolean distinct; - public QueryIterTopN(QueryIterator qIter, List<SortCondition> conditions, long numItems, boolean distinct, ExecutionContext context) { - this(qIter, new BindingComparator(conditions, context), numItems, distinct, context); + public static QueryIterator create(QueryIterator qIter, List<SortCondition> conditions, long numItems, boolean distinct, ExecutionContext context) { + return create(qIter, new BindingComparator(conditions, context), numItems, distinct, context); } - public QueryIterTopN(QueryIterator qIter, Comparator<Binding> comparator, long numItems, boolean distinct, ExecutionContext context) { + public static QueryIterator create(QueryIterator qIter, Comparator<Binding> comparator, long numItems, boolean distinct, ExecutionContext context) { + return new QueryIterTopN(qIter, comparator, numItems, distinct, context); + } + + private QueryIterTopN(QueryIterator qIter, Comparator<Binding> comparator, long numItems, boolean distinct, ExecutionContext context) { super(null, context); - this.embeddedIterator = qIter; + this.inputIterator = qIter; this.distinct = distinct; limit = numItems; @@ -84,13 +88,13 @@ public class QueryIterTopN extends QueryIterPlainWrapper @Override public void requestCancel() { - this.embeddedIterator.cancel(); + this.inputIterator.cancel(); super.requestCancel(); } @Override protected void closeIterator() { - this.embeddedIterator.close(); + this.inputIterator.close(); super.closeIterator(); } diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java index b226208706..68fd08623e 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/iterator/QueryIteratorBase.java @@ -55,7 +55,10 @@ public abstract class QueryIteratorBase // It causes notification to cancellation to be made, once, by calling .requestCancel() // which is called synchronously with .cancel() and asynchronously with iterator execution. private final AtomicBoolean requestingCancel; + + // Used so that requestCanel is called once. private volatile boolean cancelOnce = false; + private Object cancelLock = new Object(); /** QueryIteratorBase with no cancellation facility */ @@ -76,8 +79,6 @@ public abstract class QueryIteratorBase return (requestingCancel != null && requestingCancel.get()) || Thread.currentThread().isInterrupted() ; } - private void haveCancelled() {} - // -------- The contract with the subclasses /** diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/OpExecutor.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/OpExecutor.java index e3bd31f5c5..9d6adf76d3 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/OpExecutor.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/main/OpExecutor.java @@ -373,7 +373,7 @@ public class OpExecutor { protected QueryIterator execute(OpOrder opOrder, QueryIterator input) { QueryIterator qIter = exec(opOrder.getSubOp(), input); - qIter = new QueryIterSort(qIter, opOrder.getConditions(), execCxt); + qIter = QueryIterSort.create(qIter, opOrder.getConditions(), execCxt); return qIter; } @@ -386,10 +386,10 @@ public class OpExecutor { if ( opTop.getSubOp() instanceof OpDistinct ) { OpDistinct opDistinct = (OpDistinct)opTop.getSubOp(); qIter = exec(opDistinct.getSubOp(), input); - qIter = new QueryIterTopN(qIter, opTop.getConditions(), opTop.getLimit(), true, execCxt); + qIter = QueryIterTopN.create(qIter, opTop.getConditions(), opTop.getLimit(), true, execCxt); } else { qIter = exec(opTop.getSubOp(), input); - qIter = new QueryIterTopN(qIter, opTop.getConditions(), opTop.getLimit(), false, execCxt); + qIter = QueryIterTopN.create(qIter, opTop.getConditions(), opTop.getLimit(), false, execCxt); } return qIter; } diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/EvaluatorSimple.java b/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/EvaluatorSimple.java index ffc6a9ae59..c7de0c8bd1 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/EvaluatorSimple.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/engine/ref/EvaluatorSimple.java @@ -260,7 +260,7 @@ public class EvaluatorSimple implements Evaluator { @Override public Table order(Table table, List<SortCondition> conditions) { QueryIterator qIter = table.iterator(getExecContext()); - qIter = new QueryIterSort(qIter, conditions, getExecContext()); + qIter = QueryIterSort.create(qIter, conditions, getExecContext()); return new TableN(qIter); } 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 cfe5f10faf..b2a7b974fa 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 @@ -96,13 +96,13 @@ public class TestQueryIterSort { assertEquals(0, iterator.getReturnedElementCount()); Context context = new Context(); ExecutionContext executionContext = createExecutionContext(context); - QueryIterSort qIter = new QueryIterSort(iterator, comparator, executionContext); + QueryIterSort qIter = (QueryIterSort)QueryIterSort.create(iterator, comparator, executionContext); try { assertEquals(0, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.hasNext(); assertEquals(500, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); } finally { qIter.close(); } @@ -115,25 +115,25 @@ public class TestQueryIterSort { Context context = new Context(); context.set(ARQ.spillToDiskThreshold, 10L); ExecutionContext executionContext = createExecutionContext(context); - QueryIterSort qIter = new QueryIterSort(iterator, comparator, executionContext); + QueryIterSort qIter = (QueryIterSort)QueryIterSort.create(iterator, comparator, executionContext); try { assertEquals(0, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.hasNext(); assertEquals(500, iterator.getReturnedElementCount()); - assertEquals(49, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(49, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); } finally { qIter.close(); } - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); } @Test public void testCloseClosesSourceIterator() { Context context = new Context(); ExecutionContext ec = createExecutionContext(context); - QueryIterSort qis = new QueryIterSort(iterator, comparator, ec); + QueryIterSort qis = (QueryIterSort)QueryIterSort.create(iterator, comparator, ec); qis.close(); assertTrue(iterator.isClosed(), "source iterator should have been closed"); } @@ -143,7 +143,7 @@ public class TestQueryIterSort { iterator.setCallback(() -> {}); Context context = new Context(); ExecutionContext ec = createExecutionContext(context); - QueryIterSort qis = new QueryIterSort(iterator, comparator, ec); + QueryIterSort qis = (QueryIterSort)QueryIterSort.create(iterator, comparator, ec); while (qis.hasNext()) qis.next(); assertTrue(iterator.isClosed(), "source iterator should have been closed"); @@ -153,7 +153,7 @@ public class TestQueryIterSort { public void testCancelClosesSourceIterator() { Context context = new Context(); ExecutionContext ec = createExecutionContext(context); - QueryIterSort qis = new QueryIterSort(iterator, comparator, ec); + QueryIterSort qis = (QueryIterSort)QueryIterSort.create(iterator, comparator, ec); try { while (qis.hasNext()) qis.next(); @@ -170,20 +170,20 @@ public class TestQueryIterSort { Context context = new Context(); context.set(ARQ.spillToDiskThreshold, 10L); ExecutionContext executionContext = createExecutionContext(context); - QueryIterSort qIter = new QueryIterSort(iterator, comparator, executionContext); + QueryIterSort qIter = (QueryIterSort)QueryIterSort.create(iterator, comparator, executionContext); // Usually qIter should be in a try/finally block, but we are testing the // case that the user forgot to do that. // As a failsafe, QueryIteratorBase should close it when the iterator is // exhausted. assertEquals(0, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.hasNext(); assertEquals(500, iterator.getReturnedElementCount()); - assertEquals(49, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(49, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); while (qIter.hasNext()) qIter.next(); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.close(); } @@ -194,10 +194,10 @@ public class TestQueryIterSort { Context context = new Context(); context.set(ARQ.spillToDiskThreshold, 10L); ExecutionContext executionContext = createExecutionContext(context); - QueryIterSort qIter = new QueryIterSort(iterator, comparator, executionContext); + QueryIterSort qIter = (QueryIterSort)QueryIterSort.create(iterator, comparator, executionContext); try { assertEquals(0, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.cancel(); assertThrows(QueryCancelledException.class, ()->qIter.hasNext() ) ; @@ -205,11 +205,11 @@ public class TestQueryIterSort { } finally { assertTrue(iterator.isCanceled()); assertEquals(0, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.close(); } - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); } @@ -219,12 +219,12 @@ public class TestQueryIterSort { Context context = new Context(); context.set(ARQ.spillToDiskThreshold, 10L); ExecutionContext executionContext = createExecutionContext(context); - QueryIterSort qIter = new QueryIterSort(iterator, comparator, executionContext); + QueryIterSort qIter = (QueryIterSort)QueryIterSort.create(iterator, comparator, executionContext); assertThrows(QueryCancelledException.class, ()->{ try { assertEquals(0, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.hasNext(); // throws a QueryCancelledException } catch (QueryCancelledException e) { // expected @@ -233,13 +233,13 @@ public class TestQueryIterSort { // throwing the QueryCancelledException. // It does this as a failsafe in case the user doesn't close the // QueryIterator themselves. - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); throw e; } finally { qIter.close(); } }); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); } @Test @@ -250,10 +250,10 @@ public class TestQueryIterSort { Context context = new Context(); context.set(ARQ.spillToDiskThreshold, 10L); ExecutionContext executionContext = createExecutionContext(context); - QueryIterSort qIter = new QueryIterSort(iterator, comparator, executionContext); + QueryIterSort qIter = (QueryIterSort)QueryIterSort.create(iterator, comparator, executionContext); try { assertTrue(qIter.hasNext()); - assertEquals(49, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(49, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); assertNotNull(qIter.next()); assertTrue(qIter.hasNext()); @@ -263,11 +263,11 @@ public class TestQueryIterSort { } finally { // assertTrue(iterator.isCanceled()); assertEquals(500, iterator.getReturnedElementCount()); - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); qIter.close(); } - assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.db)); + assertEquals(0, DataBagExaminer.countTemporaryFiles(qIter.dataBag)); } @Test @@ -276,7 +276,7 @@ public class TestQueryIterSort { boolean distinct = false; Context context = new Context(); ExecutionContext ec = createExecutionContext(context); - QueryIterTopN tn = new QueryIterTopN(iterator, comparator, numItems, distinct, ec); + QueryIterator tn = QueryIterTopN.create(iterator, comparator, numItems, distinct, ec); tn.close(); assertTrue(iterator.isClosed()); } @@ -288,7 +288,7 @@ public class TestQueryIterSort { boolean distinct = false; Context context = new Context(); ExecutionContext ec = createExecutionContext(context); - QueryIterTopN tn = new QueryIterTopN(iterator, comparator, numItems, distinct, ec); + QueryIterator tn = QueryIterTopN.create(iterator, comparator, numItems, distinct, ec); while (tn.hasNext()) tn.next(); assertTrue(iterator.isClosed()); diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestSortedDataBagCancellation.java b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestSortedDataBagCancellation.java index 0616e8d724..75964dc431 100644 --- a/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestSortedDataBagCancellation.java +++ b/jena-arq/src/test/java/org/apache/jena/sparql/engine/iterator/TestSortedDataBagCancellation.java @@ -40,6 +40,7 @@ import org.apache.jena.sparql.core.DatasetGraph; import org.apache.jena.sparql.core.DatasetGraphFactory; import org.apache.jena.sparql.core.Var; import org.apache.jena.sparql.engine.ExecutionContext; +import org.apache.jena.sparql.engine.QueryIterator; import org.apache.jena.sparql.engine.binding.Binding; import org.apache.jena.sparql.engine.binding.BindingComparator; import org.apache.jena.sparql.engine.binding.BindingFactory; @@ -80,7 +81,7 @@ public class TestSortedDataBagCancellation { return iter; } - QueryIterSort qs = new QueryIterSort(baseIter, bc, ec); + private QueryIterator qs = QueryIterSort.create(baseIter, bc, ec); /** * In this test, the iterator is not cancelled; all items should be @@ -145,7 +146,8 @@ public class TestSortedDataBagCancellation { while (qs.hasNext()) qs.next(); } catch (QueryCancelledException qe) { - assertTrue(qs.db.isCancelled()); + QueryIterSort qIterSort = (QueryIterSort)qs; + assertTrue(qIterSort.dataBag.isCancelled()); return; } fail("query was not cancelled");
