Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch offset does not always work #392

Closed
gaborfeher opened this issue Apr 21, 2020 · 1 comment · Fixed by #399
Closed

fetch offset does not always work #392

gaborfeher opened this issue Apr 21, 2020 · 1 comment · Fixed by #399
Assignees
Labels
api: datastore Issues related to the googleapis/python-ndb API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@gaborfeher
Copy link
Contributor

I have an example where offset values larger than 1000 are still handled as if they were only set to 1000. For a demonstration I have reused the same project that I was using for #386 . So I guess the example could be a bit simpler but hopefully it's simple enough: https://github.com/gaborfeher/ndb-testbed/tree/offset-issue

The short summary is that the below 3 queries are yielding the same set of results for me. I've only tried this in a GAE project, not locally.

ProductCopy5.query().fetch(limit=10, offset=1000)
ProductCopy5.query().fetch(limit=10, offset=2000)
ProductCopy5.query().fetch(limit=10, offset=3000)
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Apr 22, 2020
@busunkim96 busunkim96 added the api: datastore Issues related to the googleapis/python-ndb API. label Apr 22, 2020
@gaborfeher
Copy link
Contributor Author

I've managed to simplify my example. It happens both in Datastore and Firestore:

def demo():
    class EmptyStuff(ndb.Model):
        pass

    ndb.put_multi(
        EmptyStuff(key=ndb.Key('EmptyStuff', i+1))
        for i in range(1100))

    q = EmptyStuff.query()
    findings = [
        (i, q.fetch(limit=1, offset=i)[0].key.id())
        for i in range(995, 1006)
    ]
    return str(findings) + '\n'

Expected output:

[(995, 996), (996, 997), (997, 998), (998, 999), (999, 1000), (1000, 1001), (1001, 1001), (1002, 1002), (1003, 1003), (1004, 1004), (1005, 1005)]

Observed output:

[(995, 996), (996, 997), (997, 998), (998, 999), (999, 1000), (1000, 1001), (1001, 1001), (1002, 1001), (1003, 1001), (1004, 1001), (1005, 1001)]

@chrisrossi chrisrossi assigned chrisrossi and unassigned andrewsg Apr 24, 2020
@chrisrossi chrisrossi added api: clouddebugger Issues related to the Stackdriver Debugger API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. and removed triage me I really want to be triaged. api: clouddebugger Issues related to the Stackdriver Debugger API. labels Apr 24, 2020
chrisrossi pushed a commit to chrisrossi/python-ndb that referenced this issue Apr 24, 2020
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 googleapis#392
chrisrossi pushed a commit that referenced this issue Apr 26, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/python-ndb API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants