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

union operation in find method, similar to the IN Operator in SQL #74

Closed
wants to merge 6 commits into from

Conversation

joker-777
Copy link
Contributor

Hi

I always wanted s.th. similar like the SQL Operatorfor 'IN' for ohm.

I've implemented it now for Model.find and I was wondering if you have any thoughts about it?

In our case we have 4 slugs and instead of doing this:

[slug1, slug2, slug3, slug4].each do
  object = Model.find(slug: slug, user_id: some_id)
  ...
end

We wanna do

Model.find(slug: [slug1, slug2, slug3, slug4], user_id: some_id).each do |object|
  ...
end

Could there be some performance issues? What would be more performant?
I didn't run any test yet nor didn't write any. I first would like to get some other views about this subject.

Thanks

@cyx
Copy link
Collaborator

cyx commented Oct 5, 2012

Hello,

Due to my nature to want to be able to control these sort of stuff (and most of the time use cases like these
appear in some kind of performant-requiring context), I tend to hand code these.

Here's an example of a simple enough implementation: https://gist.github.com/d05e53cc3f1ae7623a8e

@gingerlime
Copy link

@cyx - thanks for the snippet. It is quite simple as you said, but still requires much more familiarity with the inner mechanisms of both ohm and redis.

I'm working with @joker-777 so perhaps I'm biased, but I think that from an ohm end-user's perspective (in my case, a rather inexperienced one), using find with an array makes it much more similar to other ORMs and much more intuitive. If this doesn't break anything, I think it could be a very elegant and useful addition to ohm.

@joker-777
Copy link
Contributor Author

Hi @cyx

Thanks for your answer. I think similar to @gingerlime.
Just finished it up, fixed some tests and added some more. I also changed the documentation a bit. My implementation only breaks this tag usescase where you have an array as an attribute and you can filter by single elements of this array.
This has to be put now in another array...like this:

User.find(:tag => [["ruby", "python"]])

I hope you will merge this pull request nevertheless.

@gingerlime
Copy link

any chance of getting this merged? or a reason why it definitely shouldn't?

@gokulj
Copy link

gokulj commented Oct 3, 2013

+1 for including this.

@frodsan
Copy link
Contributor

frodsan commented Dec 14, 2013

I think you can do User.find(slug: [slug1, slug2]).union(user_id: some_id).

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

6 participants