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

Support default expression and expression indexes for MySQL #34307

Merged
merged 2 commits into from Oct 26, 2018

Conversation

Projects
None yet
2 participants
@kamipo
Member

kamipo commented Oct 25, 2018

MySQL 8.0.13 is released a few days ago.

https://mysqlserverteam.com/the-mysql-8-0-13-maintenance-release-is-generally-available/

Now we can supports default expression and expression indexes for mysql2 adapter.

@rails-bot rails-bot bot added the activerecord label Oct 25, 2018

kamipo added some commits Oct 24, 2018

Support expression indexes for MySQL
MySQL 8.0.13 and higher supports functional key parts that index
expression values rather than column or column prefix values.

https://dev.mysql.com/doc/refman/8.0/en/create-index.html
Support default expression for MySQL
MySQL 8.0.13 and higher supports default value to be a function or
expression.

https://dev.mysql.com/doc/refman/8.0/en/create-table.html
lengths = options.delete(:lengths)
columns = index[-2].map { |name|
[ name.to_sym, expressions[name] || +quote_column_name(name) ]

This comment has been minimized.

@christophemaximin

christophemaximin Oct 25, 2018

Contributor

Why would we want to dup quote_column_name(name) here?

This comment has been minimized.

@kamipo

kamipo Oct 25, 2018

Member

This is because the quoted columns would be mutated by add_index_length and add_index_sort_order:

def add_index_length(quoted_columns, **options)
lengths = options_for_index_columns(options[:length])
quoted_columns.each do |name, column|
column << "(#{lengths[name]})" if lengths[name].present?

def add_index_sort_order(quoted_columns, **options)
orders = options_for_index_columns(options[:order])
quoted_columns.each do |name, column|
column << " #{orders[name].upcase}" if orders[name].present?

This comment has been minimized.

@christophemaximin

christophemaximin Oct 26, 2018

Contributor

Ah I see, I thought quote_column_name would return something that was already duped somehow... because it is the case of

def quote_column_name(column_name)
column_name.to_s
end

... but I didn't realise the method may always return the same object like ...
def quote_column_name(name)
@quoted_column_names[name] ||= "`#{super.gsub('`', '``')}`"
end

... got it :)

I'm just realising that there's no way (AFAIK) to know whether a method returns an object that is referenced elsewhere or not, which is kind of sad, because we may be duplicating strings for no reason (if a method returns a new object or an existing object), maybe we should do something about that one day.

This comment has been minimized.

@christophemaximin

christophemaximin Oct 26, 2018

Contributor

Nevermind, I misread this whole thing 👍

@kamipo kamipo merged commit 9e9e2b7 into rails:master Oct 26, 2018

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kamipo kamipo deleted the kamipo:mysql_expression_support branch Oct 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment