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

Add index support #54

Merged
merged 2 commits into from Jan 10, 2013
Merged

Add index support #54

merged 2 commits into from Jan 10, 2013

Conversation

cedricfung
Copy link
Contributor

The $elemMatch selector won't use multiKey index.

For issue#45, Ruby 1.9 is now more popular than 1.8, so we shouldn't
sacrify performance for it. If we want to support 1.8, consider
OrderedHash from ActiveSupport

The $elemMatch selector won't use multiKey index.

For issue#45, Ruby 1.9 is now more popular than 1.8, so we shouldn't
sacrify performance for it. If we want to support 1.8, consider
OrderedHash from ActiveSupport
@dblock
Copy link
Collaborator

dblock commented Jan 10, 2013

Are you saying this will break Ruby 1.8.x? It looks like it. In which case we can maybe do a programmatic if RUBY_VERSION?

@cedricfung
Copy link
Contributor Author

@dblock Indeed the OrderedHash from ActiveSupport is doing this, we can use OrderedHash if don't mind another dependence.

@dblock
Copy link
Collaborator

dblock commented Jan 10, 2013

I don't mind another dependency. Go for it. We can't break existing users though, even if they are stuck in the 20th century Ruby :)

@cedricfung
Copy link
Contributor Author

So we have three simple choices

  1. Create different branches and versions for 1.8.x and 1.9+
  2. Use $elemMatch for 1.8.x
  3. Use OrderedHash

Maybe someone find better solutions.

@cedricfung
Copy link
Contributor Author

OK, I will try an OrderedHash commit.

@cedricfung
Copy link
Contributor Author

Ruby 1.8.x is supported, all specs are green.

I found ActiveSupport is already the dependecy with ActiveSupport::Concern

@dblock
Copy link
Collaborator

dblock commented Jan 10, 2013

Ok, thanks, merging.

dblock added a commit that referenced this pull request Jan 10, 2013
@dblock dblock merged commit 5512c41 into mongoid:master Jan 10, 2013
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.

None yet

2 participants