Skip to content

Commit 792f8ca

Browse files
author
Chris Rossi
authored
fix: Correctly handle limit and offset when batching query results. (#237)
Fixes #236. It was found that when a result set spanned multiple batches, we weren't updating `limit` and `offset` on subsequent queries. Now we do.
1 parent 9e95874 commit 792f8ca

File tree

3 files changed

+68
-1
lines changed

3 files changed

+68
-1
lines changed

packages/google-cloud-ndb/google/cloud/ndb/_datastore_query.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,11 @@ def _next_batch(self):
311311

312312
if more_results:
313313
# Fix up query for next batch
314+
limit = self._query.limit
315+
if limit is not None:
316+
limit -= len(self._batch)
314317
self._query = self._query.copy(
315-
start_cursor=Cursor(batch.end_cursor)
318+
start_cursor=Cursor(batch.end_cursor), offset=None, limit=limit
316319
)
317320

318321
def next(self):

packages/google-cloud-ndb/tests/system/test_query.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,33 @@ def make_entities():
9696
assert [entity.foo for entity in results][:5] == [0, 1, 2, 3, 4]
9797

9898

99+
@pytest.mark.usefixtures("client_context")
100+
def test_high_limit(dispose_of):
101+
"""Regression test for Issue #236
102+
103+
https://github.com/googleapis/python-ndb/issues/236
104+
"""
105+
n_entities = 500
106+
107+
class SomeKind(ndb.Model):
108+
foo = ndb.IntegerProperty()
109+
110+
@ndb.toplevel
111+
def make_entities():
112+
entities = [SomeKind(foo=i) for i in range(n_entities)]
113+
keys = yield [entity.put_async() for entity in entities]
114+
raise ndb.Return(keys)
115+
116+
for key in make_entities():
117+
dispose_of(key._key)
118+
119+
query = SomeKind.query()
120+
eventually(query.fetch, _length_equals(n_entities))
121+
results = query.fetch(limit=400)
122+
123+
assert len(results) == 400
124+
125+
99126
@pytest.mark.usefixtures("client_context")
100127
def test_fetch_and_immediately_cancel(dispose_of):
101128
# Make a lot of entities so the query call won't complete before we get to

packages/google-cloud-ndb/tests/unit/test__datastore_query.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,43 @@ def test__next_batch_has_more(_datastore_run_query):
356356
assert iterator._has_next_batch
357357
assert iterator._query.start_cursor.cursor == b"abc"
358358

359+
@staticmethod
360+
@pytest.mark.usefixtures("in_context")
361+
@mock.patch("google.cloud.ndb._datastore_query._datastore_run_query")
362+
def test__next_batch_has_more_w_offset_and_limit(_datastore_run_query):
363+
"""Regression test for Issue #236
364+
365+
https://github.com/googleapis/python-ndb/issues/236
366+
"""
367+
entity_results = [
368+
mock.Mock(entity="entity1", cursor=b"a"),
369+
mock.Mock(entity="entity2", cursor=b"b"),
370+
mock.Mock(entity="entity3", cursor=b"c"),
371+
]
372+
_datastore_run_query.return_value = utils.future_result(
373+
mock.Mock(
374+
batch=mock.Mock(
375+
entity_result_type=query_pb2.EntityResult.FULL,
376+
entity_results=entity_results,
377+
end_cursor=b"abc",
378+
more_results=query_pb2.QueryResultBatch.NOT_FINISHED,
379+
)
380+
)
381+
)
382+
383+
query = query_module.QueryOptions(offset=5, limit=5)
384+
iterator = _datastore_query._QueryIteratorImpl(query)
385+
assert iterator._next_batch().result() is None
386+
assert iterator._index == 0
387+
assert len(iterator._batch) == 3
388+
assert iterator._batch[0].result_pb.entity == "entity1"
389+
assert iterator._batch[0].result_type == query_pb2.EntityResult.FULL
390+
assert iterator._batch[0].order_by is None
391+
assert iterator._has_next_batch
392+
assert iterator._query.start_cursor.cursor == b"abc"
393+
assert iterator._query.offset is None
394+
assert iterator._query.limit == 2
395+
359396
@staticmethod
360397
def test_next_done():
361398
iterator = _datastore_query._QueryIteratorImpl("foo")

0 commit comments

Comments
 (0)