-
Notifications
You must be signed in to change notification settings - Fork 232
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
Test the limitNum key when appending the limit #286
Conversation
In the current implementation, it is possible that invalid queries are displayed in the profiler: ```php use myDb; db.User.find({ "$and": [ { "_id": ObjectId("54ef2e9780c7e11e188b49ab") }, { "deletedAt": null } ] }).limit(1).limit(); db.Group.find({ "_id": { "$in": [ ObjectId("54ef2e9b80c7e11e188b4c5d") ] } }).sort([ ]); db.Town.find({ "_id": ObjectId("54ef2e9280c7e11e188b4589") }).limit(1).limit(); ``` With the suggested change, this result in: ```php use myDb; db.User.find({ "$and": [ { "_id": ObjectId("54ef2e9780c7e11e188b49ab") }, { "deletedAt": null } ] }).limit(1); db.Group.find({ "_id": { "$in": [ ObjectId("54ef2e9b80c7e11e188b4c5d") ] } }).sort([ ]); db.Town.find({ "_id": ObjectId("54ef2e9280c7e11e188b4589") }).limit(1); ```
👍 Noticed the same, quite annoying to be misled by the Profiler |
@coudenysj could you maybe add a test for this? |
Signed-off-by: Jachim Coudenys <[email protected]>
I've added a unit test with data from my project, but I see the problem is not in the |
@malarzm Is this normal behaviour? For one specific query, I get the following data:
|
@coudenysj what's concerning you? |
@malarzm It is not concerning me :). I was just wondering if this query logging is correct. If so, my unit test is correct an this pull request is complete :). |
@malarzm Does this PR needs any changes? |
@coudenysj I'm sorry, I was totally into ODM lately. As far as I'm concerned PR looks good but please let me revisit this and other PRs here soon. I've set milestone to not forget about it :) |
Merged manually into 3.0 branch in 3a7419f. Thanks! |
In the current implementation, it is possible that invalid queries are displayed in the profiler:
With the suggested change, this results in: