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

Improve SchemaManager logic for comparing text indexes #1829

Merged
merged 3 commits into from
Jul 18, 2018

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Jul 18, 2018

Q A
Type bug
BC Break no
Fixed issues #1821

Summary

This is a cherry-pick of #1828 for master and includes various CS fixes. I also added two tests for the loose equality checks for keys/weights.

@jmikola jmikola requested review from alcaeus and malarzm July 18, 2018 16:06
@jmikola jmikola changed the title Gh1821 Improve SchemaManager logic for comparing text indexes Jul 18, 2018
@jmikola jmikola added this to the 2.0.0 milestone Jul 18, 2018
@jmikola jmikola added the Bug label Jul 18, 2018

/* Avoid a strict equality check here. The numeric type returned by
* MongoDB may differ from the document index without implying that the
* indexes themselves are inequivalent. */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #1828 (comment) for additional context.

foreach (['default_language', 'language_override', 'textIndexVersion'] as $option) {
/* Text indexes will always report defaults for these options, so
* only compare if we have explicit values in the document index. */
if (isset($mongoIndex[$option]) && isset($documentIndexOptions[$option]) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issets can be combined to one isset($mongoIndex[$option], $documentIndexOptions[$option])

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had copied this from the ['bits', 'max', 'min'] loop above. Will make the change in both places.

}

/* MongoDB returns the weights sorted by field name, but we'll sort both
* arrays in case that is internal behavior not be be relied upon. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doubled "be" :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I meant to write "to be". Good catch!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also fixed in 1.2.x: 70441ac

jmikola added a commit that referenced this pull request Jul 18, 2018
@jmikola
Copy link
Member Author

jmikola commented Jul 18, 2018

Please ignore the continuous-integration/travis-ci/push build. I inadvertently pushed the branch to the doctrine repository instead of my fork.

@jmikola jmikola merged commit 183805f into doctrine:master Jul 18, 2018
@jmikola jmikola deleted the gh1821 branch July 18, 2018 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants