Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Set the primary key during #copy_table if necessary. Fixes [#2312] #6186

Merged
merged 1 commit into from May 15, 2012
Jump to file or symbol
Failed to load files and symbols.
+13 −1
Split
@@ -522,7 +522,11 @@ def move_table(from, to, options = {}, &block) #:nodoc:
end
def copy_table(from, to, options = {}) #:nodoc:
- options = options.merge(:id => (!columns(from).detect{|c| c.name == 'id'}.nil? && 'id' == primary_key(from).to_s))
+ from_primary_key = primary_key(from)
+ options[:primary_key] = from_primary_key if from_primary_key != 'id'
+ unless options[:primary_key]
+ options[:id] = columns(from).detect{|c| c.name == 'id'}.present? && from_primary_key == 'id'
+ end
@carlosantoniodasilva

carlosantoniodasilva May 7, 2012

Owner

Just a question: before it was being tested for primary_key(from).to_s, now to_s is gone.. is that a problem or everything is working fine - ie all tests passing?

@scally

scally May 7, 2012

Contributor

Yes, all tests are passing.

I couldn't think of a good reason for to_s to remain. It's not necessary to guard against nil in a string comparison, and the column name should already be a string.

@carlosantoniodasilva

carlosantoniodasilva May 7, 2012

Owner

Ok, great. Thanks!

create_table(to, options) do |definition|
@definition = definition
columns(from).each do |column|
@@ -57,6 +57,14 @@ def test_copy_table_with_id_col_that_is_not_primary_key
end
end
+ def test_copy_table_with_unconventional_primary_key
+ test_copy_table('owners', 'owners_unconventional') do |from, to, options|
+ original_pk = @connection.primary_key('owners')
+ copied_pk = @connection.primary_key('owners_unconventional')
+ assert_equal original_pk, copied_pk
+ end
+ end
+
protected
def copy_table(from, to, options = {})
@connection.copy_table(from, to, {:temporary => true}.merge(options))