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

Correctly dump :options on create_table for MySQL #17569

Merged
merged 1 commit into from May 3, 2015

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Nov 9, 2014

Related with #12172.

Is often used to specify table_options in MySQL.

(For example, "ENGINE=MyISAM" for full-text search, "ENGINE=InnoDB ROW_FORMAT=DYNAMIC" or "ENGINE=InnoDB ROW_FORMAT=COMPRESSED KEY_BLOCK_SIZE=8" for use InnoDB of Barracuda File Format.)

SchemaDumper should support a dump of table_options.

'cp932' => 'cp932_japanese_ci',
'eucjpms' => 'ucjpms_japanese_ci'
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure these are necessary. If collation is omitted, MySQL will use its default for that charset.

@jeremy
Copy link
Member

jeremy commented Nov 9, 2014

👍 to the idea. It's important to support Barracuda formats.

@kamipo
Copy link
Member Author

kamipo commented Nov 15, 2014

I fixed to dump table_options explicitly.
When #17569 and #17574 will be merged, then can be correctly restored charset and collation without depend on the database default charset and collation.

@kamipo kamipo force-pushed the dump_table_options branch 2 times, most recently from 9a69d48 to 7f1606d Compare December 4, 2014 01:56
@rafaelfranca
Copy link
Member

@matthewd any concern about this one? It will update all the schema tables to explicitly set the options even if it is the default. I'm fine with it but you commented about this possible problem here #12172 (comment)

@matthewd
Copy link
Member

matthewd commented Jan 3, 2015

👍; avoiding a bulk schema change does seem slightly desirable, but not important. And explicitness has its own value here, too.

@mtparet
Copy link

mtparet commented Feb 19, 2015

👍 I have currently to monkey patch activerecord to correctly create table from the schema and with the rights options (We're verifying that we able to store utf8bm4 character like 𝌆 for example.

@kamipo kamipo changed the title Add SchemaDumper support table_options for MySQL. Correctly dump :options on create_table for MySQL May 3, 2015
@kamipo
Copy link
Member Author

kamipo commented May 3, 2015

@rafaelfranca Could you merge the changes?

@rafaelfranca rafaelfranca merged commit 22ba159 into rails:master May 3, 2015
rafaelfranca added a commit that referenced this pull request May 3, 2015
Correctly dump `:options` on `create_table` for MySQL
@kamipo kamipo deleted the dump_table_options branch May 3, 2015 22:00
@mtparet
Copy link

mtparet commented May 4, 2015

👍

kamipo added a commit to kamipo/rails that referenced this pull request Aug 25, 2017
I was added a table options after `force: :cascade` in rails#17569 for not
touching existing tests (reducing diff). But `force: :cascade` is not an
important information. So I prefer to place a table options before
`force: :cascade`.
tgxworld pushed a commit to tgxworld/rails that referenced this pull request Aug 27, 2017
I was added a table options after `force: :cascade` in rails#17569 for not
touching existing tests (reducing diff). But `force: :cascade` is not an
important information. So I prefer to place a table options before
`force: :cascade`.
kamipo added a commit to kamipo/rails that referenced this pull request May 23, 2020
…e agnostic

5 years ago, I made dumping full table options at rails#17569, especially to
dump `ENGINE=InnoDB ROW_FORMAT=DYNAMIC` to use utf8mb4 with large key
prefix.

In that time, omitting the default engine `ENGINE=InnoDB` was not useful
since `ROW_FORMAT=DYNAMIC` always remains as long as using utf8mb4 with
large key prefix.

But now, MySQL 5.7.9 has finally changed the default row format to
DYNAMIC, utf8mb4 with large key prefix can be used without dumping the
default engine and the row format explicitly.

So now is a good time to make the default engine is omitted.

Before:

```ruby
create_table "accounts", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci", force: :cascade do |t|
end
```

After:

```ruby
create_table "accounts", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
end
```

To entirely omit `:options` option to make schema agnostic, I've added
`:charset` and `:collation` table options to exclude `CHARSET` and
`COLLATE` from `:options`.

Fixes rails#26209.
Closes rails#29472.

See also rails#33608, rails#33853, and rails#34742.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants