This project is archived and is in readonly mode.
count method doesn't work with specified conditions
Reported by Jan Xie | April 2nd, 2009 @ 08:15 AM | in 3.x
AssociationCollection#count method won't take conditions in arguments into account if there is finder_sql or counter_sql. e.g.
class User has_many :photos, :finder_sql => '...' end
some_user.photos.count(:conditions => ['photos.created_at > ?', Time.today]) # all user photos will be returned, not only those created today
I wrote a patch to fix this (compatible with older implementation)
Comments and changes to this ticket
-
Jan Xie May 11th, 2009 @ 09:11 AM
- Assigned user set to Jeremy Kemper
-
Jeremy Kemper May 11th, 2009 @ 07:34 PM
We avoid touching custom finder_sql since appending " AND conditions" may just break the query.
The patch has a passing test case. Does it fail for other uses of finder_ or counter_sql?
-
Jan Xie May 12th, 2009 @ 04:19 AM
Though I think it will work at most time, yes it will break some finder_sql. To make it works in all cases, a mini sql parser need to be introduced, which doesn't look like a good option :)
-
Dan Pickett May 9th, 2010 @ 06:00 PM
- Tag changed from active_record, conditions, count to active_record, bugmash, conditions, count
-
Gavin Stark May 16th, 2010 @ 12:40 AM
If modifications to the conditions of an association using a finder_sql are not allowed, then they should not be silently ignored. Perhaps an exception could be raised which would at least tell the developer something rather than executing and returning unexpected results?
-
Neeraj Singh July 1st, 2010 @ 06:31 PM
- Importance changed from to
After this ticket was created Jeremy said "We avoid touching custom finder_sql since appending " AND conditions" may just break the query."
+1 for that.
I would agree with Gavin that raising an exception should be a reasonable compromise.
Jeremy any thoughts on how we should proceed on this one.
-
Jeremy Kemper July 1st, 2010 @ 06:48 PM
- State changed from new to open
Failing fast -- raising an exception -- would be great. Patch + test welcome!
-
Neeraj Singh July 2nd, 2010 @ 02:45 AM
- Tag changed from active_record, bugmash, conditions, count to active_record, bugmash, conditions, count, patch
Attached is patch with test.
-
Neeraj Singh July 13th, 2010 @ 08:04 PM
- Assigned user changed from Jeremy Kemper to José Valim
-
Repository July 13th, 2010 @ 09:05 PM
- State changed from open to resolved
(from [edb5401039ee15c37b201244c5dbf660bed51fb4]) count method should not take options if it is operated on has_many association which has finder_sql or counter_sql
[#2395 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/edb5401039ee15c37b201244c5dbf6...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 2395 count method doesn't work with specified conditions [#2395 state:resolved]