Skip to content

Fix #607 exists() bug and assure compatibility with Laravel 5.1.19 #620

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

Merged
merged 1 commit into from
Oct 15, 2015
Merged

Conversation

alexandre-butynski
Copy link
Contributor

Laravel 5.1.19 version breaks the exists() method (see #607) because of a complete rewriting of the Laravel base method.

I think that the method should be rewrite in this package. This implementation is close to the best in term of performance but a small improvement can be done by using db.collection.find().limit(1)->count(1) but I don't know how to use the count method like that (if there is no argument, count() don't care about limit()).

If this pull request is accepted, a new release would be appreciated :)

@jenssegers
Copy link
Contributor

Can't we use the same logic that Laravel had before instead of calling first()? Probably more efficient to force limit(1) on the exists query.

@alexandre-butynski
Copy link
Contributor Author

$this->first() is faster than $this->limit(1)->count() because count() method ignores the effect of limit() method. A quick test on a 2 000 documents dataset shows a request 6 times faster.

See this doc page for more informations : http://docs.mongodb.org/manual/reference/method/cursor.count. The best performance would be reached by setting applySkipLimit to true to the cursor count() method but a simple find() is at least better than the previous solution.

@jenssegers
Copy link
Contributor

Ok I will merge this until we find actual complications :)

jenssegers added a commit that referenced this pull request Oct 15, 2015
Fix #607 exists() bug and assure compatibility with Laravel 5.1.19
@jenssegers jenssegers merged commit 979dc80 into mongodb:master Oct 15, 2015
@alexandre-butynski
Copy link
Contributor Author

Thanks.

Maybe did I have made this PR in the wrong branch but this should be integrated in the 2.2 version of the package too !

mnphpexpert added a commit to mnphpexpert/laravel-mongodb that referenced this pull request Sep 2, 2024
Fix mongodb#607 exists() bug and assure compatibility with Laravel 5.1.19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants