Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

respect auto_increment in rename_column for mysql #9928

Merged
merged 1 commit into from

3 participants

@vipulnsward

Closes #3493

The PR extracts "extra" on mysql column to fetch auto_increment information, which is associated with the column.

This information is then used when migrating.

@vipulnsward

@rafaelfranca , @carlosantoniodasilva : could you take a look at this?

activerecord/test/cases/migration/columns_test.rb
@@ -62,6 +62,13 @@ def test_rename_column_preserves_default_value_not_null
assert_equal 70000, default_after
end
+ def test_mysql_rename_column_preserves_auto_increment
+ if current_adapter?(:MysqlAdapter, :Mysql2Adapter)

The if check could go outside the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
activerecord/test/cases/migration/columns_test.rb
@@ -62,6 +62,13 @@ def test_rename_column_preserves_default_value_not_null
assert_equal 70000, default_after
end
+ def test_mysql_rename_column_preserves_auto_increment
+ if current_adapter?(:MysqlAdapter, :Mysql2Adapter)
+ rename_column "test_models", "id", "id_test"
+ assert_equal connection.columns("test_models").find { |c| c.name == "id_test" }.extra , "auto_increment"

I think the args should be inverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva commented on the diff
...b/active_record/connection_adapters/mysql2_adapter.rb
@@ -63,8 +63,8 @@ def each_hash(result) # :nodoc:
end
end
- def new_column(field, default, type, null, collation) # :nodoc:
- Column.new(field, default, type, null, collation, strict_mode?)
+ def new_column(field, default, type, null, collation, extra = "") # :nodoc:

Should these extra args be nil by default instead? Or should they have a default?

extras default to "" instead of NULL in mysql, so the empty string

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

fixed/rebased

@rafaelfranca

@vipulnsward could you add a CHANGELOG entry?

@rafaelfranca rafaelfranca was assigned
@rafaelfranca rafaelfranca merged commit 1707763 into from
@vipulnsward vipulnsward deleted the branch
@rafaelfranca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 26, 2013
  1. @vipulnsward
This page is out of date. Refresh to see the latest.
View
7 activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* `rename_column` preserves auto_increment in mysql migrations.
+ Fixes #3493.
+
+ *Vipul A M*
+
* PostgreSQL geometric type point is supported by ActiveRecord. Fixes #7324.
*Martin Schuerrer*
@@ -24,7 +29,7 @@
*Ian Young*
-* If ``:inverse_of` is true on an association, then when one calls `find()` on
+* If `:inverse_of` is true on an association, then when one calls `find()` on
the association, Active Record will first look through the in-memory objects
in the association for a particular id. Then, it will go to the DB if it
is not found. This is accomplished by calling `find_by_scan` in
View
3  activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -688,6 +688,9 @@ def add_column_options!(sql, options) #:nodoc:
if options[:null] == false
sql << " NOT NULL"
end
+ if options[:auto_increment] == true
+ sql << " AUTO_INCREMENT"
+ end
end
# SELECT DISTINCT clause for a given set of columns and a given ORDER BY clause.
View
13 activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -25,12 +25,12 @@ def schema_creation
end
class Column < ConnectionAdapters::Column # :nodoc:
- attr_reader :collation, :strict
+ attr_reader :collation, :strict, :extra
- def initialize(name, default, sql_type = nil, null = true, collation = nil, strict = false)
+ def initialize(name, default, sql_type = nil, null = true, collation = nil, strict = false, extra = "")
@strict = strict
@collation = collation
-
+ @extra = extra
super(name, default, sql_type, null)
end
@@ -217,8 +217,8 @@ def each_hash(result) # :nodoc:
end
# Overridden by the adapters to instantiate their specific Column type.
- def new_column(field, default, type, null, collation) # :nodoc:
- Column.new(field, default, type, null, collation)
+ def new_column(field, default, type, null, collation, extra = "") # :nodoc:
+ Column.new(field, default, type, null, collation, extra)
end
# Must return the Mysql error number from the exception, if the exception has an
@@ -448,7 +448,7 @@ def columns(table_name)#:nodoc:
sql = "SHOW FULL FIELDS FROM #{quote_table_name(table_name)}"
execute_and_free(sql, 'SCHEMA') do |result|
each_hash(result).map do |field|
- new_column(field[:Field], field[:Default], field[:Type], field[:Null] == "YES", field[:Collation])
+ new_column(field[:Field], field[:Default], field[:Type], field[:Null] == "YES", field[:Collation], field[:Extra])
end
end
end
@@ -682,6 +682,7 @@ def rename_column_sql(table_name, column_name, new_column_name)
if column = columns(table_name).find { |c| c.name == column_name.to_s }
options[:default] = column.default
options[:null] = column.null
+ options[:auto_increment] = (column.extra == "auto_increment")
else
raise ActiveRecordError, "No such column: #{table_name}.#{column_name}"
end
View
4 activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -63,8 +63,8 @@ def each_hash(result) # :nodoc:
end
end
- def new_column(field, default, type, null, collation) # :nodoc:
- Column.new(field, default, type, null, collation, strict_mode?)
+ def new_column(field, default, type, null, collation, extra = "") # :nodoc:

Should these extra args be nil by default instead? Or should they have a default?

extras default to "" instead of NULL in mysql, so the empty string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ Column.new(field, default, type, null, collation, strict_mode?, extra)
end
def error_number(exception)
View
4 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -150,8 +150,8 @@ def each_hash(result) # :nodoc:
end
end
- def new_column(field, default, type, null, collation) # :nodoc:
- Column.new(field, default, type, null, collation, strict_mode?)
+ def new_column(field, default, type, null, collation, extra = "") # :nodoc:
+ Column.new(field, default, type, null, collation, strict_mode?, extra)
end
def error_number(exception) # :nodoc:
View
7 activerecord/test/cases/migration/columns_test.rb
@@ -62,6 +62,13 @@ def test_rename_column_preserves_default_value_not_null
assert_equal 70000, default_after
end
+ if current_adapter?(:MysqlAdapter, :Mysql2Adapter)
+ def test_mysql_rename_column_preserves_auto_increment
+ rename_column "test_models", "id", "id_test"
+ assert_equal "auto_increment", connection.columns("test_models").find { |c| c.name == "id_test" }.extra
+ end
+ end
+
def test_rename_nonexistent_column
exception = if current_adapter?(:PostgreSQLAdapter, :OracleAdapter)
ActiveRecord::StatementInvalid
Something went wrong with that request. Please try again.