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

Alter {users,job_definitions}.id to bigint #117

Merged
merged 1 commit into from
May 21, 2018
Merged

Conversation

riseshia
Copy link
Contributor

There is the problem that some tables has different type of primary key from
its installed:

  • before 0.2.0: using unsigned int for primary key
  • after 0.2.0: using signed int for primary key

This problem was made by fa75b20.

This makes hard to keep consistency of code / infra.
For instance, we need to care about 2 types of primary when adding foreign key.
Actually, this is already blocker of #110

To solve the problem, this patch proposes to alter these primary key to
bigint, which is next default type of rails.

Please review this @cookpad/dev-infra
cc @takonomura

@eagletmt
Copy link
Member

Could you update spec/dummy/db/schema.rb by running the migration?

@riseshia riseshia force-pushed the alter branch 2 times, most recently from 88a2658 to 366e186 Compare May 18, 2018 02:37
@riseshia
Copy link
Contributor Author

riseshia commented May 18, 2018

Changed(rebased):

  • apply migration to db/schema
    • duplicated table definition is deleted.
  • Respect current migration when rollback
    • meaning to say, these 2 columns will be signed int after rollback
  • add auto_increment which seem to be lacked when change_type ;)

@@ -0,0 +1,37 @@
class AlterKeyAndIndexToBigint < ActiveRecord::Migration[5.0]
def up
change_column :job_definitions, :id, :bigint, unsigned: false, auto_increment: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is unsigned: false needed? Isn't it default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right ;) I removed it. (rebased)

There is the problem that some tables has different type of primary key from
its installed:

- before 0.2.0: using unsigned int for primary key
- after 0.2.0: using signed int for primary key

This problem was made by fa75b20.

This makes hard to keep consistency of code / infra.
For instance, we need to care about 2 types of primary when adding foreign key.
Actually, this is already blocker of cookpad#110

To solve the problem, this patch proposes to alter these primary key to
bigint, which is next default type of rails.
@riseshia riseshia merged commit 8b4bb4d into cookpad:master May 21, 2018
@riseshia riseshia deleted the alter branch May 21, 2018 00:43
@riseshia riseshia mentioned this pull request May 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants