Skip to content

Commit 35d87df

Browse files
Handle the zero limit case explicitly for MongoDB aggregation (#187)
1 parent 17c84c4 commit 35d87df

File tree

5 files changed

+68
-5
lines changed

5 files changed

+68
-5
lines changed

document-store/src/integrationTest/java/org/hypertrace/core/documentstore/DocStoreQueryV1Test.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,6 +2053,17 @@ public void testAggregateWithSingleKey(final String datastoreName) throws IOExce
20532053
}
20542054
}
20552055

2056+
@ParameterizedTest
2057+
@ArgumentsSource(AllProvider.class)
2058+
public void testAggregateWithZeroLimitAndOffset(final String datastoreName) throws IOException {
2059+
final Collection collection = getCollection(datastoreName);
2060+
2061+
final Iterator<Document> resultDocs =
2062+
collection.aggregate(
2063+
Query.builder().setPagination(Pagination.builder().limit(0).offset(0).build()).build());
2064+
assertDocsAndSizeEqual(datastoreName, resultDocs, "query/empty_response.json", 0);
2065+
}
2066+
20562067
@Nested
20572068
class AtomicUpdateTest {
20582069
@ParameterizedTest

document-store/src/main/java/org/hypertrace/core/documentstore/mongo/query/MongoPaginationHelper.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ public class MongoPaginationHelper {
1212
private static final String LIMIT_CLAUSE = "$limit";
1313

1414
static BasicDBObject getSkipClause(final Query query) {
15-
Optional<Pagination> paginationOptional = query.getPagination();
15+
Optional<Pagination> paginationOptional =
16+
query.getPagination().filter(pagination -> pagination.getOffset() > 0);
1617
return paginationOptional
1718
.map(pagination -> new BasicDBObject(SKIP_CLAUSE, pagination.getOffset()))
1819
.orElse(new BasicDBObject());
1920
}
2021

2122
static BasicDBObject getLimitClause(final Query query) {
22-
Optional<Pagination> paginationOptional = query.getPagination();
23+
Optional<Pagination> paginationOptional =
24+
query.getPagination().filter(pagination -> pagination.getLimit() > 0);
2325
return paginationOptional
2426
.map(pagination -> new BasicDBObject(LIMIT_CLAUSE, pagination.getLimit()))
2527
.orElse(new BasicDBObject());

document-store/src/main/java/org/hypertrace/core/documentstore/mongo/query/MongoQueryExecutor.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818

1919
import com.mongodb.BasicDBObject;
2020
import com.mongodb.MongoCommandException;
21+
import com.mongodb.ServerAddress;
22+
import com.mongodb.ServerCursor;
2123
import com.mongodb.client.AggregateIterable;
2224
import com.mongodb.client.FindIterable;
2325
import com.mongodb.client.MongoCursor;
2426
import java.util.Collection;
2527
import java.util.List;
28+
import java.util.NoSuchElementException;
2629
import java.util.function.Function;
2730
import java.util.stream.Collectors;
2831
import java.util.stream.Stream;
@@ -49,6 +52,47 @@ public class MongoQueryExecutor {
4952
query -> singleton(getSkipClause(query)),
5053
query -> singleton(getLimitClause(query)));
5154

55+
private static final Integer ZERO = Integer.valueOf(0);
56+
private static final MongoCursor<BasicDBObject> EMPTY_CURSOR =
57+
new MongoCursor<>() {
58+
@Override
59+
public void close() {
60+
// Do nothing
61+
}
62+
63+
@Override
64+
public boolean hasNext() {
65+
return false;
66+
}
67+
68+
@Override
69+
public BasicDBObject next() {
70+
throw new NoSuchElementException();
71+
}
72+
73+
@Override
74+
public int available() {
75+
return 0;
76+
}
77+
78+
@Override
79+
public BasicDBObject tryNext() {
80+
throw new NoSuchElementException();
81+
}
82+
83+
@Override
84+
public ServerCursor getServerCursor() {
85+
// It is okay to throw exception since we are never invoking this method
86+
throw new UnsupportedOperationException();
87+
}
88+
89+
@Override
90+
public ServerAddress getServerAddress() {
91+
// It is okay to throw exception since we are never invoking this method
92+
throw new UnsupportedOperationException();
93+
}
94+
};
95+
5296
private final com.mongodb.client.MongoCollection<BasicDBObject> collection;
5397

5498
public MongoCursor<BasicDBObject> find(final Query query) {
@@ -70,6 +114,11 @@ public MongoCursor<BasicDBObject> find(final Query query) {
70114
}
71115

72116
public MongoCursor<BasicDBObject> aggregate(final Query originalQuery) {
117+
if (originalQuery.getPagination().map(Pagination::getLimit).map(ZERO::equals).orElse(false)) {
118+
log.debug("Not executing query because of a 0 limit");
119+
return EMPTY_CURSOR;
120+
}
121+
73122
Query query = transformAndLog(originalQuery);
74123

75124
List<BasicDBObject> pipeline =

document-store/src/main/java/org/hypertrace/core/documentstore/query/Pagination.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public static class PaginationBuilder {
1717
public Pagination build() {
1818
Preconditions.checkArgument(limit != null, "limit is null");
1919
Preconditions.checkArgument(offset != null, "offset is null");
20+
21+
Preconditions.checkArgument(limit >= 0, "limit must be non-negative");
22+
Preconditions.checkArgument(offset >= 0, "offset must be non-negative");
23+
2024
return new Pagination(limit, offset);
2125
}
2226
}

document-store/src/test/resources/mongo/pipeline/with_pagination.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@
66
}
77
}
88
},
9-
{
10-
"$skip": 0
11-
},
129
{
1310
"$limit": 10
1411
}

0 commit comments

Comments
 (0)