SQLite, MySQL, PostgreSQL, Oracle: quote column names in column migration SQL statements. Closes #8466.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6889 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

6 files changed, 33 insertions, 15 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index b17af37d4b..0ae8cbdd93 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* SQLite, MySQL, PostgreSQL, Oracle: quote column names in column migration SQL statements.  #8466 [marclove, lorenjohnson]
+
 * Allow nil serialized attributes with a set class constraint.  #7293 [sandofsky]
 
 * Oracle: support binary fixtures.  #7987 [Michael Schoen]
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index a3d211c85c..6ba973041b 100755
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -388,7 +388,7 @@ module ActiveRecord
       def change_column_default(table_name, column_name, default) #:nodoc:
         current_type = select_one("SHOW COLUMNS FROM #{table_name} LIKE '#{column_name}'")["Type"]
 
-        execute("ALTER TABLE #{table_name} CHANGE #{column_name} #{column_name} #{current_type} DEFAULT #{quote(default)}")
+        execute("ALTER TABLE #{table_name} CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{current_type} DEFAULT #{quote(default)}")
       end
 
       def change_column(table_name, column_name, type, options = {}) #:nodoc:
@@ -396,14 +396,14 @@ module ActiveRecord
           options[:default] = select_one("SHOW COLUMNS FROM #{table_name} LIKE '#{column_name}'")["Default"]
         end
 
-        change_column_sql = "ALTER TABLE #{table_name} CHANGE #{column_name} #{column_name} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
+        change_column_sql = "ALTER TABLE #{table_name} CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
         add_column_options!(change_column_sql, options)
         execute(change_column_sql)
       end
 
       def rename_column(table_name, column_name, new_column_name) #:nodoc:
         current_type = select_one("SHOW COLUMNS FROM #{table_name} LIKE '#{column_name}'")["Type"]
-        execute "ALTER TABLE #{table_name} CHANGE #{column_name} #{new_column_name} #{current_type}"
+        execute "ALTER TABLE #{table_name} CHANGE #{quote_column_name(column_name)} #{quote_column_name(new_column_name)} #{current_type}"
       end
 
 
diff --git a/activerecord/lib/active_record/connection_adapters/oracle_adapter.rb b/activerecord/lib/active_record/connection_adapters/oracle_adapter.rb
index 25eca87304..cccb38f5b2 100644
--- a/activerecord/lib/active_record/connection_adapters/oracle_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/oracle_adapter.rb
@@ -387,21 +387,21 @@ begin
         end
 
         def change_column_default(table_name, column_name, default) #:nodoc:
-          execute "ALTER TABLE #{table_name} MODIFY #{column_name} DEFAULT #{quote(default)}"
+          execute "ALTER TABLE #{table_name} MODIFY #{quote_column_name(column_name)} DEFAULT #{quote(default)}"
         end
 
         def change_column(table_name, column_name, type, options = {}) #:nodoc:
-          change_column_sql = "ALTER TABLE #{table_name} MODIFY #{column_name} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
+          change_column_sql = "ALTER TABLE #{table_name} MODIFY #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
           add_column_options!(change_column_sql, options)
           execute(change_column_sql)
         end
 
         def rename_column(table_name, column_name, new_column_name) #:nodoc:
-          execute "ALTER TABLE #{table_name} RENAME COLUMN #{column_name} to #{new_column_name}"
+          execute "ALTER TABLE #{table_name} RENAME COLUMN #{quote_column_name(column_name)} to #{new_column_name}"
         end
 
         def remove_column(table_name, column_name) #:nodoc:
-          execute "ALTER TABLE #{table_name} DROP COLUMN #{column_name}"
+          execute "ALTER TABLE #{table_name} DROP COLUMN #{quote_column_name(column_name)}"
         end
 
         # Find a table's primary key and sequence. 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 3fb1655526..d6e0b3d26d 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -325,32 +325,35 @@ module ActiveRecord
         default = options[:default]
         notnull = options[:null] == false
 
+        quoted_column_name = quote_column_name(column_name)
+
         # Add the column.
-        execute("ALTER TABLE #{table_name} ADD COLUMN #{column_name} #{type_to_sql(type, options[:limit])}")
+        execute("ALTER TABLE #{table_name} ADD COLUMN #{quoted_column_name} #{type_to_sql(type, options[:limit])}")
 
         # Set optional default. If not null, update nulls to the new default.
         if options_include_default?(options)
           change_column_default(table_name, column_name, default)
           if notnull
-            execute("UPDATE #{table_name} SET #{column_name}=#{quote(default, options[:column])} WHERE #{column_name} IS NULL")
+            execute("UPDATE #{table_name} SET #{quoted_column_name}=#{quote(default, options[:column])} WHERE #{quoted_column_name} IS NULL")
           end
         end
 
         if notnull
-          execute("ALTER TABLE #{table_name} ALTER #{column_name} SET NOT NULL")
+          execute("ALTER TABLE #{table_name} ALTER #{quoted_column_name} SET NOT NULL")
         end
       end
 
       def change_column(table_name, column_name, type, options = {}) #:nodoc:
         begin
-          execute "ALTER TABLE #{table_name} ALTER COLUMN #{column_name} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
+          execute "ALTER TABLE #{table_name} ALTER COLUMN #{quoted_column_name} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
         rescue ActiveRecord::StatementInvalid
           # This is PG7, so we use a more arcane way of doing it.
           begin_db_transaction
-          add_column(table_name, "#{column_name}_ar_tmp", type, options)
-          execute "UPDATE #{table_name} SET #{column_name}_ar_tmp = CAST(#{column_name} AS #{type_to_sql(type, options[:limit], options[:precision], options[:scale])})"
+          tmp_column_name = "#{column_name}_ar_tmp"
+          add_column(table_name, tmp_column_name, type, options)
+          execute "UPDATE #{table_name} SET #{quote_column_name(tmp_column_name)} = CAST(#{quote_column_name(column_name)} AS #{type_to_sql(type, options[:limit], options[:precision], options[:scale])})"
           remove_column(table_name, column_name)
-          rename_column(table_name, "#{column_name}_ar_tmp", column_name)
+          rename_column(table_name, tmp_column_name, column_name)
           commit_db_transaction
         end
 
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 5da97ceff6..963d4b92bd 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -327,8 +327,10 @@ module ActiveRecord
           rename.inject(column_mappings) {|map, a| map[a.last] = a.first; map}
           from_columns = columns(from).collect {|col| col.name}
           columns = columns.find_all{|col| from_columns.include?(column_mappings[col])}
+          quoted_columns = columns.map { |col| quote_column_name(col) } * ','
+
           @connection.execute "SELECT * FROM #{from}" do |row|
-            sql = "INSERT INTO #{to} ("+columns*','+") VALUES ("
+            sql = "INSERT INTO #{to} (#{quoted_columns}) VALUES ("
             sql << columns.map {|col| quote row[column_mappings[col]]} * ', '
             sql << ')'
             @connection.execute sql
diff --git a/activerecord/test/migration_test.rb b/activerecord/test/migration_test.rb
index 8a3c1d05ed..d1e2059c9a 100644
--- a/activerecord/test/migration_test.rb
+++ b/activerecord/test/migration_test.rb
@@ -398,6 +398,17 @@ if ActiveRecord::Base.connection.supports_migrations?
       end
     end
 
+    def test_rename_column_with_sql_reserved_word
+      begin
+        assert_nothing_raised { Person.connection.rename_column "people", "first_name", "group" }
+        Person.reset_column_information
+        assert Person.column_names.include?("group")
+      ensure
+        Person.connection.remove_column("people", "group") rescue nil
+        Person.connection.add_column("people", "first_name", :string) rescue nil
+      end
+    end
+
     def test_rename_table
       begin
         ActiveRecord::Base.connection.create_table :octopuses do |t|