fix activerecord query_method regression with offset into Fixnum #4410

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

denisj commented Jan 11, 2012

This fix the regression related in #4409

Member

josevalim commented Jan 11, 2012

Tests pls?

Contributor

denisj commented Jan 11, 2012

Seems to occur only with mysql and mysql2 !!

@tenderlove tenderlove commented on an outdated diff Jan 11, 2012

activerecord/test/cases/finder_test.rb
@@ -1171,6 +1171,13 @@ def test_find_one_message_with_custom_primary_key
end
end
+ def test_finder_with_offset_string
+ sql_query = Topic.send(:construct_finder_arel, {:offset => "3"}).to_sql
+ results = Topic.connection.select_values(sql_query)
+
+ assert_equal 1, results.count
+ end
+
@tenderlove

tenderlove Jan 11, 2012

Owner

Is it possible to test this without using private methods like construct_finder_sql and to_sql? We need to make sure things work with the public API.

Member

arunagw commented Mar 11, 2012

How about this PR guys? Is this ready to go in? @denisj added one tests also.

@denisj I think you need to squash your commits.

Member

arunagw commented Mar 12, 2012

Closing this as a2c2f40 done here!

Thanks :-)

arunagw closed this Mar 12, 2012

Owner

pixeltrix commented Mar 12, 2012

Squashed and merged in #5390

This was referenced Mar 13, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment