Skip to content

Commit

Permalink
Quote find_in_batches ORDER BY clause [#6620 state:resolved]
Browse files Browse the repository at this point in the history
  • Loading branch information
pixeltrix committed Mar 29, 2011
1 parent a9dafbb commit 555d016
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 3 deletions.
13 changes: 12 additions & 1 deletion activerecord/lib/active_record/attribute_methods/primary_key.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ def primary_key
@primary_key ||= reset_primary_key @primary_key ||= reset_primary_key
end end


# Returns a quoted version of the primary key name, used to construct SQL statements.
def quoted_primary_key
@quoted_primary_key ||= connection.quote_column_name(primary_key)
end

def reset_primary_key #:nodoc: def reset_primary_key #:nodoc:
key = self == base_class ? get_primary_key(base_class.name) : key = self == base_class ? get_primary_key(base_class.name) :
base_class.primary_key base_class.primary_key
Expand All @@ -43,7 +48,12 @@ def get_primary_key(base_name) #:nodoc:
end end


attr_accessor :original_primary_key attr_accessor :original_primary_key
attr_writer :primary_key
# Attribute writer for the primary key column
def primary_key=(value)
@quoted_primary_key = nil
@primary_key = value
end


# Sets the name of the primary key column to use to the given value, # Sets the name of the primary key column to use to the given value,
# or (if the value is nil or false) to the value returned by the given # or (if the value is nil or false) to the value returned by the given
Expand All @@ -53,6 +63,7 @@ def get_primary_key(base_name) #:nodoc:
# set_primary_key "sysid" # set_primary_key "sysid"
# end # end
def set_primary_key(value = nil, &block) def set_primary_key(value = nil, &block)
@quoted_primary_key = nil
@primary_key ||= '' @primary_key ||= ''
self.original_primary_key = @primary_key self.original_primary_key = @primary_key
value &&= value.to_s value &&= value.to_s
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Relation


# These are explicitly delegated to improve performance (avoids method_missing) # These are explicitly delegated to improve performance (avoids method_missing)
delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to => :to_a delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to => :to_a
delegate :table_name, :primary_key, :to => :klass delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :to => :klass


attr_reader :table, :klass, :loaded attr_reader :table, :klass, :loaded
attr_accessor :extensions attr_accessor :extensions
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation/batches.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def find_in_batches(options = {})
private private


def batch_order def batch_order
"#{table_name}.#{primary_key} ASC" "#{quoted_table_name}.#{quoted_primary_key} ASC"
end end
end end
end end
10 changes: 10 additions & 0 deletions activerecord/test/cases/batches_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -83,4 +83,14 @@ def test_find_in_batches_shouldnt_excute_query_unless_needed
Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch } Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch }
end end
end end

def test_find_in_batches_should_quote_batch_order
c = Post.connection
assert_sql(/ORDER BY #{c.quote_table_name('posts')}.#{c.quote_column_name('id')}/) do
Post.find_in_batches(:batch_size => 1) do |batch|
assert_kind_of Array, batch
assert_kind_of Post, batch.first
end
end
end
end end
9 changes: 9 additions & 0 deletions activerecord/test/cases/primary_keys_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -136,4 +136,13 @@ def test_primary_key_returns_nil_if_it_does_not_exist
assert_nil ActiveRecord::Base.connection.primary_key('developers_projects') assert_nil ActiveRecord::Base.connection.primary_key('developers_projects')
end end
end end

def test_quoted_primary_key_after_set_primary_key
k = Class.new( ActiveRecord::Base )
assert_equal k.connection.quote_column_name("id"), k.quoted_primary_key
k.primary_key = "foo"
assert_equal k.connection.quote_column_name("foo"), k.quoted_primary_key
k.set_primary_key "bar"
assert_equal k.connection.quote_column_name("bar"), k.quoted_primary_key
end
end end

0 comments on commit 555d016

Please sign in to comment.