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

regression in 0.10.0 compared to 0.9.0 for queryset limit after count #1136

Closed
alon opened this issue Oct 27, 2015 · 3 comments
Closed

regression in 0.10.0 compared to 0.9.0 for queryset limit after count #1136

alon opened this issue Oct 27, 2015 · 3 comments
Assignees
Labels

Comments

@alon
Copy link

alon commented Oct 27, 2015

The following test passes in 0.9.0 but fails in 0.10.0:

client = connect("alon-test")

class Obj(Document):
    pass

[x.delete() for x in Obj.objects()] # remove existing items - just a nicety

[Obj().save() for x in xrange(10)]

assert(2 == len(Obj.objects().limit(2))) # no regression
qs = Obj.objects()
count = qs.count()
assert(2 == len(qs.limit(2).all())) # no regression
print(len(qs.limit(2)))
assert(2 == len(qs.limit(2))) # regression - right hand side returns 10
@amcgregor
Copy link
Contributor

Yup, bit by the same bug. Was quite shocking to have multiple thousands of records that were formerly paginated suddenly not be. ;) What's particularly strange is that I can find no reason for this in the _cursor_args, _initial_query, and _limit attributes of the query pre- and post- count().

Temporarily working around this by cloning: cursor.count() becomes cursor.clone().count()

This problem persists in 0.10.5.

@wojcikstefan
Copy link
Member

Wow, this is an awful regression. Will look at it with high priority. I believe the following should return 2:


In [1]: from mongoengine import *

In [2]: connect('testdb')
Out[2]: MongoClient('localhost', 27017)

In [3]: class Obj(Document):
        pass
   ...:

In [4]: Obj.drop_collection()

In [5]: for _ in range(10):
   ...:     Obj.objects.create()
   ...:

In [6]: Obj.objects.limit(2).count()
Out[6]: 10

@wojcikstefan wojcikstefan self-assigned this Dec 27, 2016
@wojcikstefan
Copy link
Member

wojcikstefan commented Dec 27, 2016

Apparently this was changed in #793 as a fix for #759, which complains that MongoEngine's behavior is inconsistent with PyMongo's behavior of Cursor.count (http://api.mongodb.com/python/current/api/pymongo/cursor.html?highlight=with_limit_and_skip#pymongo.cursor.Cursor.count).

Having tested it, it indeed looks consistent now:

In [1]: from mongoengine import *

In [2]: connect('testdb')
Out[2]: MongoClient(host=['localhost:27017'], document_class=dict, tz_aware=False, connect=True, read_preference=Primary())

In [3]: class User(Document):
        first_name = StringField(max_length=60, required=True)
        last_name = StringField(max_length=60)
   ...:

In [4]: User.objects.create(first_name='John', last_name='Stewart')
Out[4]: <User: User object>

In [5]: User.objects.create(first_name='John', last_name='Oliver')
Out[5]: <User: User object>

In [6]: User.objects.filter(first_name='John').limit(1).count()  # MongoEngine's count
Out[6]: 2

In [7]: User._get_collection().find({'first_name': 'John'}).limit(1).count()  # PyMongo's count
Out[7]: 2

I guess it's not as much a regression as a conscious change of behavior, which was included in the changelog (https://github.com/MongoEngine/mongoengine/pull/793/files#diff-b0508adba031c447ece5a495a136a3d1R8). It should've been communicated in a better fashion, with upgrade docs warning about this breaking change, but overall it seems to work as desired.

Personally, I'd prefer the default with_limit_and_skip to be True, but I guess we should follow PyMongo's lead :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants