Skip to content

Commit 759f79f

Browse files
author
Chris Rossi
authored
fix: use skipped_results from query results to adjust offset (#399)
Fixes a bug where we blithely assumed that if we sent Datastore/Firestore a query with an offset, that the first batch returned would skip the entire offset. In practice, for high offsets, it's possible for Datastore/Firestore to return a results batch that is empty and which has `skipped_results` set to some number less than the value of `offset` that we sent it. In this case, we still need to send a value for `offset` when retreiving the next batch. This patch uses `skipped_results` to compute a new `offset` for the follow up batch. Fixes #392
1 parent de64eac commit 759f79f

File tree

3 files changed

+33
-2
lines changed

3 files changed

+33
-2
lines changed

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,8 +316,15 @@ def _next_batch(self):
316316
limit = self._query.limit
317317
if limit is not None:
318318
limit -= len(self._batch)
319+
320+
offset = self._query.offset
321+
if offset:
322+
offset -= response.batch.skipped_results
323+
319324
self._query = self._query.copy(
320-
start_cursor=Cursor(batch.end_cursor), offset=None, limit=limit
325+
start_cursor=Cursor(batch.end_cursor),
326+
offset=offset,
327+
limit=limit,
321328
)
322329

323330
def next(self):

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1752,3 +1752,26 @@ class SomeKind(ndb.Model):
17521752
assert results[0].foo == ndb.key.Key(
17531753
"test_key", 3, project=project, namespace=namespace
17541754
)
1755+
1756+
1757+
@pytest.mark.usefixtures("client_context")
1758+
def test_high_offset(dispose_of):
1759+
"""Regression test for Issue #392
1760+
1761+
https://github.com/googleapis/python-ndb/issues/392
1762+
"""
1763+
n_entities = 1100
1764+
1765+
class SomeKind(ndb.Model):
1766+
foo = ndb.IntegerProperty()
1767+
1768+
entities = [SomeKind(id=i + 1, foo=i) for i in range(n_entities)]
1769+
keys = ndb.put_multi(entities)
1770+
for key in keys:
1771+
dispose_of(key._key)
1772+
1773+
eventually(SomeKind.query().fetch, _length_equals(n_entities))
1774+
query = SomeKind.query(order_by=[SomeKind.foo])
1775+
index = n_entities - 5
1776+
result = query.fetch(offset=index, limit=1)[0]
1777+
assert result.foo == index

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ def test__next_batch_has_more_w_offset_and_limit(_datastore_run_query):
393393
entity_result_type=query_pb2.EntityResult.FULL,
394394
entity_results=entity_results,
395395
end_cursor=b"abc",
396+
skipped_results=5,
396397
more_results=query_pb2.QueryResultBatch.NOT_FINISHED,
397398
)
398399
)
@@ -408,7 +409,7 @@ def test__next_batch_has_more_w_offset_and_limit(_datastore_run_query):
408409
assert iterator._batch[0].order_by is None
409410
assert iterator._has_next_batch
410411
assert iterator._query.start_cursor.cursor == b"abc"
411-
assert iterator._query.offset is None
412+
assert iterator._query.offset == 0
412413
assert iterator._query.limit == 2
413414

414415
@staticmethod

0 commit comments

Comments
 (0)