Merge branch 'rm-create-with-index'

Author: rafaelfranca

activerecord/CHANGELOG.md

@@ -1,3 +1,20 @@
+*   Create indexes inline in CREATE TABLE for MySQL.
+
+    This is important, because adding an index on a temporary table after it has been created
+    would commit the transaction.
+
+    It also allows creating and dropping indexed tables with fewer queries and fewer permissions
+    required.
+
+    Example:
+
+        create_table :temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query" do |t|
+          t.index :zip
+        end
+        # => CREATE TEMPORARY TABLE temp (INDEX (zip)) AS SELECT id, name, zip FROM a_really_complicated_query
+
+    *Cody Cutrer*, *Steve Rice*, *Rafael Mendonça Franca*
+
 *   Save `has_one` association even if the record doesn't changed.
 
     Fixes #14407.

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -186,24 +186,23 @@ def column_exists?(table_name, column_name, type = nil, options = {})
       def create_table(table_name, options = {})
         td = create_table_definition table_name, options[:temporary], options[:options], options[:as]
 
-        if !options[:as]
-          unless options[:id] == false
-            pk = options.fetch(:primary_key) {
-              Base.get_primary_key table_name.to_s.singularize
-            }
-
-            td.primary_key pk, options.fetch(:id, :primary_key), options
+        if options[:id] != false && !options[:as]
+          pk = options.fetch(:primary_key) do
+            Base.get_primary_key table_name.to_s.singularize
           end
 
-          yield td if block_given?
+          td.primary_key pk, options.fetch(:id, :primary_key), options
         end
 
+        yield td if block_given?
+
         if options[:force] && table_exists?(table_name)
           drop_table(table_name, options)
         end
 
-        execute schema_creation.accept td
-        td.indexes.each_pair { |c,o| add_index table_name, c, o }
+        result = execute schema_creation.accept td
+        td.indexes.each_pair { |c, o| add_index(table_name, c, o) } unless supports_indexes_in_create?
+        result
       end
 
       # Creates a new join table with the name created using the lexical order of the first two
@@ -740,6 +739,40 @@ def update_table_definition(table_name, base) #:nodoc:
         Table.new(table_name, base)
       end
 
+      def add_index_options(table_name, column_name, options = {}) #:nodoc:
+        column_names = Array(column_name)
+        index_name   = index_name(table_name, column: column_names)
+
+        options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type)
+
+        index_type = options[:unique] ? "UNIQUE" : ""
+        index_type = options[:type].to_s if options.key?(:type)
+        index_name = options[:name].to_s if options.key?(:name)
+        max_index_length = options.fetch(:internal, false) ? index_name_length : allowed_index_name_length
+
+        if options.key?(:algorithm)
+          algorithm = index_algorithms.fetch(options[:algorithm]) {
+            raise ArgumentError.new("Algorithm must be one of the following: #{index_algorithms.keys.map(&:inspect).join(', ')}")
+          }
+        end
+
+        using = "USING #{options[:using]}" if options[:using].present?
+
+        if supports_partial_index?
+          index_options = options[:where] ? " WHERE #{options[:where]}" : ""
+        end
+
+        if index_name.length > max_index_length
+          raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{max_index_length} characters"
+        end
+        if table_exists?(table_name) && index_name_exists?(table_name, index_name, false)
+          raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' already exists"
+        end
+        index_columns = quoted_columns_for_index(column_names, options).join(", ")
+
+        [index_name, index_type, index_columns, index_options, algorithm, using]
+      end
+
       protected
         def add_index_sort_order(option_strings, column_names, options = {})
           if options.is_a?(Hash) && order = options[:order]
@@ -770,40 +803,6 @@ def options_include_default?(options)
           options.include?(:default) && !(options[:null] == false && options[:default].nil?)
         end
 
-        def add_index_options(table_name, column_name, options = {})
-          column_names = Array(column_name)
-          index_name   = index_name(table_name, column: column_names)
-
-          options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type)
-
-          index_type = options[:unique] ? "UNIQUE" : ""
-          index_type = options[:type].to_s if options.key?(:type)
-          index_name = options[:name].to_s if options.key?(:name)
-          max_index_length = options.fetch(:internal, false) ? index_name_length : allowed_index_name_length
-
-          if options.key?(:algorithm)
-            algorithm = index_algorithms.fetch(options[:algorithm]) {
-              raise ArgumentError.new("Algorithm must be one of the following: #{index_algorithms.keys.map(&:inspect).join(', ')}")
-            }
-          end
-
-          using = "USING #{options[:using]}" if options[:using].present?
-
-          if supports_partial_index?
-            index_options = options[:where] ? " WHERE #{options[:where]}" : ""
-          end
-
-          if index_name.length > max_index_length
-            raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{max_index_length} characters"
-          end
-          if index_name_exists?(table_name, index_name, false)
-            raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' already exists"
-          end
-          index_columns = quoted_columns_for_index(column_names, options).join(", ")
-
-          [index_name, index_type, index_columns, index_options, algorithm, using]
-        end
-
         def index_name_for_remove(table_name, options = {})
           index_name = index_name(table_name, options)
 

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -146,28 +146,24 @@ def adapter_name
         'Abstract'
       end
 
-      # Does this adapter support migrations? Backend specific, as the
-      # abstract adapter always returns +false+.
+      # Does this adapter support migrations?
       def supports_migrations?
         false
       end
 
       # Can this adapter determine the primary key for tables not attached
-      # to an Active Record class, such as join tables? Backend specific, as
-      # the abstract adapter always returns +false+.
+      # to an Active Record class, such as join tables?
       def supports_primary_key?
         false
       end
 
-      # Does this adapter support using DISTINCT within COUNT? This is +true+
-      # for all adapters except sqlite.
+      # Does this adapter support using DISTINCT within COUNT?
       def supports_count_distinct?
         true
       end
 
       # Does this adapter support DDL rollbacks in transactions? That is, would
-      # CREATE TABLE or ALTER TABLE get rolled back by a transaction? PostgreSQL,
-      # SQL Server, and others support this. MySQL and others do not.
+      # CREATE TABLE or ALTER TABLE get rolled back by a transaction?
       def supports_ddl_transactions?
         false
       end
@@ -176,16 +172,14 @@ def supports_bulk_alter?
         false
       end
 
-      # Does this adapter support savepoints? PostgreSQL and MySQL do,
-      # SQLite < 3.6.8 does not.
+      # Does this adapter support savepoints?
       def supports_savepoints?
         false
       end
 
       # Should primary key values be selected from their corresponding
       # sequence before the insert statement? If true, next_sequence_value
       # is called before each insert to set the record's primary key.
-      # This is false for all adapters but Firebird.
       def prefetch_primary_key?(table_name = nil)
         false
       end
@@ -200,8 +194,7 @@ def supports_partial_index?
         false
       end
 
-      # Does this adapter support explain? As of this writing sqlite3,
-      # mysql2, and postgresql are the only ones that do.
+      # Does this adapter support explain?
       def supports_explain?
         false
       end
@@ -211,12 +204,17 @@ def supports_transaction_isolation?
         false
       end
 
-      # Does this adapter support database extensions? As of this writing only
-      # postgresql does.
+      # Does this adapter support database extensions?
       def supports_extensions?
         false
       end
 
+      # Does this adapter support creating indexes in the same statement as
+      # creating the table?
+      def supports_indexes_in_create?
+        false
+      end
+
       # This is meant to be implemented by the adapters that support extensions
       def disable_extension(name)
       end
@@ -225,14 +223,12 @@ def disable_extension(name)
       def enable_extension(name)
       end
 
-      # A list of extensions, to be filled in by adapters that support them. At
-      # the moment only postgresql does.
+      # A list of extensions, to be filled in by adapters that support them.
       def extensions
         []
       end
 
       # A list of index algorithms, to be filled by adapters that support them.
-      # MySQL and PostgreSQL have support for them right now.
       def index_algorithms
         {}
       end
@@ -293,7 +289,6 @@ def clear_cache!
       end
 
       # Returns true if its required to reload the connection between requests for development mode.
-      # This is not the case for Ruby/MySQL and it's not necessary for any adapters except SQLite.
       def requires_reloading?
         false
       end

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -6,12 +6,25 @@ class AbstractMysqlAdapter < AbstractAdapter
       include Savepoints
 
       class SchemaCreation < AbstractAdapter::SchemaCreation
-
         def visit_AddColumn(o)
           add_column_position!(super, column_options(o))
         end
 
         private
+
+        def visit_TableDefinition(o)
+          name = o.name
+          create_sql = "CREATE#{' TEMPORARY' if o.temporary} TABLE #{quote_table_name(name)} "
+
+          statements = o.columns.map { |c| accept c }
+          statements.concat(o.indexes.map { |column_name, options| index_in_create(name, column_name, options) })
+
+          create_sql << "(#{statements.join(', ')}) " if statements.present?
+          create_sql << "#{o.options}"
+          create_sql << " AS #{@conn.to_sql(o.as)}" if o.as
+          create_sql
+        end
+
         def visit_ChangeColumnDefinition(o)
           column = o.column
           options = o.options
@@ -29,6 +42,11 @@ def add_column_position!(sql, options)
           end
           sql
         end
+
+        def index_in_create(table_name, column_name, options)
+          index_name, index_type, index_columns, index_options, index_algorithm, index_using = @conn.add_index_options(table_name, column_name, options)
+          "#{index_type} INDEX #{quote_column_name(index_name)} #{index_using} (#{index_columns})#{index_options} #{index_algorithm}"
+        end
       end
 
       def schema_creation
@@ -225,6 +243,10 @@ def supports_transaction_isolation?
         version[0] >= 5
       end
 
+      def supports_indexes_in_create?
+        true
+      end
+
       def native_database_types
         NATIVE_DATABASE_TYPES
       end

activerecord/test/cases/adapters/mysql/active_schema_test.rb

@@ -17,7 +17,8 @@ def execute(sql, name = nil) return sql end
   end
 
   def test_add_index
-    # add_index calls index_name_exists? which can't work since execute is stubbed
+    # add_index calls table_exists? and index_name_exists? which can't work since execute is stubbed
+    def (ActiveRecord::Base.connection).table_exists?(*); true; end
     def (ActiveRecord::Base.connection).index_name_exists?(*); false; end
 
     expected = "CREATE  INDEX `index_people_on_last_name`  ON `people` (`last_name`) "
@@ -116,6 +117,18 @@ def test_remove_timestamps
     end
   end
 
+  def test_indexes_in_create
+    ActiveRecord::Base.connection.stubs(:table_exists?).with(:temp).returns(false)
+    ActiveRecord::Base.connection.stubs(:index_name_exists?).with(:index_temp_on_zip).returns(false)
+
+    expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip`  (`zip`) ) ENGINE=InnoDB AS SELECT id, name, zip FROM a_really_complicated_query"
+    actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t|
+      t.index :zip
+    end
+
+    assert_equal expected, actual
+  end
+
   private
     def with_real_execute
       ActiveRecord::Base.connection.singleton_class.class_eval do

activerecord/test/cases/adapters/mysql2/active_schema_test.rb

@@ -17,7 +17,8 @@ def execute(sql, name = nil) return sql end
   end
 
   def test_add_index
-    # add_index calls index_name_exists? which can't work since execute is stubbed
+    # add_index calls table_exists? and index_name_exists? which can't work since execute is stubbed
+    def (ActiveRecord::Base.connection).table_exists?(*); true; end
     def (ActiveRecord::Base.connection).index_name_exists?(*); false; end
 
     expected = "CREATE  INDEX `index_people_on_last_name`  ON `people` (`last_name`) "
@@ -116,6 +117,18 @@ def test_remove_timestamps
     end
   end
 
+  def test_indexes_in_create
+    ActiveRecord::Base.connection.stubs(:table_exists?).with(:temp).returns(false)
+    ActiveRecord::Base.connection.stubs(:index_name_exists?).with(:index_temp_on_zip).returns(false)
+
+    expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip`  (`zip`) ) ENGINE=InnoDB AS SELECT id, name, zip FROM a_really_complicated_query"
+    actual = ActiveRecord::Base.connection.create_table(:temp, temporary: true, as: "SELECT id, name, zip FROM a_really_complicated_query") do |t|
+      t.index :zip
+    end
+
+    assert_equal expected, actual
+  end
+
   private
     def with_real_execute
       ActiveRecord::Base.connection.singleton_class.class_eval do