-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Conversation
r? @sgrif (@rails-bot has picked a reviewer for you, use r? to override) |
17f6a81
to
699a4e6
Compare
else | ||
version >= "5.7.9" | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we feature-detect innodb_large_prefix
instead of version check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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`" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. updated to raise an exception in this case. Please review this also English grammar point of view.
else | ||
version >= "5.7.9" | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Dropped MySQL 5.1 support and bump the minimum version of MySQL to 5.5.8.
a022072
to
2a25bf6
Compare
utf8
character set by default if MySQL does not support longer index key prefixutf8mb4
character set and longer index key prefix is supported
…pported Once rails#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." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention about config/database.yml
in this method?
create_database
could work without config/database.yml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Removed config/database.yml
from the message.
…utf8mb4 are not supported
2a25bf6
to
7bed21a
Compare
* 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
7bed21a
to
a338314
Compare
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
… 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`
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
Since we already bumped the minimum version of MySQL to 5.5.8 at #33853.
Validate if `utf8mb4` character set and longer index key prefix is supported rails/rails#33853
…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.
mysql2 gem dropped testing for MySQL 5.1 which was released in Dec 2008 and already EOL in Dec 2013 at efa47a9. If we no longer support MySQL 5.1, utf8 (utf8mb3) is not appropriate default encoding for MySQL 5.5 or higher, utf8mb4 should be set for that. FYI, Rails 6.0 no longer support MySQL 5.1 and set utf8mb4 by default. rails/rails#33608 rails/rails#33853
mysql2 gem dropped testing for MySQL 5.1 which was released in Dec 2008 and already EOL in Dec 2013 at efa47a9. If we no longer support MySQL 5.1, utf8 (utf8mb3) is not appropriate default encoding for MySQL 5.5 or higher, utf8mb4 should be set for that. FYI, Rails 6.0 no longer support MySQL 5.1 and set utf8mb4 by default. rails/rails#33608 rails/rails#33853
mysql2 gem dropped testing for MySQL 5.1 which was released in Dec 2008 and already EOL in Dec 2013 at efa47a9. If we no longer support MySQL 5.1, utf8 (utf8mb3) is not appropriate default encoding for MySQL 5.5 or higher, utf8mb4 should be set for that. FYI, Rails 6.0 no longer support MySQL 5.1 and set utf8mb4 by default. rails/rails#33608 rails/rails#33853
Summary
Use
utf8
character set by default if MySQL server does not support longer index key prefixOnce #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
frommysql.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.