[patch] .count() doesn't accept conditions/options
Reported by Adam French | December 5th, 2007 @ 04:43 PM
User.count() works great, but sometimes people are going to need to perform conditional counts like User.count(:name.like => 'Harry')
Comments and changes to this ticket
-
Bjørn Arild Mæland December 10th, 2007 @ 01:27 PM
It does, in latest trunk: Person.count("name LIKE 'John'")
Currently only supports pure sql tho.
-
Bjørn Arild Mæland December 11th, 2007 @ 07:31 PM
- State changed from “new” to “resolved”
-
Adam French December 18th, 2007 @ 11:51 AM
- State changed from “resolved” to “open”
I'd say this needs to remain open until .count() takes the same (or very nearly the same) options hash that #all() and #first() do
-
Tony Pitale December 19th, 2007 @ 08:50 AM
- Title changed from “.count() doesn't accept conditions/options” to “[patch] .count() doesn't accept conditions/options”
Here's a patch that duplicates the functionality from load_command for use in the table mapping. #count should now be able to take a parameter hash. E.g. :conditions => {:field1.eql => '32145', :field2.like => '%A%'}.
The problem is the amount of duplicated code. I'm going to look into making this a support class that will be usable from within load_command.rb and for use with count.
-
Tony Pitale December 19th, 2007 @ 09:09 AM
The patch does not work when a column name is different from the property name. E.g. property :description, :text, :column => 'descr'. If you use description as part of the conditions as you would with #all or #first the count will fail, if you use descr with #count, #first or #all will fail.
This code needs to come over from load_command.rb as well.
-
Adam French December 19th, 2007 @ 10:04 AM
toss in a few specs and we're golden. awesome work, dude.
-
Tony Pitale December 26th, 2007 @ 08:05 PM
I'll post another patch shortly that is actually a better solution. After speaking to many people on irc, it was decided to extract the logic from load_command.rb for creating the sql for a conditions hash.
-
Sam Smoot December 27th, 2007 @ 10:17 AM
- Milestone cleared.
Not sure the status on this, but I've assigned it to the 0.3.0 release, which will be the next release, and "Production Ready" &tm;
-
Tony Pitale December 27th, 2007 @ 02:14 PM
Here's another patch, having duplicated code from load_command.rb for building the conditions sql.
I could go either way. This patch has a lot of duplicate code (which could be removed from load_command.rb) but, the first patch does not allow using the same exact conditions as a query.
-
Sam Smoot December 28th, 2007 @ 10:08 PM
Tony, sorry it took so long. Finally got a chance to review the patch.
Few comments:
Symbol#to_s should be unnecessary. We have a faster cached version in place. I'm curious why you added it?
Other than that, it looks good to me. I agree, we should remove the duplication from LoadCommand though when this is applied. If you could take that on as well I'd appreciate it. :-)
Only other comment is that it might be nice to add a few new specs/docs while you're in there. Especially with the Conditions class separated out, it should be much easier to test.
Other than that, great job. Looking forward to seeing it committed!
-
Tony Pitale January 9th, 2008 @ 03:59 PM
Hey, sorry I've been out of it for a few days. I'll try and get to this tonight.
I'm not sure about to_s. I think it was for something else that might have gotten caught up in the diff.
I'll be on irc tonight to ask about writing specs, it is definitely needed. Also, I'll probe about replacing the dupe code in LoadCommand.
Thanks!
-
-
Tony Pitale January 14th, 2008 @ 12:42 PM
Learning to make proper specs, if anyone wants to help out to speed this along it would be greatly appreciated.
Tony++
-
Kevin Bullock January 14th, 2008 @ 03:36 PM
I'll be on IRC (I'm TheMystic) this evening around 7pm Central, and I can help out with this. Come find me if you'd like some assistance.
-
Tony Pitale January 17th, 2008 @ 09:12 AM
Here's my first attempt at a basic spec. I'm not certain I'm accessing the class correctly from within the spec. Do we have some conventions I should be following?
-
Adam French January 22nd, 2008 @ 11:03 PM
Here's a quick API spec for how .count() should act. This doesn't test the underlying sql generation though.
attached: count_spec.diff
-
-
Tony Pitale January 24th, 2008 @ 02:18 PM
Great, thanks! I'll put these together and clean up a bit and try and commit this evening.
-
Tony Pitale January 24th, 2008 @ 09:15 PM
- Title changed from “[patch] .count() doesn't accept conditions/options” to “Commited in 708”
Thanks to Adam French, I've got this working, minus the ability to count distinct. I have yet to migrate load_command stuff for find into this condition.rb class but, this all might be for nothing because Sam has now put Query class out there.
-
Adam French January 24th, 2008 @ 11:46 PM
- Title changed from “Commited in 708” to “[patch] .count() doesn't accept conditions/options”
[[ changing the title back ]]
-
Adam French January 24th, 2008 @ 11:52 PM
There's no reason why we can't have a (empty) subclass of Query called Calculation where Count, Sum, Min, Max, and Avg could inherit from?
It'd look something like this....(( see attached query_calculation_spec.diff ))
What do you think?
-
-
Adam French March 17th, 2008 @ 12:29 PM
we need to verify that .count() can take :join and perform accordingly....
...ideally, the same conditions hash you feed to .all() or .first() you should be able to feed to .count()
-
Tony Pitale March 27th, 2008 @ 07:07 PM
Because of the way we are refactoring in 0.9 this will be pretty useless. I think this should get cut and closed. Marked as a feature for the next release.
-
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
Bjørn Arild Mæland
Rick
Tony Pitale