Skip to content
This repository was archived by the owner on May 7, 2026. It is now read-only.

fix: Address queries not fully satisfying requested offset - #18

Merged
crwilcox merged 8 commits into
masterfrom
large_offsets
Apr 7, 2020
Merged

fix: Address queries not fully satisfying requested offset#18
crwilcox merged 8 commits into
masterfrom
large_offsets

Conversation

@crwilcox

@crwilcoxcrwilcox commented Mar 31, 2020

Copy link
Copy Markdown
Contributor

@crwilcoxcrwilcox added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 31, 2020
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 31, 2020
@crwilcox

crwilcox commented Mar 31, 2020

Copy link
Copy Markdown
ContributorAuthor

Test Added. Not really confirming the case. Will need to further flesh out.

def test_query_paginate_with_large_offset(self):
page_query = self._base_query()
page_query.order = "appearances"
offset = 1000 # This is greater than the number of entries.
limit = 3
# Offset beyond items. Verify no items found
iterator = page_query.fetch(limit=limit, offset=offset)
page = six.next(iterator.pages)
entities = list(page)
self.assertEqual(len(entities), 0)
# Offset beyond items, explicit start_cursor
new_iterator = page_query.fetch(limit=limit, offset=offset, start_cursor=None)
entities = list(new_iterator)
self.assertEqual(len(entities), 0)
# Offset beyond items, use cursor one item in.
new_iterator = page_query.fetch(limit=1, offset=0)
cursor = new_iterator.next_page_token
new_iterator = page_query.fetch(limit=limit, offset=offset, start_cursor=cursor)
entities = list(new_iterator)
self.assertEqual(len(entities), 0)

@crwilcoxcrwilcox changed the title fix: datastore client not properly respecting limits and offsets.WIP: fix: datastore client not properly respecting limits and offsets.Apr 2, 2020
@crwilcox
crwilcoxforce-pushed the large_offsets branch 2 times, most recently from 76817ee to 992d8adCompareApril 6, 2020 21:31
@crwilcoxcrwilcox changed the title WIP: fix: datastore client not properly respecting limits and offsets.fix: datastore client not properly respecting limits and offsets.Apr 6, 2020
@crwilcoxcrwilcox removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 6, 2020
@crwilcoxcrwilcox changed the title fix: datastore client not properly respecting limits and offsets.fix: datastore client not properly respecting limits and offsetsApr 6, 2020
@crwilcoxcrwilcox changed the title fix: datastore client not properly respecting limits and offsetsfix: Address queries not fully satisfying requested offsetApr 6, 2020
@crwilcox

Copy link
Copy Markdown
ContributorAuthor

10 tests fail both with and without my changes

FAILED tests/system/test_system.py::TestDatastoreQuery::test_ancestor_query - AssertionError: 4 != 8
FAILED tests/system/test_system.py::TestDatastoreQuery::test_limit_queries - AssertionError: 4 != 5
FAILED tests/system/test_system.py::TestDatastoreQuery::test_ordered_query - AssertionError: 4 != 8
FAILED tests/system/test_system.py::TestDatastoreQuery::test_projection_query - AssertionError: 4 != 9
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_distinct_on - AssertionError: 1 != 2
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_key_filter - AssertionError: 0 != 1
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_multiple_filters - AssertionError: 3 != 4
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_paginate_with_offset - AssertionError: 2 != 3
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_paginate_with_start_cursor - AssertionError: 2 != 3
FAILED tests/system/test_system.py::TestDatastoreQuery::test_query_simple_filter - AssertionError: 4 != 6

@crwilcoxcrwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 7, 2020
@crwilcox

Copy link
Copy Markdown
ContributorAuthor

Update: after refreshing test dataset, all tests are passing.

@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 7, 2020
@crwilcox
crwilcox merged commit e7b5fc9 into masterApr 7, 2020
@crwilcox

Copy link
Copy Markdown
ContributorAuthor

Bypassing kokoro. Only failures are lint and docs. On areas not touched by this change.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@crwilcox@busunkim96@googlebot@yoshi-kokoro