This project is archived and is in readonly mode.
AssociationCollection.count Returns Incorrect Value
Reported by Patrick Joyce | March 3rd, 2009 @ 03:33 PM | in 3.0.2
AssociationCollection.count can return an incorrect value if a finder_sql is a multiline string and there is a new line character immediately following "SELECT". This issue affects HasManyAssociations, HasAndBelongToManyAssociations, and HasManyThroughAssociations.
The root of the problem is that when construct_sql generates the counter_sql from the finder_sql it uses a regex that includes a space after the SELECT. This is the current regex:
/SELECT (\/\*.*?\*\/ )?(.*)\bFROM\b/im
The solution is to match any word break character after the
SELECT
/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im
I added tests to show the problem to both the HasManyAssociationsTest and HasAndBelongsToManyAssociationsTest. I did not add a test to the HasManyThroughAssociationsTest because there were no existing tests for using finder_sql and counter_sql.
NOTES ON THE PATCH:
To show the issue in the HasManyAssociationsTest I needed to add an additional client fixture. This is because of the way that AssociationsCollection count uses connection.select_value to execute the counter_sql. select_value returns a single value from a record. If there are multiple values in the record, it returns the first element of the hash. In this case, that was the rating for companies(:another_client) and the rating happened to be 1, which was the expected count. I added another company that was a client of companies(:another_firm) so that the expected count would be 2. This required changing several tests to reflect that there now was another company and largely accounts for the size of my patch.
In addition to changing the regular expression to respect line breaks immediately following the SELECT statement in finder_sql, I also refactored the code that sets the counter_sql into a method, construct_counter_sql, on the base AssociationCollection. I did this because the code to generate the counter_sql was identical in HasManyCollection, HasAndBelongsToManyCollection, and HasManyThroughCollection. Modifying the regex in 3 separate places felt dirty.
Comments and changes to this ticket
-
Repository June 21st, 2009 @ 10:10 PM
- State changed from new to resolved
(from [4851ca9e13a4317342df02ae25b1929340523f7a]) Generate proper :counter_sql from :finder_sql when there is a newline character immediately following 'SELECT' [#2118 state:resolved]
Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/4851ca9e13a4317342df02ae25b192... -
Yehuda Katz (wycats) June 22nd, 2009 @ 08:06 PM
- State changed from resolved to open
This patch doesn't work on Postgres because the test you added (test_count_with_finder_sql) doesn't work on Postgres, even in the single line version. Can we make this work?
-
Yehuda Katz (wycats) June 22nd, 2009 @ 08:06 PM
- Milestone cleared.
-
Repository June 22nd, 2009 @ 08:07 PM
- State changed from open to resolved
(from [80f1f863cd0f9cba89079511282de5710a2e1832]) Revert "Generate proper :counter_sql from :finder_sql when there is a newline character immediately following 'SELECT' [#2118 state:resolved]"
This reverts commit 4851ca9e13a4317342df02ae25b1929340523f7a.
The tests do not pass for postgresql.
http://github.com/rails/rails/commit/80f1f863cd0f9cba89079511282de5... -
Yehuda Katz (wycats) June 22nd, 2009 @ 11:29 PM
- State changed from resolved to open
-
Repository June 23rd, 2009 @ 10:50 PM
- State changed from open to resolved
(from [33dc4eb9ebe7d1a577aa7edf7215b0c14352205d]) Generate proper :counter_sql from :finder_sql when there is a newline character immediately following 'SELECT' [#2118 state:resolved]
Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/33dc4eb9ebe7d1a577aa7edf7215b0... -
Repository June 23rd, 2009 @ 10:50 PM
(from [89e4ccfae9fd2b845464336e0b6ab63ae9a18827]) Revert "Generate proper :counter_sql from :finder_sql when there is a newline character immediately following 'SELECT' [#2118 state:resolved]"
This reverts commit 4851ca9e13a4317342df02ae25b1929340523f7a.
The tests do not pass for postgresql.
http://github.com/rails/rails/commit/89e4ccfae9fd2b845464336e0b6ab6... -
Patrick Joyce June 24th, 2009 @ 02:30 PM
i'll fix the patch to work with Postgres but it will probably have to wait until the weekend.
-
Repository July 1st, 2009 @ 12:07 AM
(from [45e6f19925f23c3db257c15371d8f512cca843cd]) Revert "Revert "Generate proper :counter_sql from :finder_sql when there is a newline character immediately following 'SELECT' [#2118 state:resolved]""
This reverts commit 80f1f863cd0f9cba89079511282de5710a2e1832.
The feature doesn't work on Postgres, so don't test it on Postgres.
Also, Postgres compatibility is irrelevant to the ticket/patch in question.
http://github.com/rails/rails/commit/45e6f19925f23c3db257c15371d8f5... -
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to 3.0.2
- Importance changed from to Medium
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
- 2118 AssociationCollection.count Returns Incorrect Value (from [4851ca9e13a4317342df02ae25b1929340523f7a]) Generat...
- 2118 AssociationCollection.count Returns Incorrect Value (from [80f1f863cd0f9cba89079511282de5710a2e1832]) Revert ...
- 2118 AssociationCollection.count Returns Incorrect Value (from [33dc4eb9ebe7d1a577aa7edf7215b0c14352205d]) Generat...
- 2118 AssociationCollection.count Returns Incorrect Value (from [89e4ccfae9fd2b845464336e0b6ab63ae9a18827]) Revert ...
- 2118 AssociationCollection.count Returns Incorrect Value (from [45e6f19925f23c3db257c15371d8f512cca843cd]) Revert ...