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

better performance for exists() method of Query class #298

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ziaratban
Copy link
Contributor

@ziaratban ziaratban commented Feb 15, 2020

Q A
Is bugfix? yes
New feature? no
Breaks BC? no
Tests pass? yes
Fixed issues #290

@ziaratban
Copy link
Contributor Author

@samdark what is milestone for this PR ?

@samdark samdark added this to the 2.1.10 milestone Feb 21, 2020
@samdark
Copy link
Member

samdark commented Feb 21, 2020

2.1.10. Overall it looks extremely hacky. Need @yiisoft/core-developers and @yiisoft/reviewers opinions about it.

@schmunk42
Copy link
Contributor

Some more explanation why this is better would be nice.
Did you benchmark this?

Also test do not pass.

@samdark
Copy link
Member

samdark commented Feb 21, 2020

@schmunk42 the explanation is in #290

@schmunk42
Copy link
Contributor

How about

public function exists($db = null, $optimized = false)

and do the modifications when $optimized is true.

@samdark
Copy link
Member

samdark commented Feb 21, 2020

@schmunk42 does it make sense to do it in non-optimized way?

@schmunk42
Copy link
Contributor

@schmunk42 does it make sense to do it in non-optimized way?

I am no mongo expert, my concern is only about BC.

Counterquestion: Why is it not done by mongo, if it makes no difference?

@ziaratban
Copy link
Contributor Author

@schmunk42 does it make sense to do it in non-optimized way?

@samdark please see Checking if a document exists – MongoDB slow findOne vs find

@samdark samdark added the type:enhancement Enhancement label Feb 23, 2020
@samdark
Copy link
Member

samdark commented Feb 23, 2020

The link could be added to comment since the code there causes questions immediately.

@samdark samdark modified the milestones: 2.1.10, 2.1.11 Nov 10, 2020
@samdark samdark modified the milestones: 2.1.11, 2.1.12 Dec 23, 2020
@samdark samdark removed this from the 2.1.12 milestone Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants