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

MySQL: Fix schema dumping enum and set columns correctly #36604

Merged
merged 1 commit into from Jul 6, 2019

Conversation

@kamipo
Copy link
Member

@kamipo kamipo commented Jul 5, 2019

enum and set are typed cast as :string, but currently the
:string type is incorrectly reused for schema dumping.

A cast type on columns is not always the same with sql_type, this
fixes schema dumping enum and set columns to use sql_type instead
of type correctly.

`enum` and `set` are typed cast as `:string`, but currently the
`:string` type is incorrectly reused for schema dumping.

A cast type on columns is not always the same with `sql_type`, this
fixes schema dumping `enum` and `set` columns to use `sql_type` instead
of `type` correctly.
@kamipo kamipo merged commit a05a9b0 into rails:master Jul 6, 2019
2 checks passed
@kamipo kamipo deleted the fix_schema_dumping_enum branch Jul 6, 2019
kamipo added a commit that referenced this issue Jul 8, 2019
MySQL: Fix schema dumping `enum` and `set` columns correctly
@dudo
Copy link

@dudo dudo commented Jul 9, 2019

Any hope of a similar fix for Postgres?

kamipo added a commit to kamipo/rails that referenced this issue May 24, 2020
Before rails#36604, `enum` and `set` columns were incorrectly dumped as a
`string` column.

If an `enum` column is defined as `foo enum('apple','orange')`, it was
dumped as `t.string :foo, limit: 6`, the `limit: 6` is seemed to
restrict invalid string longer than `'orange'`.

But now, `enum` and `set` columns are correctly dumped as `enum` and
`set` columns, the limit as longest element is no longer used.
@ghiculescu
Copy link
Member

@ghiculescu ghiculescu commented Feb 16, 2021

@dudo just came across this PR recently and had the same thought. Check out #41469

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

3 participants