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

Use BSON::Regexp::Raw over Ruby's Regexp when querying the server #238

Merged
merged 1 commit into from
Mar 30, 2017

Conversation

mzikherman
Copy link
Collaborator

@mzikherman mzikherman commented Mar 29, 2017

This closes #235

As discussed in that issue, and then continued in the discussion mongodb/bson-ruby#71 (comment), it's better to use Regexp::Raw over Ruby's Regexp when querying the server. This allows prefixed expressions to use an index.

However, when querying embedded documents that are already loaded in-memory, we should just be using Ruby's.

This isn't new functionality, so I didn't add any specs besides ensuring the current suite is passing.

However, as mentioned in #235:

Then, is there a way to write a test for this in mongoid-slug (eg. examining the query results and making sure it hit an index or the number of documents scanned was 1 when there're 2 records in the DB for example), and then way to modify the query and remove that option in mongoid-slug?

This seems like an awesome test! And one that is appropriate for this PR, as that's the crux of the change. I'll give that a shot, but wanted to open this PR in the meantime for feedback.

@mongoid-bot
Copy link

mongoid-bot commented Mar 29, 2017

1 Warning
⚠️ There’re library changes, but not tests. That’s OK as long as you’re refactoring existing code.

Generated by 🚫 danger

@mzikherman mzikherman force-pushed the regexp_raw branch 3 times, most recently from 06124ba to 6f5b4df Compare March 29, 2017 17:31
@dblock
Copy link
Collaborator

dblock commented Mar 30, 2017

So does it not work for embedded? or the older versions of Mongoid with Regexp::Raw and why? The case implies that it doesn't, otherwise it would make things much simpler in code, no?

@mzikherman
Copy link
Collaborator Author

Yea, embedded? docs are already loaded in memory and can just use Ruby's Regexp. There's an open PR on Mongoid to bring that in to avoid the special handling for embedded docs.

The older versions of Mongoid bring in earlier versions of the driver (+ BSON gems) that don't include Regexp::Raw, but also don't include the update that makes it not performant to use Regexp on the server.

@dblock dblock merged commit a67332d into mongoid:master Mar 30, 2017
@dblock
Copy link
Collaborator

dblock commented Mar 30, 2017

Thanks @mzikherman. Would you like to make a release? Happy to add you to maintainers + what's your rubygems email?

@mzikherman
Copy link
Collaborator Author

Yea, I'd love that! I am [email protected] on Rubygems

@dblock
Copy link
Collaborator

dblock commented Mar 31, 2017

Added you here and Rubygems @mzikherman. Maybe you can also contribute a RELEASING doc ala https://github.com/mongoid/mongoid-compatibility/blob/master/RELEASING.md?

Btw, you're matt[at]artsymail.com on rubygems...

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.

Very slow slug generation queries with latest BSON and mongo driver
3 participants