This project is archived and is in readonly mode.
Default to :all if :select contains comma when counting
Reported by James Le Cuirot | March 24th, 2009 @ 11:01 PM | in 3.0.2
I just upgraded my application to 2.3 and pagination started generating invalid SQL. Admittedly 2.2 hadn't been calculating the pages properly before either but I hadn't noticed because this was the first time it had caused an error.
It fails because the scope I'm applying defines :select with a calculated column. In fact, if you just specify more than one column of any kind, it fails because it tries to do a query like this.
SELECT COUNT(foo, bar) FROM baz
I can't simply remove the calculated column when counting in my application code because the process of fetching the records and counting the number of pages is combined in plugins such as will_paginate.
This patch adds a single line that changes column_name to :all if it contains a comma. This shouldn't break anything that isn't broken already. A test is included.
Comments and changes to this ticket
-
CancelProfileIsBroken August 5th, 2009 @ 03:47 PM
- Tag changed from “2.3, 2.3.x, activerecord, active_record, calculations, patch, select, test” to “2.3, 2.3.x, activerecord, active_record, bugmash, calculations, patch, select, test”
-
Rajesh August 8th, 2009 @ 10:10 PM
Could not reproduce on the 2-3-stable. The given test case is passing without any changes to the code.
-
-
James Le Cuirot August 8th, 2009 @ 10:39 PM
There's a reason for that. The change that allowed scoped :select to be passed through to count created problems with has_many :through associations and was recently reverted as noted in ticket #2189. I was a little disappointed by this decision as I was relying on this feature. I haven't seen the discussion on rails-core and I would have to see that before commenting further. Time for bed now though. ;) Sorry for not commenting on this earlier but I've been away and just got back.
-
-
dira August 9th, 2009 @ 02:27 PM
+1 - I used paging for queries with calculated columns in a project.
-
Rizwan Reza August 9th, 2009 @ 03:25 PM
verified
The patch works fine under master but doesn't apply to 2-3-stable. All tests pass for master as well. Good work!
-
Elad Meidar August 9th, 2009 @ 04:32 PM
+1 on idea, -1 on patch, does not apply "Patch does not have a valid e-mail address."
-
James Le Cuirot August 9th, 2009 @ 04:48 PM
It won't fix anything on the latest 2-3-stable anyway as I have already said because the feature was reverted. I have now read the rails-core discussion and I don't think the feature was reverted in master, hence why it still works there.
The issue is related to ticket #2189 but this patch alone won't fix that and so a more general fix needs to be considered. Please read around these issues before simply stating "it doesn't apply" or "it works".
-
Rizwan Reza January 20th, 2010 @ 11:08 AM
- State changed from “new” to “resolved”
- Tag changed from “2.3, 2.3.x, activerecord, active_record, bugmash, calculations, patch, select, test” to “2.3, 2.3.x, activerecord, active_record, calculations, patch, select, test”
- Milestone cleared.
-
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to “3.0.2”
- Importance changed from “” to “”
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
James Le Cuirot
Attachments
Referenced by
-
2189
count breaks has_many through association collection with named scope
I hit a very similar issue, which I reported in ticket #2...
-
2189
count breaks has_many through association collection with named scope
I hit a very similar issue, which I reported in ticket #2...