Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove default ENGINE=InnoDB for Mysql2 adapter #31177

Merged
merged 2 commits into from
Dec 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def table_comment(table_name) # :nodoc:
end

def create_table(table_name, **options) #:nodoc:
super(table_name, options: "ENGINE=InnoDB", **options)
super(table_name, options: "", **options)
end

def bulk_change_table(table_name, operations) #:nodoc:
Expand Down
8 changes: 8 additions & 0 deletions activerecord/lib/active_record/migration/compatibility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ def change_column(table_name, column_name, type, options = {})
super
end
end

def create_table(table_name, options = {})
if adapter_name == "Mysql2"
super(table_name, options: "ENGINE=InnoDB", **options)
else
super
end
end
end

class V5_0 < V5_1
Expand Down
6 changes: 3 additions & 3 deletions activerecord/test/cases/adapters/mysql2/active_schema_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,14 @@ def test_index_in_create
def (ActiveRecord::Base.connection).data_source_exists?(*); false; end

%w(SPATIAL FULLTEXT UNIQUE).each do |type|
expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name` (`last_name`)) ENGINE=InnoDB"
expected = "CREATE TABLE `people` (#{type} INDEX `index_people_on_last_name` (`last_name`)) "
actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t|
t.index :last_name, type: type
end
assert_equal expected, actual
end

expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10))) ENGINE=InnoDB"
expected = "CREATE TABLE `people` ( INDEX `index_people_on_last_name` USING btree (`last_name`(10))) "
actual = ActiveRecord::Base.connection.create_table(:people, id: false) do |t|
t.index :last_name, length: 10, using: :btree
end
Expand Down Expand Up @@ -160,7 +160,7 @@ def test_indexes_in_create
ActiveRecord::Base.connection.stubs(:data_source_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"
expected = "CREATE TEMPORARY TABLE `temp` ( INDEX `index_temp_on_zip` (`zip`)) 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
Expand Down
75 changes: 75 additions & 0 deletions activerecord/test/cases/adapters/mysql2/table_options_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,78 @@ def teardown
assert_match %r{COLLATE=utf8mb4_bin}, options
end
end

class Mysql2DefaultEngineOptionSchemaDumpTest < ActiveRecord::Mysql2TestCase
include SchemaDumpingHelper
self.use_transactional_tests = false

def setup
@verbose_was = ActiveRecord::Migration.verbose
ActiveRecord::Migration.verbose = false
end

def teardown
ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true
ActiveRecord::Migration.verbose = @verbose_was
ActiveRecord::SchemaMigration.delete_all rescue nil
end

test "schema dump includes ENGINE=InnoDB if not provided" do
ActiveRecord::Base.connection.create_table "mysql_table_options", force: true

output = dump_table_schema("mysql_table_options")
options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
assert_match %r{ENGINE=InnoDB}, options
end

test "schema dump includes ENGINE=InnoDB in legacy migrations" do
migration = Class.new(ActiveRecord::Migration[5.1]) do
def migrate(x)
create_table "mysql_table_options", force: true
end
end.new

ActiveRecord::Migrator.new(:up, [migration]).migrate

output = dump_table_schema("mysql_table_options")
options = %r{create_table "mysql_table_options", options: "(?<options>.*)"}.match(output)[:options]
assert_match %r{ENGINE=InnoDB}, options
end
end

class Mysql2DefaultEngineOptionSqlOutputTest < ActiveRecord::Mysql2TestCase
self.use_transactional_tests = false

def setup
@logger_was = ActiveRecord::Base.logger
@log = StringIO.new
@verbose_was = ActiveRecord::Migration.verbose
ActiveRecord::Base.logger = ActiveSupport::Logger.new(@log)
ActiveRecord::Migration.verbose = false
end

def teardown
ActiveRecord::Base.logger = @logger_was
ActiveRecord::Migration.verbose = @verbose_was
ActiveRecord::Base.connection.drop_table "mysql_table_options", if_exists: true
ActiveRecord::SchemaMigration.delete_all rescue nil
end

test "new migrations do not contain default ENGINE=InnoDB option" do
ActiveRecord::Base.connection.create_table "mysql_table_options", force: true

assert_no_match %r{ENGINE=InnoDB}, @log.string
end

test "legacy migrations contain default ENGINE=InnoDB option" do
migration = Class.new(ActiveRecord::Migration[5.1]) do
def migrate(x)
create_table "mysql_table_options", force: true
end
end.new

ActiveRecord::Migrator.new(:up, [migration]).migrate

assert_match %r{ENGINE=InnoDB}, @log.string
end
end
3 changes: 1 addition & 2 deletions guides/source/active_record_migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,7 @@ create_table :products, options: "ENGINE=BLACKHOLE" do |t|
end
```

will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table
(when using MySQL or MariaDB, the default is `ENGINE=InnoDB`).
will append `ENGINE=BLACKHOLE` to the SQL statement used to create the table.

Also you can pass the `:comment` option with any description for the table
that will be stored in database itself and can be viewed with database administration
Expand Down