From 86173b00ef0a1c5f617aa976394dbf9c29813792 Mon Sep 17 00:00:00 2001 From: Brett Wooldridge Date: Fri, 4 Sep 2026 12:40:25 +0900 Subject: [PATCH 1/7] perf: stream equality and range index scans instead of materializing every id NitriteIndexer.findByFilter returns a LinkedHashSet of every matching id, so find(k = v).firstOrNull() built the whole match set before handing back one row, and a bounded page paid for the entire result. On a non-unique index over a low-cardinality field that set is a large fraction of the collection on every lookup. The composite layout already keeps its rows in key order, so the two plan shapes that map onto one bounded walk of it, an equality on the indexed field and a two-sided range on it, are now served by a lazy iterator that starts at the first key inside the bounds and stops at the first key outside them. It honours the plan's reverse scan order by visiting the key groups backwards while reading each group forwards, exactly as the materialized scan orders them, skips entries removed in an open transaction, and returns a document indexed under several keys once. NitriteIndex.findNitriteIdStream and NitriteIndexer.findByFilterStream are new default methods returning null, so every other index type, plugin indexer and plan shape keeps the materialized path unchanged. ReadOperations prefers the stream when one is offered; the covered-count shortcut that lets size() answer without fetching documents is kept by counting the streamed ids on demand, so size() still reads the index only. Tests compare the stream with the materialized scan for equality, range and reverse order, check the shapes it declines, show with a spied map that only one key is read for the first row, and exercise counts, paging, descending order, multi-valued fields and removals through the public API. Co-Authored-By: Claude Fable 5.1 --- .../collection/operation/ReadOperations.java | 67 ++++--- .../no2/common/streams/DocumentStream.java | 12 ++ .../no2/common/streams/IndexedStream.java | 21 +- .../dizitart/no2/index/ComparableIndexer.java | 7 + .../org/dizitart/no2/index/NitriteIndex.java | 14 ++ .../dizitart/no2/index/NitriteIndexer.java | 14 ++ .../dizitart/no2/index/SingleFieldIndex.java | 185 ++++++++++++++++++ .../no2/collection/LazyIndexScanTest.java | 103 ++++++++++ .../no2/index/SingleFieldIndexTest.java | 85 ++++++++ 9 files changed, 480 insertions(+), 28 deletions(-) create mode 100644 nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java index e3de0e96..852b64cc 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java @@ -116,35 +116,43 @@ private void prepareLogicalFilter(LogicalFilter logicalFilter) { } private DocumentCursor createCursor(FindPlan findPlan) { - // -1 means "not an index scan"; the index branch records the exact id-set size here. - long[] indexedIdCount = { -1 }; - RecordStream> recordStream = findSuitableStream(findPlan, indexedIdCount); + IndexScan scan = new IndexScan(); + RecordStream> recordStream = findSuitableStream(findPlan, scan); DocumentStream cursor = new DocumentStream(recordStream, processorChain); cursor.setFindPlan(findPlan); - cursor.setCoveredCount(computeCoveredCount(findPlan, indexedIdCount[0])); + if (isCountCovered(findPlan)) { + if (findPlan.getIndexDescriptor() == null) { + // pure full scan over the whole collection + cursor.setCoveredCount(nitriteMap.size()); + } else if (scan.lazyStream != null) { + // the ids are read lazily: count them from the index only if size() is asked + cursor.setCoveredCountSupplier(scan.lazyStream::countIds); + } else if (scan.idCount >= 0) { + // the index supplied the exact matching id set + cursor.setCoveredCount(scan.idCount); + } + } return cursor; } + /** What an index scan handed back: an exact id count, or the lazy stream, or neither. */ + private static final class IndexScan { + private long idCount = -1; + private IndexedStream lazyStream; + } + /** * Returns the exact match count when the query is fully answered without fetching documents, * or {@code null} when the cursor must be drained to count. The count is exact only when * nothing downstream drops or changes cardinality (a post-filter, skip, or limit); sort does * not change the count, and an OR-union needs de-duplication so its count cannot be derived. */ - private Long computeCoveredCount(FindPlan findPlan, long indexedIdCount) { - if (!findPlan.getSubPlans().isEmpty() - || findPlan.getCollectionScanFilter() != null - || findPlan.getSkip() != null - || findPlan.getLimit() != null - || findPlan.getByIdFilter() != null) { - return null; - } - if (findPlan.getIndexDescriptor() != null) { - // the index supplied the exact matching id set - return indexedIdCount >= 0 ? indexedIdCount : null; - } - // pure full scan over the whole collection - return nitriteMap.size(); + private boolean isCountCovered(FindPlan findPlan) { + return findPlan.getSubPlans().isEmpty() + && findPlan.getCollectionScanFilter() == null + && findPlan.getSkip() == null + && findPlan.getLimit() == null + && findPlan.getByIdFilter() == null; } /** @@ -198,7 +206,7 @@ private static Object indexedValue(DBValue dbValue) { return dbValue == null || dbValue instanceof DBNull ? null : dbValue.getValue(); } - private RecordStream> findSuitableStream(FindPlan findPlan, long[] indexedIdCount) { + private RecordStream> findSuitableStream(FindPlan findPlan, IndexScan scan) { RecordStream> rawStream; RecordStream> indexSortedStream = null; @@ -207,7 +215,7 @@ private RecordStream> findSuitableStream(FindPlan find List>> subStreams = new ArrayList<>(); for (FindPlan subPlan : findPlan.getSubPlans()) { // a sub-plan's own id count cannot answer the union's count (dedup), so discard it - RecordStream> suitableStream = findSuitableStream(subPlan, new long[]{ -1 }); + RecordStream> suitableStream = findSuitableStream(subPlan, new IndexScan()); subStreams.add(suitableStream); } @@ -237,14 +245,21 @@ private RecordStream> findSuitableStream(FindPlan find if (indexDescriptor != null) { // get optimized filter NitriteIndexer indexer = nitriteConfig.findIndexer(indexDescriptor.getIndexType()); - LinkedHashSet nitriteIds = indexer.findByFilter(findPlan, nitriteConfig); + RecordStream idStream = indexer.findByFilterStream(findPlan, nitriteConfig); + if (idStream != null) { + // the index walks its matches lazily; a size() is counted from it on demand + scan.lazyStream = new IndexedStream(idStream, nitriteMap); + rawStream = scan.lazyStream; + } else { + LinkedHashSet nitriteIds = indexer.findByFilter(findPlan, nitriteConfig); - // the index supplied the exact matching id set; record its size so a size() - // with no row-dropping step downstream can answer from it without fetching - indexedIdCount[0] = nitriteIds.size(); + // the index supplied the exact matching id set; record its size so a size() + // with no row-dropping step downstream can answer from it without fetching + scan.idCount = nitriteIds.size(); - // create indexed stream from optimized filter - rawStream = new IndexedStream(nitriteIds, nitriteMap); + // create indexed stream from optimized filter + rawStream = new IndexedStream(nitriteIds, nitriteMap); + } } else { indexSortedStream = indexSortedStream(findPlan); rawStream = indexSortedStream != null ? indexSortedStream : nitriteMap.entries(); diff --git a/nitrite/src/main/java/org/dizitart/no2/common/streams/DocumentStream.java b/nitrite/src/main/java/org/dizitart/no2/common/streams/DocumentStream.java index e2b2e175..6dd03171 100644 --- a/nitrite/src/main/java/org/dizitart/no2/common/streams/DocumentStream.java +++ b/nitrite/src/main/java/org/dizitart/no2/common/streams/DocumentStream.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.Iterator; +import java.util.function.LongSupplier; /** * @since 4.0 @@ -53,6 +54,13 @@ public class DocumentStream implements DocumentCursor { @Setter private Long coveredCount; + /** + * Answers {@link #size()} from the index on demand when the match count is known to be + * covered but the ids are streamed lazily rather than materialized; evaluated once. + */ + @Setter + private LongSupplier coveredCountSupplier; + public DocumentStream(RecordStream> recordStream, ProcessorChain processorChain) { this.recordStream = recordStream; @@ -64,6 +72,10 @@ public long size() { if (coveredCount != null) { return coveredCount; } + if (coveredCountSupplier != null) { + coveredCount = coveredCountSupplier.getAsLong(); + return coveredCount; + } return Iterables.size(this); } diff --git a/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java b/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java index 285d8f7c..24d65af3 100644 --- a/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java +++ b/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java @@ -24,8 +24,11 @@ import org.dizitart.no2.store.NitriteMap; import java.util.Iterator; +<<<<<<< HEAD import java.util.NoSuchElementException; import java.util.Set; +======= +>>>>>>> a09dd45e (perf: stream equality and range index scans instead of materializing every id) /** * @author Anindya Chatterjee @@ -33,9 +36,9 @@ */ public class IndexedStream implements RecordStream> { private final NitriteMap nitriteMap; - private final Set nitriteIds; + private final Iterable nitriteIds; - public IndexedStream(Set nitriteIds, + public IndexedStream(Iterable nitriteIds, NitriteMap nitriteMap) { this.nitriteIds = nitriteIds; this.nitriteMap = nitriteMap; @@ -46,6 +49,20 @@ public Iterator> iterator() { return new IndexedStreamIterator(nitriteIds.iterator(), nitriteMap); } + /** + * Counts the ids the index supplied, walking the id source only, without fetching a + * single document. + * + * @return the number of ids + */ + public long countIds() { + long count = 0; + for (NitriteId ignored : nitriteIds) { + count++; + } + return count; + } + private static class IndexedStreamIterator implements Iterator>, SkippableIterator { private final Iterator iterator; diff --git a/nitrite/src/main/java/org/dizitart/no2/index/ComparableIndexer.java b/nitrite/src/main/java/org/dizitart/no2/index/ComparableIndexer.java index 49630d1c..45a03d3c 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/ComparableIndexer.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/ComparableIndexer.java @@ -21,6 +21,7 @@ import org.dizitart.no2.collection.NitriteId; import org.dizitart.no2.common.DBValue; import org.dizitart.no2.common.FieldValues; +import org.dizitart.no2.common.RecordStream; import org.dizitart.no2.common.Fields; import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.exceptions.IndexingException; @@ -66,6 +67,12 @@ public LinkedHashSet findByFilter(FindPlan findPlan, NitriteConfig ni return nitriteIndex.findNitriteIds(findPlan); } + @Override + public RecordStream findByFilterStream(FindPlan findPlan, NitriteConfig nitriteConfig) { + NitriteIndex nitriteIndex = findNitriteIndex(findPlan.getIndexDescriptor(), nitriteConfig); + return nitriteIndex.findNitriteIdStream(findPlan); + } + @Override public List> readSortKeys(IndexDescriptor indexDescriptor, NitriteConfig nitriteConfig, diff --git a/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java index d10f48b1..31af2f26 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java @@ -21,6 +21,7 @@ import org.dizitart.no2.collection.NitriteId; import org.dizitart.no2.common.DBValue; import org.dizitart.no2.common.FieldValues; +import org.dizitart.no2.common.RecordStream; import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.exceptions.UniqueConstraintException; import org.dizitart.no2.exceptions.ValidationException; @@ -74,6 +75,19 @@ public interface NitriteIndex { */ LinkedHashSet findNitriteIds(FindPlan findPlan); + /** + * Streams the ids matching the plan lazily, in index order and without duplicates, or + * returns {@code null} when this index cannot do so for the given plan, in which case the + * caller falls back to {@link #findNitriteIds(FindPlan)}. A stream lets a query that only + * needs the first rows, or a bounded page, stop reading the index as soon as it has them. + * + * @param findPlan the find plan + * @return a re-iterable stream of ids, or {@code null} + */ + default RecordStream findNitriteIdStream(FindPlan findPlan) { + return null; + } + /** * Reads every {@code (indexed value, id)} pair out of the index, so a sorted query can * decide its order without deserializing a single document. diff --git a/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndexer.java b/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndexer.java index ba8085f6..bc51fa35 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndexer.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/NitriteIndexer.java @@ -21,6 +21,7 @@ import org.dizitart.no2.collection.NitriteId; import org.dizitart.no2.common.DBValue; import org.dizitart.no2.common.FieldValues; +import org.dizitart.no2.common.RecordStream; import org.dizitart.no2.common.Fields; import org.dizitart.no2.common.module.NitritePlugin; import org.dizitart.no2.common.tuples.Pair; @@ -88,6 +89,19 @@ public interface NitriteIndexer extends NitritePlugin { */ LinkedHashSet findByFilter(FindPlan findPlan, NitriteConfig nitriteConfig); + /** + * Streams the ids matching the plan lazily, or returns {@code null} when the indexer has no + * lazy path for it and {@link #findByFilter(FindPlan, NitriteConfig)} must be used. The + * default is {@code null}, so existing indexer plugins are unaffected. + * + * @param findPlan the find plan + * @param nitriteConfig the nitrite config + * @return a re-iterable stream of ids, or {@code null} + */ + default RecordStream findByFilterStream(FindPlan findPlan, NitriteConfig nitriteConfig) { + return null; + } + /** * Reads every {@code (indexed value, id)} pair out of the given index, so a sorted query * can decide its order without deserializing a single document. diff --git a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java index d324c8eb..84479eba 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java @@ -27,6 +27,9 @@ import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.filters.ComparableFilter; import org.dizitart.no2.exceptions.UniqueConstraintException; +import org.dizitart.no2.common.RecordStream; +import org.dizitart.no2.filters.EqualsFilter; +import org.dizitart.no2.filters.SortingAwareFilter; import org.dizitart.no2.store.NitriteMap; import org.dizitart.no2.store.NitriteStore; @@ -34,6 +37,8 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.NoSuchElementException; +import java.util.Iterator; import java.util.Set; import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName; @@ -147,6 +152,186 @@ public LinkedHashSet findNitriteIds(FindPlan findPlan) { return scanIndex(findPlan, iMap); } + /** + * A lazy id stream for the two plan shapes the composite layout answers with one bounded + * walk of the map: an equality on the indexed field, and a two-sided range on it. The walk + * starts at the first key inside the bounds and stops at the first key outside them, so a + * caller that wants the first row, or a page, reads only that far. Every other shape, and + * the unique layout, returns {@code null} and is served by {@link #findNitriteIds(FindPlan)}. + */ + @Override + public RecordStream findNitriteIdStream(FindPlan findPlan) { + if (!useCompositeLayout() || findPlan.getIndexScanFilter() == null) { + return null; + } + List filters = findPlan.getIndexScanFilter().getFilters(); + Range range = Range.of(filters); + if (range == null) { + return null; + } + String field = filters.get(0).getField(); + boolean reverse = findPlan.getIndexScanOrder() != null + && Boolean.TRUE.equals(findPlan.getIndexScanOrder().get(field)); + NitriteMap compositeMap = findCompositeMap(); + return RecordStream.fromIterable(() -> new CompositeRangeIterator(compositeMap, range, reverse)); + } + + /** Inclusive-or-exclusive bounds on the indexed value; {@code null} when a plan has another shape. */ + private static final class Range { + private final DBValue lower; + private final boolean lowerInclusive; + private final DBValue upper; + private final boolean upperInclusive; + + private Range(DBValue lower, boolean lowerInclusive, DBValue upper, boolean upperInclusive) { + this.lower = lower; + this.lowerInclusive = lowerInclusive; + this.upper = upper; + this.upperInclusive = upperInclusive; + } + + static Range of(List filters) { + if (filters == null || filters.isEmpty()) { + return null; + } + if (filters.size() == 1 && filters.get(0).getClass() == EqualsFilter.class) { + Object value = filters.get(0).getValue(); + if (value == null) { + return new Range(DBNull.getInstance(), true, DBNull.getInstance(), true); + } + if (!(value instanceof Comparable)) { + return null; + } + DBValue key = new DBValue((Comparable) value); + return new Range(key, true, key, true); + } + + // a two-sided range on one field, the same shape IndexScanner.scanBoundedRange takes + String field = filters.get(0).getField(); + DBValue lower = null, upper = null; + boolean lowerInclusive = false, upperInclusive = false; + for (ComparableFilter filter : filters) { + if (!(filter instanceof SortingAwareFilter) || field == null || !field.equals(filter.getField())) { + return null; + } + Object value = filter.getValue(); + if (!(value instanceof Comparable)) { + return null; + } + DBValue key = new DBValue((Comparable) value); + switch (((SortingAwareFilter) filter).getComparisonMode()) { + case GreaterEqual: + if (lower != null) return null; + lower = key; lowerInclusive = true; break; + case Greater: + if (lower != null) return null; + lower = key; lowerInclusive = false; break; + case LesserEqual: + if (upper != null) return null; + upper = key; upperInclusive = true; break; + case Lesser: + if (upper != null) return null; + upper = key; upperInclusive = false; break; + default: + return null; + } + } + return lower == null || upper == null ? null : new Range(lower, lowerInclusive, upper, upperInclusive); + } + } + + /** + * Walks the composite map between the bounds, in index order or in reverse, skipping + * entries removed in an open transaction and ids already returned (a multi-valued field + * indexes one document under several keys). Ids sharing a key are always returned in their + * stored order, so a reverse walk visits the key groups backwards but reads each group + * forwards, exactly as the materialized scan orders them. + */ + private static final class CompositeRangeIterator implements Iterator { + private final NitriteMap map; + private final Range range; + private final boolean reverse; + private final Set seen = new HashSet<>(); + private final java.util.ArrayDeque group = new java.util.ArrayDeque<>(); + private IndexEntryKey key; + private NitriteId next; + private boolean started; + + CompositeRangeIterator(NitriteMap map, Range range, boolean reverse) { + this.map = map; + this.range = range; + this.reverse = reverse; + } + + @Override + public boolean hasNext() { + if (next == null) { + advance(); + } + return next != null; + } + + @Override + public NitriteId next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + NitriteId id = next; + next = null; + return id; + } + + private void advance() { + if (!started) { + started = true; + key = reverse + ? (range.upperInclusive ? map.floorKey(IndexEntryKey.upperBound(range.upper)) : map.lowerKey(IndexEntryKey.lowerBound(range.upper))) + : (range.lowerInclusive ? map.ceilingKey(IndexEntryKey.lowerBound(range.lower)) : map.higherKey(IndexEntryKey.upperBound(range.lower))); + } + while (true) { + while (!group.isEmpty()) { + NitriteId id = group.pollFirst(); + if (seen.add(id)) { + next = id; + return; + } + } + if (key == null || !within(key)) { + key = null; + return; + } + if (reverse) { + // read this key's group forwards, then continue below it + DBValue value = key.getValue(); + for (IndexEntryKey k = map.ceilingKey(IndexEntryKey.lowerBound(value)); + k != null && k.getValue().compareTo(value) == 0; + k = map.higherKey(k)) { + if (map.get(k) != null) { + group.addLast(k.getNitriteId()); + } + } + key = map.lowerKey(IndexEntryKey.lowerBound(value)); + } else { + IndexEntryKey current = key; + key = map.higherKey(current); + if (map.get(current) != null) { + // removed in the current transaction otherwise; navigation still surfaces the key + group.addLast(current.getNitriteId()); + } + } + } + } + + private boolean within(IndexEntryKey candidate) { + if (reverse) { + int cmp = candidate.getValue().compareTo(range.lower); + return range.lowerInclusive ? cmp >= 0 : cmp > 0; + } + int cmp = candidate.getValue().compareTo(range.upper); + return range.upperInclusive ? cmp <= 0 : cmp < 0; + } + } + @Override @SuppressWarnings("unchecked") public List> readSortKeys(long collectionSize) { diff --git a/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java b/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java new file mode 100644 index 00000000..8ea41b0c --- /dev/null +++ b/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java @@ -0,0 +1,103 @@ +/* + * Copyright (c) 2017-2020. Nitrite author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.dizitart.no2.collection; + +import org.dizitart.no2.Nitrite; +import org.dizitart.no2.common.SortOrder; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.util.List; + +import static org.dizitart.no2.collection.FindOptions.orderBy; +import static org.dizitart.no2.collection.FindOptions.skipBy; +import static org.dizitart.no2.filters.FluentFilter.where; +import static org.dizitart.no2.index.IndexOptions.indexOptions; +import static org.dizitart.no2.index.IndexType.NON_UNIQUE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +/** + * Equality and two-sided range queries on a non-unique index now stream their ids from the + * index instead of materializing every match first. The results, their order, their count and + * paging over them must be exactly what the materialized scan produced. + */ +public class LazyIndexScanTest { + private Nitrite db; + private NitriteCollection collection; + + @Before + public void setUp() { + db = Nitrite.builder().openOrCreate(); + collection = db.getCollection("lazy"); + collection.createIndex(indexOptions(NON_UNIQUE), "k"); + collection.createIndex(indexOptions(NON_UNIQUE), "tags"); + for (int i = 0; i < 200; i++) { + collection.insert(Document.createDocument("n", i).put("k", i % 10).put("tags", new String[]{"t" + (i % 3), "x"})); + } + } + + @After + public void tearDown() { + db.close(); + } + + @Test + public void testEqualityResultsCountAndPaging() { + DocumentCursor cursor = collection.find(where("k").eq(3)); + assertEquals(20, cursor.size()); + assertEquals(20, cursor.toList().size()); + assertNotNull(collection.find(where("k").eq(3)).firstOrNull()); + assertEquals(3, collection.find(where("k").eq(3), skipBy(5).limit(3)).toList().size()); + assertEquals(0, collection.find(where("k").eq(42)).size()); + } + + @Test + public void testRangeResultsInIndexOrderBothWays() { + List ascending = collection.find(where("k").between(2, 4)).toList(); + assertEquals(60, ascending.size()); + assertEquals(2, ascending.get(0).get("k", Integer.class).intValue()); + assertEquals(4, ascending.get(ascending.size() - 1).get("k", Integer.class).intValue()); + + List descending = collection.find(where("k").between(2, 4), orderBy("k", SortOrder.Descending)).toList(); + assertEquals(60, descending.size()); + assertEquals(4, descending.get(0).get("k", Integer.class).intValue()); + assertEquals(2, descending.get(descending.size() - 1).get("k", Integer.class).intValue()); + } + + @Test + public void testMultiValuedFieldReturnsEachDocumentOnce() { + assertEquals(200, collection.find(where("tags").eq("x")).size()); + assertEquals(200, collection.find(where("tags").eq("x")).toList().size()); + assertEquals(200, collection.find(where("tags").between("t0", "t9")).size()); + } + + @Test + public void testCountFollowsRemovals() { + collection.remove(where("n").eq(3)); + assertEquals(19, collection.find(where("k").eq(3)).size()); + assertEquals(19, collection.find(where("k").eq(3)).toList().size()); + } + + @Test + public void testShapesOutsideTheLazyPathAreUnchanged() { + assertEquals(40, collection.find(where("k").in(1, 2)).size()); + assertEquals(40, collection.find(where("k").gt(7)).size()); + assertEquals(20, collection.find(where("k").eq(3).and(where("n").lt(200))).size()); + } +} diff --git a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java index 9942dba5..281fc52d 100644 --- a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java @@ -25,6 +25,10 @@ import org.dizitart.no2.common.tuples.Pair; import org.dizitart.no2.filters.ComparableFilter; import org.dizitart.no2.filters.IndexScanFilter; +import org.dizitart.no2.common.RecordStream; +import org.dizitart.no2.store.NitriteStore; +import org.dizitart.no2.store.memory.InMemoryMap; +import org.dizitart.no2.filters.SortingAwareFilter; import org.dizitart.no2.store.NitriteMap; import org.dizitart.no2.store.memory.InMemoryStore; import org.dizitart.no2.exceptions.UniqueConstraintException; @@ -36,6 +40,8 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.Map; +import java.util.HashMap; import static org.dizitart.no2.common.tuples.Pair.pair; import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName; @@ -43,6 +49,10 @@ import static org.dizitart.no2.common.util.IndexUtils.deriveUniqueIndexMapName; import static org.dizitart.no2.filters.FluentFilter.where; import static org.junit.Assert.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.*; public class SingleFieldIndexTest { @Test @@ -191,6 +201,81 @@ public void testReadSortKeysOfUniqueIndex() { assertEquals(Arrays.asList(NitriteId.createId(1L), NitriteId.createId(2L), NitriteId.createId(3L)), keys.stream().map(Pair::getSecond).collect(java.util.stream.Collectors.toList())); assertNull("an index that does not cover every document cannot stand in for it", index.readSortKeys(4)); + public void testLazyStreamMatchesMaterializedScanForEqualityAndRange() { + InMemoryStore store = new InMemoryStore(); + IndexDescriptor desc = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("k"), "c"); + SingleFieldIndex index = new SingleFieldIndex(desc, store); + for (long id = 1; id <= 30; id++) { + index.write(values(id, "k", (int) (id % 5))); // five keys, six ids each + } + index.write(values(31L, "k", new int[]{1, 2, 3})); // one document under three keys + + FindPlan eq = plan(desc, Collections.singletonList((ComparableFilter) where("k").eq(2)), null); + assertEquals(new ArrayList<>(index.findNitriteIds(eq)), index.findNitriteIdStream(eq).toList()); + + List between = Arrays.asList((ComparableFilter) where("k").gte(1), (ComparableFilter) where("k").lt(3)); + FindPlan range = plan(desc, between, null); + assertEquals(new ArrayList<>(index.findNitriteIds(range)), index.findNitriteIdStream(range).toList()); + assertEquals("id 31 is under two keys of the range but returned once", 13, index.findNitriteIdStream(range).toList().size()); + + Map descending = new HashMap<>(); + descending.put("k", true); + FindPlan reversed = plan(desc, between, descending); + assertEquals(new ArrayList<>(index.findNitriteIds(reversed)), index.findNitriteIdStream(reversed).toList()); + // key groups are visited backwards, ids inside a group keep their stored order + List forward = index.findNitriteIdStream(range).toList(); + List backward = index.findNitriteIdStream(reversed).toList(); + assertEquals(NitriteId.createId(1L), forward.get(0)); + assertEquals(NitriteId.createId(2L), backward.get(0)); + assertEquals(forward.size(), backward.size()); + } + + @Test + public void testLazyStreamDeclinesShapesItDoesNotServe() { + InMemoryStore store = new InMemoryStore(); + IndexDescriptor desc = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("k"), "c"); + SingleFieldIndex index = new SingleFieldIndex(desc, store); + index.write(values(1L, "k", 1)); + + assertNull("one-sided range", index.findNitriteIdStream(plan(desc, Collections.singletonList((ComparableFilter) where("k").gt(0)), null))); + assertNull("in filter", index.findNitriteIdStream(plan(desc, Collections.singletonList((ComparableFilter) where("k").in(1, 2)), null))); + assertNull("no scan filter", index.findNitriteIdStream(new FindPlan())); + + IndexDescriptor unique = new IndexDescriptor(IndexType.UNIQUE, Fields.withNames("k"), "c"); + SingleFieldIndex uniqueIndex = new SingleFieldIndex(unique, store); + uniqueIndex.write(values(1L, "k", 1)); + assertNull("unique layout", uniqueIndex.findNitriteIdStream(plan(unique, Collections.singletonList((ComparableFilter) where("k").eq(1)), null))); + } + + @Test + public void testLazyStreamReadsOnlyAsFarAsConsumed() { + IndexDescriptor desc = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("k"), "c"); + InMemoryMap composite = spy(new InMemoryMap<>(deriveCompositeIndexMapName(desc), new InMemoryStore())); + for (long id = 1; id <= 500; id++) { + composite.put(new IndexEntryKey(new DBValue("same"), NitriteId.createId(id)), Boolean.TRUE); + } + NitriteStore store = mock(NitriteStore.class); + when(store.hasMap(anyString())).thenReturn(false); + doReturn(composite).when(store).openMap(eq(deriveCompositeIndexMapName(desc)), any(), any()); + clearInvocations(composite); + + SingleFieldIndex index = new SingleFieldIndex(desc, store); + FindPlan eq = plan(desc, Collections.singletonList((ComparableFilter) where("k").eq("same")), null); + RecordStream stream = index.findNitriteIdStream(eq); + assertNotNull(stream); + + assertEquals(NitriteId.createId(1L), stream.iterator().next()); + verify(composite, atMost(2)).higherKey(any()); + verify(composite, never()).entries(); + assertEquals(500, stream.toList().size()); + } + + private static FindPlan plan(IndexDescriptor desc, List filters, Map scanOrder) { + FindPlan plan = new FindPlan(); + plan.setIndexDescriptor(desc); + plan.setIndexScanFilter(new IndexScanFilter(filters)); + plan.setIndexScanOrder(scanOrder); + return plan; } private static FieldValues values(long id, String field, Object value) { From 7345f50c69df66fb0c95d0e806eac9f09f22d79d Mon Sep 17 00:00:00 2001 From: Anindya Chatterjee Date: Fri, 4 Sep 2026 18:44:48 +0530 Subject: [PATCH 2/7] Style: one declaration and one statement per line in the range parser Codacy's quality gate flagged five new issues on this branch. The switch arms packed an assignment, a flag and a break onto one line, the bounds were declared two to a line, and ArrayDeque was written out fully qualified. Co-Authored-By: Claude Opus 5 --- .../no2/common/streams/IndexedStream.java | 4 --- .../dizitart/no2/index/SingleFieldIndex.java | 35 ++++++++++++------- .../no2/index/SingleFieldIndexTest.java | 3 ++ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java b/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java index 24d65af3..bb937c1b 100644 --- a/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java +++ b/nitrite/src/main/java/org/dizitart/no2/common/streams/IndexedStream.java @@ -24,11 +24,7 @@ import org.dizitart.no2.store.NitriteMap; import java.util.Iterator; -<<<<<<< HEAD import java.util.NoSuchElementException; -import java.util.Set; -======= ->>>>>>> a09dd45e (perf: stream equality and range index scans instead of materializing every id) /** * @author Anindya Chatterjee diff --git a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java index 84479eba..a52cfab0 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java @@ -30,15 +30,17 @@ import org.dizitart.no2.common.RecordStream; import org.dizitart.no2.filters.EqualsFilter; import org.dizitart.no2.filters.SortingAwareFilter; +import org.dizitart.no2.filters.SortingAwareFilter.ComparisonMode; import org.dizitart.no2.store.NitriteMap; import org.dizitart.no2.store.NitriteStore; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.NoSuchElementException; -import java.util.Iterator; import java.util.Set; import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName; @@ -208,8 +210,10 @@ static Range of(List filters) { // a two-sided range on one field, the same shape IndexScanner.scanBoundedRange takes String field = filters.get(0).getField(); - DBValue lower = null, upper = null; - boolean lowerInclusive = false, upperInclusive = false; + DBValue lower = null; + DBValue upper = null; + boolean lowerInclusive = false; + boolean upperInclusive = false; for (ComparableFilter filter : filters) { if (!(filter instanceof SortingAwareFilter) || field == null || !field.equals(filter.getField())) { return null; @@ -219,19 +223,24 @@ static Range of(List filters) { return null; } DBValue key = new DBValue((Comparable) value); - switch (((SortingAwareFilter) filter).getComparisonMode()) { + ComparisonMode mode = ((SortingAwareFilter) filter).getComparisonMode(); + switch (mode) { case GreaterEqual: - if (lower != null) return null; - lower = key; lowerInclusive = true; break; case Greater: - if (lower != null) return null; - lower = key; lowerInclusive = false; break; + if (lower != null) { + return null; + } + lower = key; + lowerInclusive = mode == ComparisonMode.GreaterEqual; + break; case LesserEqual: - if (upper != null) return null; - upper = key; upperInclusive = true; break; case Lesser: - if (upper != null) return null; - upper = key; upperInclusive = false; break; + if (upper != null) { + return null; + } + upper = key; + upperInclusive = mode == ComparisonMode.LesserEqual; + break; default: return null; } @@ -252,7 +261,7 @@ private static final class CompositeRangeIterator implements Iterator private final Range range; private final boolean reverse; private final Set seen = new HashSet<>(); - private final java.util.ArrayDeque group = new java.util.ArrayDeque<>(); + private final ArrayDeque group = new ArrayDeque<>(); private IndexEntryKey key; private NitriteId next; private boolean started; diff --git a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java index 281fc52d..b5bac6b5 100644 --- a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java @@ -201,6 +201,9 @@ public void testReadSortKeysOfUniqueIndex() { assertEquals(Arrays.asList(NitriteId.createId(1L), NitriteId.createId(2L), NitriteId.createId(3L)), keys.stream().map(Pair::getSecond).collect(java.util.stream.Collectors.toList())); assertNull("an index that does not cover every document cannot stand in for it", index.readSortKeys(4)); + } + + @Test public void testLazyStreamMatchesMaterializedScanForEqualityAndRange() { InMemoryStore store = new InMemoryStore(); IndexDescriptor desc = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("k"), "c"); From ee6aff4f3295014f3a097812045c37960ca26d1a Mon Sep 17 00:00:00 2001 From: Anindya Chatterjee Date: Fri, 4 Sep 2026 18:57:02 +0530 Subject: [PATCH 3/7] Style: extract the range seek, wrap the long test lines Codacy still reported three new issues. The seek was a ternary of ternaries spanning 153 characters; it is a named method now, which also gives the four bound cases somewhere to be explained. The rest were long lines in the tests. Co-Authored-By: Claude Opus 5 --- .../dizitart/no2/index/SingleFieldIndex.java | 21 ++++++++++++++++--- .../no2/collection/LazyIndexScanTest.java | 8 +++++-- .../no2/index/SingleFieldIndexTest.java | 18 ++++++++++------ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java index a52cfab0..26e351c7 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java @@ -293,9 +293,7 @@ public NitriteId next() { private void advance() { if (!started) { started = true; - key = reverse - ? (range.upperInclusive ? map.floorKey(IndexEntryKey.upperBound(range.upper)) : map.lowerKey(IndexEntryKey.lowerBound(range.upper))) - : (range.lowerInclusive ? map.ceilingKey(IndexEntryKey.lowerBound(range.lower)) : map.higherKey(IndexEntryKey.upperBound(range.lower))); + key = seek(); } while (true) { while (!group.isEmpty()) { @@ -331,6 +329,23 @@ private void advance() { } } + /** + * The first key of the walk: the entry nearest the bound the walk starts from, on the + * inside of it. A bound's own sentinel key sorts before ({@code lowerBound}) or after + * ({@code upperBound}) every real entry holding that value, which is what turns each + * of the four cases into one navigation call. + */ + private IndexEntryKey seek() { + if (reverse) { + return range.upperInclusive + ? map.floorKey(IndexEntryKey.upperBound(range.upper)) + : map.lowerKey(IndexEntryKey.lowerBound(range.upper)); + } + return range.lowerInclusive + ? map.ceilingKey(IndexEntryKey.lowerBound(range.lower)) + : map.higherKey(IndexEntryKey.upperBound(range.lower)); + } + private boolean within(IndexEntryKey candidate) { if (reverse) { int cmp = candidate.getValue().compareTo(range.lower); diff --git a/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java b/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java index 8ea41b0c..c6e37da0 100644 --- a/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/collection/LazyIndexScanTest.java @@ -48,7 +48,9 @@ public void setUp() { collection.createIndex(indexOptions(NON_UNIQUE), "k"); collection.createIndex(indexOptions(NON_UNIQUE), "tags"); for (int i = 0; i < 200; i++) { - collection.insert(Document.createDocument("n", i).put("k", i % 10).put("tags", new String[]{"t" + (i % 3), "x"})); + collection.insert(Document.createDocument("n", i) + .put("k", i % 10) + .put("tags", new String[]{"t" + (i % 3), "x"})); } } @@ -74,7 +76,9 @@ public void testRangeResultsInIndexOrderBothWays() { assertEquals(2, ascending.get(0).get("k", Integer.class).intValue()); assertEquals(4, ascending.get(ascending.size() - 1).get("k", Integer.class).intValue()); - List descending = collection.find(where("k").between(2, 4), orderBy("k", SortOrder.Descending)).toList(); + List descending = collection + .find(where("k").between(2, 4), orderBy("k", SortOrder.Descending)) + .toList(); assertEquals(60, descending.size()); assertEquals(4, descending.get(0).get("k", Integer.class).intValue()); assertEquals(2, descending.get(descending.size() - 1).get("k", Integer.class).intValue()); diff --git a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java index b5bac6b5..67912e22 100644 --- a/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java +++ b/nitrite/src/test/java/org/dizitart/no2/index/SingleFieldIndexTest.java @@ -216,10 +216,12 @@ public void testLazyStreamMatchesMaterializedScanForEqualityAndRange() { FindPlan eq = plan(desc, Collections.singletonList((ComparableFilter) where("k").eq(2)), null); assertEquals(new ArrayList<>(index.findNitriteIds(eq)), index.findNitriteIdStream(eq).toList()); - List between = Arrays.asList((ComparableFilter) where("k").gte(1), (ComparableFilter) where("k").lt(3)); + List between = Arrays.asList( + (ComparableFilter) where("k").gte(1), (ComparableFilter) where("k").lt(3)); FindPlan range = plan(desc, between, null); assertEquals(new ArrayList<>(index.findNitriteIds(range)), index.findNitriteIdStream(range).toList()); - assertEquals("id 31 is under two keys of the range but returned once", 13, index.findNitriteIdStream(range).toList().size()); + assertEquals("id 31 is under two keys of the range but returned once", + 13, index.findNitriteIdStream(range).toList().size()); Map descending = new HashMap<>(); descending.put("k", true); @@ -240,20 +242,24 @@ public void testLazyStreamDeclinesShapesItDoesNotServe() { SingleFieldIndex index = new SingleFieldIndex(desc, store); index.write(values(1L, "k", 1)); - assertNull("one-sided range", index.findNitriteIdStream(plan(desc, Collections.singletonList((ComparableFilter) where("k").gt(0)), null))); - assertNull("in filter", index.findNitriteIdStream(plan(desc, Collections.singletonList((ComparableFilter) where("k").in(1, 2)), null))); + assertNull("one-sided range", index.findNitriteIdStream( + plan(desc, Collections.singletonList((ComparableFilter) where("k").gt(0)), null))); + assertNull("in filter", index.findNitriteIdStream( + plan(desc, Collections.singletonList((ComparableFilter) where("k").in(1, 2)), null))); assertNull("no scan filter", index.findNitriteIdStream(new FindPlan())); IndexDescriptor unique = new IndexDescriptor(IndexType.UNIQUE, Fields.withNames("k"), "c"); SingleFieldIndex uniqueIndex = new SingleFieldIndex(unique, store); uniqueIndex.write(values(1L, "k", 1)); - assertNull("unique layout", uniqueIndex.findNitriteIdStream(plan(unique, Collections.singletonList((ComparableFilter) where("k").eq(1)), null))); + assertNull("unique layout", uniqueIndex.findNitriteIdStream( + plan(unique, Collections.singletonList((ComparableFilter) where("k").eq(1)), null))); } @Test public void testLazyStreamReadsOnlyAsFarAsConsumed() { IndexDescriptor desc = new IndexDescriptor(IndexType.NON_UNIQUE, Fields.withNames("k"), "c"); - InMemoryMap composite = spy(new InMemoryMap<>(deriveCompositeIndexMapName(desc), new InMemoryStore())); + InMemoryMap composite = spy( + new InMemoryMap<>(deriveCompositeIndexMapName(desc), new InMemoryStore())); for (long id = 1; id <= 500; id++) { composite.put(new IndexEntryKey(new DBValue("same"), NitriteId.createId(id)), Boolean.TRUE); } From 25f46264d029459b7d241544639face3e8093073 Mon Sep 17 00:00:00 2001 From: Anindya Chatterjee Date: Fri, 4 Sep 2026 19:58:10 +0530 Subject: [PATCH 4/7] Split Range.of into its equality and bounded-range halves One method carried both plan shapes and every rejection path for each, which is where the remaining Codacy complexity findings sat. The two shapes have nothing in common but the return type. Co-Authored-By: Claude Opus 5 --- .../dizitart/no2/index/SingleFieldIndex.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java index 26e351c7..4a8f9618 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java @@ -197,18 +197,26 @@ static Range of(List filters) { return null; } if (filters.size() == 1 && filters.get(0).getClass() == EqualsFilter.class) { - Object value = filters.get(0).getValue(); - if (value == null) { - return new Range(DBNull.getInstance(), true, DBNull.getInstance(), true); - } - if (!(value instanceof Comparable)) { - return null; - } - DBValue key = new DBValue((Comparable) value); - return new Range(key, true, key, true); + return ofEquality(filters.get(0)); } + return ofBoundedRange(filters); + } + + /** {@code field = value}, which is the degenerate range with both bounds on that value. */ + private static Range ofEquality(ComparableFilter filter) { + Object value = filter.getValue(); + if (value == null) { + return new Range(DBNull.getInstance(), true, DBNull.getInstance(), true); + } + if (!(value instanceof Comparable)) { + return null; + } + DBValue key = new DBValue((Comparable) value); + return new Range(key, true, key, true); + } - // a two-sided range on one field, the same shape IndexScanner.scanBoundedRange takes + /** A two-sided range on one field, the same shape {@code IndexScanner.scanBoundedRange} takes. */ + private static Range ofBoundedRange(List filters) { String field = filters.get(0).getField(); DBValue lower = null; DBValue upper = null; From 2418a26fe2c7973eae9a995becf1345fc69426e4 Mon Sep 17 00:00:00 2001 From: Anindya Chatterjee Date: Fri, 4 Sep 2026 20:57:00 +0530 Subject: [PATCH 5/7] Flatten findSuitableStream and ofBoundedRange PMD's NPathComplexity, reported by Codacy: findSuitableStream at 450 against a threshold of 200, ofBoundedRange at 228. findSuitableStream was a four-deep if/else doing three unrelated jobs. The by-id and indexed sources are their own methods now, the branch is a flat else-if chain, and the residual filter's condition says what it means - subPlans.isEmpty() && collectionScanFilter != null - instead of leaving it to nesting depth. ofBoundedRange's switch became isLowerBound/isUpperBound. No behaviour change; nitrite 1783 tests pass. Co-Authored-By: Claude Opus 5 --- .../collection/operation/ReadOperations.java | 88 ++++++++++--------- .../dizitart/no2/index/SingleFieldIndex.java | 39 ++++---- 2 files changed, 67 insertions(+), 60 deletions(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java index 852b64cc..c3490a27 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java @@ -206,6 +206,42 @@ private static Object indexedValue(DBValue dbValue) { return dbValue == null || dbValue instanceof DBNull ? null : dbValue.getValue(); } + /** The single row a by-id plan can match, or nothing when that id is not there. */ + private RecordStream> byIdStream(FindPlan findPlan) { + Object idValue = findPlan.getByIdFilter().getValue(); + // the search term may be any numeric or String representation of an id, + // e.g. a String for databases written before 4.4 (gh-1263) + NitriteId nitriteId = idValue instanceof Long + ? NitriteId.createId((long) idValue) + : NitriteId.createId(String.valueOf(idValue)); + // one lookup: a document removed between containsKey and get would be a null row + Document document = nitriteMap.get(nitriteId); + return document == null + ? RecordStream.empty() + : RecordStream.single(pair(nitriteId, document)); + } + + /** + * The rows an index plan matches, lazily where the indexer offers a stream for this plan + * shape, otherwise from the materialized id set, whose size is recorded so a {@code size()} + * with no row-dropping step downstream can answer without fetching a document. + */ + private RecordStream> indexedStream(FindPlan findPlan, IndexScan scan) { + IndexDescriptor indexDescriptor = findPlan.getIndexDescriptor(); + NitriteIndexer indexer = nitriteConfig.findIndexer(indexDescriptor.getIndexType()); + + RecordStream idStream = indexer.findByFilterStream(findPlan, nitriteConfig); + if (idStream != null) { + // the index walks its matches lazily; a size() is counted from it on demand + scan.lazyStream = new IndexedStream(idStream, nitriteMap); + return scan.lazyStream; + } + + LinkedHashSet nitriteIds = indexer.findByFilter(findPlan, nitriteConfig); + scan.idCount = nitriteIds.size(); + return new IndexedStream(nitriteIds, nitriteMap); + } + private RecordStream> findSuitableStream(FindPlan findPlan, IndexScan scan) { RecordStream> rawStream; RecordStream> indexSortedStream = null; @@ -225,50 +261,18 @@ private RecordStream> findSuitableStream(FindPlan find // Always apply distinct stream for OR filters to avoid duplicates // when the same document matches multiple sub-plans (different indexes) rawStream = new DistinctStream(rawStream); + } else if (findPlan.getByIdFilter() != null) { + rawStream = byIdStream(findPlan); + } else if (findPlan.getIndexDescriptor() != null) { + rawStream = indexedStream(findPlan, scan); } else { - // and or single filter - if (findPlan.getByIdFilter() != null) { - FieldBasedFilter byIdFilter = findPlan.getByIdFilter(); - Object idValue = byIdFilter.getValue(); - // the search term may be any numeric or String representation of an id, - // e.g. a String for databases written before 4.4 (gh-1263) - NitriteId nitriteId = idValue instanceof Long - ? NitriteId.createId((long) idValue) - : NitriteId.createId(String.valueOf(idValue)); - // one lookup: a document removed between containsKey and get would be a null row - Document document = nitriteMap.get(nitriteId); - rawStream = document == null - ? RecordStream.empty() - : RecordStream.single(pair(nitriteId, document)); - } else { - IndexDescriptor indexDescriptor = findPlan.getIndexDescriptor(); - if (indexDescriptor != null) { - // get optimized filter - NitriteIndexer indexer = nitriteConfig.findIndexer(indexDescriptor.getIndexType()); - RecordStream idStream = indexer.findByFilterStream(findPlan, nitriteConfig); - if (idStream != null) { - // the index walks its matches lazily; a size() is counted from it on demand - scan.lazyStream = new IndexedStream(idStream, nitriteMap); - rawStream = scan.lazyStream; - } else { - LinkedHashSet nitriteIds = indexer.findByFilter(findPlan, nitriteConfig); - - // the index supplied the exact matching id set; record its size so a size() - // with no row-dropping step downstream can answer from it without fetching - scan.idCount = nitriteIds.size(); - - // create indexed stream from optimized filter - rawStream = new IndexedStream(nitriteIds, nitriteMap); - } - } else { - indexSortedStream = indexSortedStream(findPlan); - rawStream = indexSortedStream != null ? indexSortedStream : nitriteMap.entries(); - } - } + indexSortedStream = indexSortedStream(findPlan); + rawStream = indexSortedStream != null ? indexSortedStream : nitriteMap.entries(); + } - if (findPlan.getCollectionScanFilter() != null) { - rawStream = new FilteredStream(rawStream, findPlan.getCollectionScanFilter()); - } + // an or-plan's branches carry their own filters; only a single plan has a residual one + if (findPlan.getSubPlans().isEmpty() && findPlan.getCollectionScanFilter() != null) { + rawStream = new FilteredStream(rawStream, findPlan.getCollectionScanFilter()); } // sort and bound stage diff --git a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java index 4a8f9618..6bbf67ef 100644 --- a/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java +++ b/nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java @@ -215,6 +215,14 @@ private static Range ofEquality(ComparableFilter filter) { return new Range(key, true, key, true); } + private static boolean isLowerBound(ComparisonMode mode) { + return mode == ComparisonMode.GreaterEqual || mode == ComparisonMode.Greater; + } + + private static boolean isUpperBound(ComparisonMode mode) { + return mode == ComparisonMode.LesserEqual || mode == ComparisonMode.Lesser; + } + /** A two-sided range on one field, the same shape {@code IndexScanner.scanBoundedRange} takes. */ private static Range ofBoundedRange(List filters) { String field = filters.get(0).getField(); @@ -232,25 +240,20 @@ private static Range ofBoundedRange(List filters) { } DBValue key = new DBValue((Comparable) value); ComparisonMode mode = ((SortingAwareFilter) filter).getComparisonMode(); - switch (mode) { - case GreaterEqual: - case Greater: - if (lower != null) { - return null; - } - lower = key; - lowerInclusive = mode == ComparisonMode.GreaterEqual; - break; - case LesserEqual: - case Lesser: - if (upper != null) { - return null; - } - upper = key; - upperInclusive = mode == ComparisonMode.LesserEqual; - break; - default: + if (isLowerBound(mode)) { + if (lower != null) { return null; + } + lower = key; + lowerInclusive = mode == ComparisonMode.GreaterEqual; + } else if (isUpperBound(mode)) { + if (upper != null) { + return null; + } + upper = key; + upperInclusive = mode == ComparisonMode.LesserEqual; + } else { + return null; } } return lower == null || upper == null ? null : new Range(lower, lowerInclusive, upper, upperInclusive); From 9f58759a522425eade7231b0b33a6c362468a972 Mon Sep 17 00:00:00 2001 From: Anindya Chatterjee Date: Fri, 4 Sep 2026 21:00:27 +0530 Subject: [PATCH 6/7] Extract the sort-and-bound stage from findSuitableStream The last of PMD's NPath findings. That tail is a stage every source shares and the code already labelled it as one; it just had no method. Taking "was the source already ordered by the index" as a boolean also says what indexSortedStream was standing in for at that point. nitrite 1783 tests pass. Co-Authored-By: Claude Opus 5 --- .../collection/operation/ReadOperations.java | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java index c3490a27..5fbeb800 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java @@ -275,21 +275,34 @@ private RecordStream> findSuitableStream(FindPlan find rawStream = new FilteredStream(rawStream, findPlan.getCollectionScanFilter()); } - // sort and bound stage - if (rawStream != null) { - // the blocking sort still runs whenever the ordered ids were not used - either no - // index could answer the sort, or the one that could turned out not to cover the - // collection faithfully - if (indexSortedStream == null - && findPlan.getBlockingSortOrder() != null && !findPlan.getBlockingSortOrder().isEmpty()) { - rawStream = new SortedDocumentStream(findPlan, rawStream); - } + return sortAndBound(findPlan, rawStream, indexSortedStream != null); + } - if (findPlan.getLimit() != null || findPlan.getSkip() != null) { - long limit = findPlan.getLimit() == null ? Long.MAX_VALUE : findPlan.getLimit(); - long skip = findPlan.getSkip() == null ? 0 : findPlan.getSkip(); - rawStream = new BoundedStream<>(skip, limit, rawStream); - } + /** + * The stage every source shares: order the rows the index could not order, then cut the + * page out of them. + * + * @param sortedByIndex whether the source already came out in the requested order + */ + private RecordStream> sortAndBound( + FindPlan findPlan, RecordStream> rawStream, boolean sortedByIndex) { + + if (rawStream == null) { + return null; + } + + // the blocking sort still runs whenever the ordered ids were not used - either no + // index could answer the sort, or the one that could turned out not to cover the + // collection faithfully + if (!sortedByIndex && findPlan.getBlockingSortOrder() != null + && !findPlan.getBlockingSortOrder().isEmpty()) { + rawStream = new SortedDocumentStream(findPlan, rawStream); + } + + if (findPlan.getLimit() != null || findPlan.getSkip() != null) { + long limit = findPlan.getLimit() == null ? Long.MAX_VALUE : findPlan.getLimit(); + long skip = findPlan.getSkip() == null ? 0 : findPlan.getSkip(); + rawStream = new BoundedStream<>(skip, limit, rawStream); } return rawStream; From 07ed8e5ce6947a3cef1e0d1b105439487f7eb28a Mon Sep 17 00:00:00 2001 From: Anindya Chatterjee Date: Fri, 4 Sep 2026 21:45:27 +0530 Subject: [PATCH 7/7] Don't reassign sortAndBound's parameter The extraction carried the old method's accumulate-into-rawStream shape into a parameter. A local makes the stage read as what it is: a chain that wraps the source and returns it. Co-Authored-By: Claude Opus 5 --- .../dizitart/no2/collection/operation/ReadOperations.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java index 5fbeb800..2fab9951 100644 --- a/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java +++ b/nitrite/src/main/java/org/dizitart/no2/collection/operation/ReadOperations.java @@ -290,21 +290,22 @@ private RecordStream> sortAndBound( if (rawStream == null) { return null; } + RecordStream> stream = rawStream; // the blocking sort still runs whenever the ordered ids were not used - either no // index could answer the sort, or the one that could turned out not to cover the // collection faithfully if (!sortedByIndex && findPlan.getBlockingSortOrder() != null && !findPlan.getBlockingSortOrder().isEmpty()) { - rawStream = new SortedDocumentStream(findPlan, rawStream); + stream = new SortedDocumentStream(findPlan, stream); } if (findPlan.getLimit() != null || findPlan.getSkip() != null) { long limit = findPlan.getLimit() == null ? Long.MAX_VALUE : findPlan.getLimit(); long skip = findPlan.getSkip() == null ? 0 : findPlan.getSkip(); - rawStream = new BoundedStream<>(skip, limit, rawStream); + stream = new BoundedStream<>(skip, limit, stream); } - return rawStream; + return stream; } }