This project is archived and is in readonly mode.
Default to :all if :select contains comma when counting
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.
-
count-select-no-commas.patch
1.2 KB
-
CancelProfileIsBroken- 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
Could not reproduce on the 2-3-stable. The given test case is passing without any changes to the code.
-
Rajesh
-1 Please read my comments above
-
James Le CuirotThere'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 count breaks has_many through association collection with named scope. 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+1
-
dira
+1 - I used paging for queries with calculated columns in a project.
-
Rizwan Rezaverified
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+1 on idea, -1 on patch, does not apply "Patch does not have a valid e-mail address."
-
James Le Cuirot
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 count breaks has_many through association collection with named scope 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
- 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.
-
