#2823 Calculations break with multi-column :select argument - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#2823 incomplete

Calculations break with multi-column :select argument

Reported by Joshua Krall | June 22nd, 2009 @ 08:43 AM | in 3.0.6

I ran into a problem with geokit and will_paginate... that I tracked down to a bad interaction between the :select option and calculation methods.

It boils down to a simple case that looks like this:

Account.scoped(:select => "credit_limit, COS(credit_limit) as cosine_of_credit_limit").count

This case demonstrates what geokit adds to the :select argument, but it also fails if you do something as simple as :select=>'a,b'

I'm uploading a new patch to fix this, by splitting the column on commas and taking the first column for the COUNT() directive. So, :select=>'a,b' turns into "SELECT COUNT(a) as count_a ..."

All tests pass, but take a look and see if you find anything wrong here. Thanks!

fix_count_with_select_option.diff 2.6 KB

Comments and changes to this ticket

CancelProfileIsBroken August 7th, 2009 @ 02:12 PM
- Tag changed from “activerecord, calculations, patch” to “activerecord, bugmash, calculations, patch”
Michael Koziarski August 8th, 2009 @ 02:54 AM
- Assigned user set to “Michael Koziarski”
- Tag changed from “activerecord, bugmash, calculations, patch” to “activerecord, calculations, patch”
- Milestone cleared.
We can't just take the first column specified, while it works for your example it's not 'good enough' in general. Instead we need a way to fix it permanently

I'd question why we're even taking the :select key from find scopes. I've attached a patch that takes it from :count scopes instead.

I've attached a patch for this and would appreciate your feedback. Your fix would look like:
```
    Account.with_scope(:find=>{:select=>"COS(id), SIN(id)"}, :count=>{:select=>"id"}) do
```
- 2823-count-scope.diff 2.3 KB
Jeremy Kemper August 9th, 2009 @ 12:35 AM
- State changed from “new” to “open”
This is messy indeed. I'd almost see an error raised than let this slip through or get hacked around.

Michael, that fix works for count, but not the other calculations.
Michael Koziarski August 9th, 2009 @ 12:37 AM
OK, I agree on the error case, let's just raise an exception in the
event that the :select scope doesn't map onto the column_name
Jeremy Kemper August 9th, 2009 @ 01:07 AM
- State changed from “open” to “incomplete”
Jeremy Kemper August 30th, 2010 @ 04:10 AM
- Milestone cleared.
- Importance changed from “” to “”
[bulk edit]
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
[bulk edit]
Santiago Pastorino November 15th, 2010 @ 09:15 PM
- Milestone cleared.
[bulk edit]
Santiago Pastorino February 12th, 2011 @ 09:58 PM
- Milestone set to “3.0.5”
[bulk edit]
Santiago Pastorino February 27th, 2011 @ 03:15 AM
- Milestone changed from “3.0.5” to “3.0.6”
[bulk edit]
You flagged this item as spam.
klkk May 23rd, 2011 @ 02:59 AM
www.louisvuittonwarehouse.com