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

Support index.length for MySQL 8.0.0-dmr #26785

Merged
merged 1 commit into from
Oct 15, 2016
Merged

Conversation

yahonda
Copy link
Member

@yahonda yahonda commented Oct 13, 2016

Summary

This pull request addresses #26774 since MySQL 8.0.0-dmr SUB_PART column of information_schema.statistics changed to varbinary(12), which is bigint(3) in MySQL 5.6.

This pull request has been tested with both versions of MySQL.

Server version: 8.0.0-dmr MySQL Community Server (GPL)
Server version: 5.6.33 MySQL Community Server (GPL)

MySQL 8.0.0-dmr `SUB_PART` column of `information_schema.statistics`
changed to varbinary(12), which is bigint(3) in MySQL 5.6.

Addresses rails#26774
@rails-bot
Copy link

r? @kaspth

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

@rafaelfranca rafaelfranca merged commit 8e06900 into rails:master Oct 15, 2016
rafaelfranca added a commit that referenced this pull request Oct 15, 2016
Support index.length for MySQL 8.0.0-dmr
@rafaelfranca
Copy link
Member

Backported in 4cf4c86

@matthewd
Copy link
Member

The backport broke the build: https://travis-ci.org/rails/rails/jobs/168139524

(I haven't looked into why master doesn't have the same problem.)

@rafaelfranca
Copy link
Member

💣 I'll revert the backport :(. @yahonda could you investigate?

rafaelfranca added a commit that referenced this pull request Oct 17, 2016
This reverts commit 4cf4c86.

Reason: It broke the build
@kamipo
Copy link
Member

kamipo commented Oct 17, 2016

If row[:Sub_part] is nil, row[:Sub_part].to_i is 0.

@kamipo
Copy link
Member

kamipo commented Oct 17, 2016

Originally (index.lengths || []).compact have a bug.
https://github.com/rails/rails/blob/5-0-stable/activerecord/lib/active_record/schema_dumper.rb#L216
If #26785 will backport, it is better to backport #26745 as well.

@yahonda
Copy link
Member Author

yahonda commented Oct 17, 2016

let me check.

yahonda added a commit to yahonda/rails that referenced this pull request Oct 17, 2016
@yahonda
Copy link
Member Author

yahonda commented Oct 17, 2016

I have attempted to backport #26745 to 5-0-stable by cherry-pick 5025fd3 then it conflicts with #26155 commit a57cd38. It also conflicts with rubocop format 55f9b81

Then simply add another if condition to add length only if row[:Sub_part] is not nil. Let me open another pull request to 5-0-stable.

@yahonda
Copy link
Member Author

yahonda commented Oct 17, 2016

Opened #26806 for 5-0-stable branch.

@yahonda
Copy link
Member Author

yahonda commented Oct 17, 2016

Closed #26806 which would cause another regression. So far I'm leaving 5-0-stable current implementation as they are. Let me tackle it when I have some more time to backport commits described in #26785 (comment) without external spec change and conflicts.

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

7 participants