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

Validate if `utf8mb4` character set and longer index key prefix is supported #33853

Merged
merged 3 commits into from Sep 13, 2018

Conversation

@yahonda
Copy link
Contributor

yahonda commented Sep 12, 2018

Summary

Use utf8 character set by default if MySQL server does not support longer index key prefix

Once #33608 merged If users create a new database using MySQL 5.1.x, it will fail to create databases
since MySQL 5.1 does not know utf8mb4 character set.

This pull request removes encoding: utf8mb4 from mysql.yml.tt
to let create_database method handles default character set by MySQL server version.

Other Information

supports_longer_index_key_prefix? method will need to validate if MySQL 5.5 and 5.6 server configured correctly to support longer index key prefix, but not yet.

@rails-bot
Copy link

rails-bot commented Sep 12, 2018

r? @sgrif

(@rails-bot has picked a reviewer for you, use r? to override)

@yahonda yahonda force-pushed the yahonda:use_utf8mb4_only_if_available branch 2 times, most recently from 17f6a81 to 699a4e6 Sep 12, 2018
else
version >= "5.7.9"
end
end

This comment has been minimized.

@jeremy

jeremy Sep 12, 2018 Member

Should we feature-detect innodb_large_prefix instead of version check?

This comment has been minimized.

@yahonda

yahonda Sep 12, 2018 Author Contributor

Sure. I've been looking for what system variables are appropriate to see if the longer key prefix is configured.

  • innodb_large_prefix
    Introduced at MySQL 5.5, deprecated in MySQL 5.7 and removed in MySQL 8.0.

  • innodb_default_row_format
    Introduced at MySQL 5.7.9, does not exist in MySQL 5.6 or lower.

This comment has been minimized.

@jeremy

jeremy Sep 13, 2018 Member

This seems awkward to detect.

Another approach: optimistically use utf8mb4 charset, then rescue StatementInvalid exception and raise our explanatory RuntimeError.

else
execute "CREATE DATABASE #{quote_table_name(name)} DEFAULT CHARACTER SET #{quote_table_name(options[:charset] || 'utf8mb4')}"
execute "CREATE DATABASE #{quote_table_name(name)} DEFAULT CHARACTER SET `utf8`"

This comment has been minimized.

@jeremy

jeremy Sep 12, 2018 Member

Perhaps we can safely raise an exception now. If :charset is not specified and large prefixes / utf8mb4 are not supported, we don't want to silently default to 3-byte anymore.

This comment has been minimized.

@yahonda

yahonda Sep 12, 2018 Author Contributor

Agreed. updated to raise an exception in this case. Please review this also English grammar point of view.

@jeremy jeremy requested a review from kamipo Sep 12, 2018
else
version >= "5.7.9"
end
end

This comment has been minimized.

@jeremy

jeremy Sep 13, 2018 Member

This seems awkward to detect.

Another approach: optimistically use utf8mb4 charset, then rescue StatementInvalid exception and raise our explanatory RuntimeError.

else
execute "CREATE DATABASE #{quote_table_name(name)} DEFAULT CHARACTER SET #{quote_table_name(options[:charset] || 'utf8mb4')}"
raise "Your MySQL (#{version_string}) does not support utf8mb4 character and longer index key prefix. Configure `:charset` in your database.yml"

This comment has been minimized.

@jeremy

jeremy Sep 13, 2018 Member

Optimistic approach:

def create_database(name, options = {})
  if options[:collation]
    
  else
    charset = options.fetch(:charset, 'utf8mb4')
    execute "CREATE DATABASE #{quote_table_name(name)} DEFAULT CHARACTER SET #{quote_table_name(options[:charset])}"
  end
rescue ActiveRecord::StatementInvalid => error
  raise unless error.try(:error_number) == 1115  # Should we introduce UnknownCharset exception subclass?
  raise "Unsupported database character set. Configure a supported :charset in config/database.yml and ensure innodb_large_prefix is enabled to support indexes on varchar(255) string columns. #{error.message}"
end

This comment has been minimized.

@kamipo

kamipo Sep 13, 2018 Member

How about dropping MySQL 5.1 support?
MySQL 5.1 is only version that CREATE DATABASE `table_name` DEFAULT CHARACTER SET `utf8mb4` is failed which mysql2 adapter supported.

This comment has been minimized.

@yahonda

yahonda Sep 13, 2018 Author Contributor

Sounds good. Dropped MySQL 5.1 support and bump the minimum version of MySQL to 5.5.8.

@yahonda yahonda force-pushed the yahonda:use_utf8mb4_only_if_available branch 5 times, most recently from a022072 to 2a25bf6 Sep 13, 2018
@yahonda yahonda changed the title Use `utf8` character set by default if MySQL does not support longer index key prefix Validate if `utf8mb4` character set and longer index key prefix is supported Sep 13, 2018
…pported

Once #33608 merged If users create a new database using MySQL 5.1.x, it will fail to create databases
since MySQL 5.1 does not know `utf8mb4` character set.

This pull request removes `encoding: utf8mb4` from `mysql.yml.tt`
to let create_database method handles default character set by MySQL server version.

`supports_longer_index_key_prefix?` method will need to validate if MySQL 5.5 and 5.6 server configured
correctly to support longer index key prefix, but not yet.
else
execute "CREATE DATABASE #{quote_table_name(name)} DEFAULT CHARACTER SET #{quote_table_name(options[:charset] || 'utf8mb4')}"
raise "Configure a supported :charset in config/database.yml and ensure innodb_large_prefix is enabled to support indexes on varchar(255) string columns."

This comment has been minimized.

@kamipo

kamipo Sep 13, 2018 Member

Should we mention about config/database.yml in this method?
create_database could work without config/database.yml.

This comment has been minimized.

@yahonda

yahonda Sep 13, 2018 Author Contributor

I see. Removed config/database.yml from the message.

@yahonda yahonda force-pushed the yahonda:use_utf8mb4_only_if_available branch from 2a25bf6 to 7bed21a Sep 13, 2018
* MySQL 5.1 does not support `utf8mb4` character set

* MySQL 5.1 had been already EOLed on Dec 2013
https://www.mysql.com/support/eol-notice.html

> Per Oracle's Lifetime Support policy, as of December 31, 2013, MySQL 5.1
> is covered under Oracle Sustaining Support.

* MySQL 5.5.8 is the first General Availability of MySQL 5.5

https://dev.mysql.com/doc/relnotes/mysql/5.5/en/news-5-5-8.html
@yahonda yahonda force-pushed the yahonda:use_utf8mb4_only_if_available branch from 7bed21a to a338314 Sep 13, 2018
@jeremy jeremy added this to the 6.0.0 milestone Sep 13, 2018
@jeremy jeremy merged commit f89e2c2 into rails:master Sep 13, 2018
2 checks passed
2 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
yahonda added a commit to yahonda/rails that referenced this pull request Sep 13, 2018
yahonda added a commit to yahonda/rails that referenced this pull request Sep 13, 2018
Rails 6 drops MySQL 5.1 support and requires MySQL 5.5.8 by rails#33853
mysql2 gem 0.5.0 also drops MySQL 5.1 support.

https://github.com/brianmario/mysql2/releases/tag/0.5.0
> MySQL 5.5 or higher required. MySQL 5.0 and 5.1 are not supported.

Related to rails#31636 rails#32310 rails#33853
yahonda added a commit to yahonda/rails that referenced this pull request Sep 20, 2018
yahonda added a commit to yahonda/rails that referenced this pull request Sep 20, 2018
@yahonda yahonda deleted the yahonda:use_utf8mb4_only_if_available branch Sep 23, 2018
yahonda added a commit to yahonda/rails that referenced this pull request Oct 30, 2018
… and no `encoding:` specified

Since rails#33853 `config/database.yml` does not have `encoding:` entry by default.
If `supports_longer_index_key_prefix?` returns `true`, database character set becomes `utf8mb4`
also it should set client encoding to `utf8mb4`
yahonda added a commit to yahonda/rails that referenced this pull request Oct 30, 2018
… and no `encoding:` specified

Since rails#33853 `config/database.yml` does not have `encoding:` entry by default.
If `supports_longer_index_key_prefix?` returns `true`, database character set becomes `utf8mb4`
also it should set client encoding to `utf8mb4`
yahonda added a commit to yahonda/rails that referenced this pull request Oct 30, 2018
rails#33853 and rails#33929 removed `encoding: utf8mb4` from database.yml
since at that time MySQL 5.1 is supported with the master branch.

Since MySQL 5.1 has been dropped, we can restore `encoding: utf8mb4` in database.yml
kamipo added a commit that referenced this pull request Jan 2, 2019
Since we already bumped the minimum version of MySQL to 5.5.8 at #33853.
suketa added a commit to suketa/rails_sandbox that referenced this pull request May 25, 2019
Validate if `utf8mb4` character set and longer index key prefix is
supported
rails/rails#33853
kamipo added a commit to kamipo/rails that referenced this pull request May 20, 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.
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
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.