Firestore client library's limit API not handling limit(0) correctly #2461
Description
Please make sure you have searched for information in the following guides.
- Search the issues already opened: https://github.com/GoogleCloudPlatform/google-cloud-node/issues
- Search StackOverflow: http://stackoverflow.com/questions/tagged/google-cloud-platform+node.js
- Check our Troubleshooting guide: https://github.com/googleapis/google-cloud-node/blob/main/docs/troubleshooting.md
- Check our FAQ: https://github.com/googleapis/google-cloud-node/blob/main/docs/faq.md
- Check our libraries HOW-TO: https://github.com/googleapis/gax-nodejs/blob/main/client-libraries.md
- Check out our authentication guide: https://github.com/googleapis/google-auth-library-nodejs
- Check out handwritten samples for many of our APIs: https://github.com/GoogleCloudPlatform/nodejs-docs-samples
A screenshot that you have tested with "Try this API".
Not applicable
Link to the code that reproduces this issue. A link to a public Github Repository or gist with a minimal reproduction.
A step-by-step description of how to reproduce the issue, based on the linked reproduction.
The link includes a regression test that is expected to fail — it demonstrates the current buggy behaviour.
Another way to reproduce this (which is how I originally ran into the issue) is to spin up a local Firestore emulator, write a unit test that inserts several documents, and then query with limit(0). You’ll see that the result is unexpectedly non-empty.
A clear and concise description of what the bug is, and what you expected to happen.
When users of the query library call limit(0), the client does not actually include the limit in the request protos.
A clear and concise description WHY you expect this behavior, i.e., was it a recent change, there is documentation that points to this behavior, etc. **
This looks like an oversight. Semantically, limit(0) is a valid query. There’s also validation logic in the limit implementation that explicitly does not rule out 0 as a value:
nodejs-firestore/dev/src/reference/query.ts
Line 475 in a0c74b7
One could argue that limit(0) should simply behave as if no limit were applied. However, if that’s the intended behaviour, the validation logic should likely reject 0 values, and the library should warn users about this surprising behaviour when 0 is provided.