Skip to content

Commit e475ea8

Browse files
anidotnetclaude
andcommitted
Cover the paging shapes a source-level skip has to decline
From gh-1283, which proposed pushing the offset down per plan and named these as the cases to exclude. The skip landed differently - BoundedStream asks the iterator whether it can seek, so a stream that filters or reorders simply does not offer it - but the shapes are worth holding either way, because getting them wrong is a page that quietly starts on the wrong row rather than one that runs slowly: an OR plan unions its sub-plans, a removal leaves the natural order with holes, and a filter and an order answered from the same index must not seek twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2c65442 commit e475ea8

2 files changed

Lines changed: 116 additions & 0 deletions

File tree

nitrite-mvstore-adapter/src/test/java/org/dizitart/no2/integration/collection/CollectionPagingTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,62 @@ private long timePage(int skip) {
187187
assertEquals(PAGE, rows);
188188
return elapsed;
189189
}
190+
191+
/**
192+
* The plan shapes the seek must decline, and one it must not get wrong.
193+
*
194+
* <p>A storage-level skip is only the same thing as the pipeline's skip when nothing between
195+
* the source and the page drops or reorders rows. An OR plan unions its sub-plans and a
196+
* removal leaves the natural order with holes in it, so both are places where an offset taken
197+
* at the source would answer with the wrong rows rather than merely slowly. From
198+
* <a href="https://github.com/nitrite/nitrite-java/pull/1283">gh-1283</a>, which proposed
199+
* pushing the offset down per plan and named these as the cases to exclude.
200+
*/
201+
@Test
202+
public void anOrFilterPagesTheSameWay() {
203+
seed();
204+
collection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "index");
205+
assertPagingMatchesFullScan(
206+
org.dizitart.no2.filters.Filter.or(where("index").lt(50), where("index").gte(450)),
207+
null, 17);
208+
}
209+
210+
@Test
211+
public void pagingAfterRemovalsPagesTheSameWay() {
212+
seed();
213+
collection.remove(where("index").lt(100));
214+
assertPagingMatchesFullScan(ALL, null, 23);
215+
}
216+
217+
/** A filter and an order both answered from the same index - the seek must not double up. */
218+
@Test
219+
public void anIndexedFilterOrderedByThatIndexPagesTheSameWay() {
220+
seed();
221+
collection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "index");
222+
assertPagingMatchesFullScan(where("index").gte(100),
223+
org.dizitart.no2.common.SortOrder.Ascending, 19);
224+
}
225+
226+
private void assertPagingMatchesFullScan(org.dizitart.no2.filters.Filter filter,
227+
org.dizitart.no2.common.SortOrder order,
228+
int pageSize) {
229+
FindOptions whole = order == null ? null : FindOptions.orderBy("index", order);
230+
List<Object> expected = new ArrayList<>();
231+
for (Document document : whole == null
232+
? collection.find(filter) : collection.find(filter, whole)) {
233+
expected.add(document.get("index"));
234+
}
235+
assertTrue("the fixture must return rows for this to prove anything", !expected.isEmpty());
236+
237+
List<Object> paged = new ArrayList<>();
238+
for (int offset = 0; offset < expected.size(); offset += pageSize) {
239+
FindOptions page = order == null
240+
? FindOptions.skipBy(offset).limit(pageSize)
241+
: FindOptions.orderBy("index", order).skip((long) offset).limit((long) pageSize);
242+
for (Document document : collection.find(filter, page)) {
243+
paged.add(document.get("index"));
244+
}
245+
}
246+
assertEquals(expected, paged);
247+
}
190248
}

nitrite-rocksdb-adapter/src/test/java/org/dizitart/no2/integration/collection/CollectionPagingTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,62 @@ private long timePage(int skip) {
187187
assertEquals(PAGE, rows);
188188
return elapsed;
189189
}
190+
191+
/**
192+
* The plan shapes the seek must decline, and one it must not get wrong.
193+
*
194+
* <p>A storage-level skip is only the same thing as the pipeline's skip when nothing between
195+
* the source and the page drops or reorders rows. An OR plan unions its sub-plans and a
196+
* removal leaves the natural order with holes in it, so both are places where an offset taken
197+
* at the source would answer with the wrong rows rather than merely slowly. From
198+
* <a href="https://github.com/nitrite/nitrite-java/pull/1283">gh-1283</a>, which proposed
199+
* pushing the offset down per plan and named these as the cases to exclude.
200+
*/
201+
@Test
202+
public void anOrFilterPagesTheSameWay() {
203+
seed();
204+
collection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "index");
205+
assertPagingMatchesFullScan(
206+
org.dizitart.no2.filters.Filter.or(where("index").lt(50), where("index").gte(450)),
207+
null, 17);
208+
}
209+
210+
@Test
211+
public void pagingAfterRemovalsPagesTheSameWay() {
212+
seed();
213+
collection.remove(where("index").lt(100));
214+
assertPagingMatchesFullScan(ALL, null, 23);
215+
}
216+
217+
/** A filter and an order both answered from the same index - the seek must not double up. */
218+
@Test
219+
public void anIndexedFilterOrderedByThatIndexPagesTheSameWay() {
220+
seed();
221+
collection.createIndex(IndexOptions.indexOptions(IndexType.NON_UNIQUE), "index");
222+
assertPagingMatchesFullScan(where("index").gte(100),
223+
org.dizitart.no2.common.SortOrder.Ascending, 19);
224+
}
225+
226+
private void assertPagingMatchesFullScan(org.dizitart.no2.filters.Filter filter,
227+
org.dizitart.no2.common.SortOrder order,
228+
int pageSize) {
229+
FindOptions whole = order == null ? null : FindOptions.orderBy("index", order);
230+
List<Object> expected = new ArrayList<>();
231+
for (Document document : whole == null
232+
? collection.find(filter) : collection.find(filter, whole)) {
233+
expected.add(document.get("index"));
234+
}
235+
assertTrue("the fixture must return rows for this to prove anything", !expected.isEmpty());
236+
237+
List<Object> paged = new ArrayList<>();
238+
for (int offset = 0; offset < expected.size(); offset += pageSize) {
239+
FindOptions page = order == null
240+
? FindOptions.skipBy(offset).limit(pageSize)
241+
: FindOptions.orderBy("index", order).skip((long) offset).limit((long) pageSize);
242+
for (Document document : collection.find(filter, page)) {
243+
paged.add(document.get("index"));
244+
}
245+
}
246+
assertEquals(expected, paged);
247+
}
190248
}

0 commit comments

Comments
 (0)