This project is archived and is in readonly mode.
Calculations break with multi-column :select argument
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!
-
CancelProfileIsBroken- Tag changed from activerecord, calculations, patch to activerecord, bugmash, calculations, patch
-
Michael Koziarski- 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- 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
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
- State changed from open to incomplete
-
-
-
-
-