Support AR 6.1.0 by alpaca-tc · Pull Request #323 · ridgepole/ridgepole

alpaca-tc · 2020-12-27T15:15:44Z

previous PR #331

Affected changes in 6.1.0

Deprecate `connection_config`

ActiveRecord::Base. connection_config is deprecated. Use ActiveRecord::Base.connection_db_config.configuration_hash instead of.

Default engine `ENGINE=InnoDB` is no longer dumped to make schema more agnostic

# before
create_table(:items, id: :bigint, unsigned: true, options: 'ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci') {}

# after
create_table(:items, id: { type: :bigint, unsigned: true }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci") {}

"ENGINE=InnoDB" is deleted because it is default engine.
Added charset/collation arguments
Introduced new-style id options id: { type: :bigint, unsigned: true }

Refactor index creation to use index definition visitor

add_index have been refactored. Currently, add_index_options returns only three parameters.
More arguments are now required for remove_index calls.

Summary

Add support for Rails 6.1.

Use ActiveRecord::Base.connection_db_config.configuration_hash instead of ActiveRecord::Base. connection_config.
Dispatch add_index/remove_index methods in UseAlterIndex because it doesn't work in 6.1.0.
Don't dump charset/collation when without_table_options is enabled. Because charset/collations options are same as old-style raw table options.
Support new-style table options
- Aggregate table options to compare new-style table options(charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci") with old-style options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci". New style table options will be converted to old-style options in Ridgepole::Diff.
- Normalize pk options(id: { type: :bigint, unsigned: true } or id: :bigint, unsigned: true) to same result to compare those styles.
Added testing with Rails 6.1

I'm not sure test policy, so I did not add new test. But if need, please let me know.

lib/ridgepole/diff.rb

lib/ridgepole/default_limit.rb

lib/ridgepole/external_sql_executer.rb

winebarrel · 2020-12-28T02:57:00Z

Thank you for Pull Request 😃
But I'm sorry, I think it should not normalize the 6.1 DSL because the code is too complicated.
~~I think it should fix the schema file.~~

If ridgepole users use Active Record 6.1, I think ridgepole users should use Active Record 6.1 DSL.

winebarrel · 2020-12-28T07:32:46Z

lib/ridgepole/ext/abstract_mysql_adapter/use_alter_index.rb

      module UseAlterIndex
-        def add_index(table_name, column_name, options = {})
-          index_name, index_type, index_columns, index_options, _index_algorithm, index_using = add_index_options(table_name, column_name, **options)
+        if ActiveRecord.gem_version >= Gem::Version.new('6.1.0')


I think it would be better to remove the --mysql-use-alter option.
This option was added to alter add index to add an algorithm_option/lock_option (LOCK=NONE, ALGORITHM=INPLACE).
However, CREATE INDEX can also add algorithm_option/lock_option, so modify the --alter-extra option.

(Please wait a moment while I create a pull request.)

winebarrel · 2020-12-28T07:42:41Z

I think it's better to tolerate incompatibilities for AR 6.1 support, so I created a 0.9 branch.
https://github.com/winebarrel/ridgepole/tree/0.9

@alpaca-tc Would you please change the base branch of the pull request?

alpaca-tc · 2020-12-29T00:10:46Z

Thank you for Pull Request 😃
But I'm sorry, I think it should not normalize the 6.1 DSL because the code is too complicated.
I think it should fix the schema file.

If ridgepole users use Active Record 6.1, I think ridgepole users should use Active Record 6.1 DSL.

Okay, I'll try to fix. 😄
Maybe I can fix table options but I don't know how to fix pk options. Do you have any ideas?

alpaca-tc · 2020-12-29T01:16:06Z

@winebarrel hmm... 🤔
If ridgepole doesn't normalize table_options, spec/mysql/migrate/migrate_change_column8_spec.rb will be fail. Currently, ridgepole supports default table_options as string but AR dumps table_options to charset/collation as hash in 6.1. Therefore, ridgepole detects diff every time.

I'll try to add new options to set default hash parameters because I want to set unsigned/charset/collation as default parameters.

Ooops, --table-options can't pass hash parameters... 😢 hmm, what should i do...

winebarrel · 2020-12-31T03:37:27Z

Ooops, --table-options can't pass hash parameters... 😢 hmm, what should i do...

How about adding an option to pass a hash parameter?
e.g. --table-hash-options='{"foo":"bar"}'

related: ridgepole#323 (comment) Added option `--table-hash-options` to set default hash table options. ``` ridgepole --table-hash-options='{ unsigned: true, charset: "utf8mb4" }' ```

related: ridgepole#323 (comment) Added option `--table-hash-options` to set default hash table options. ``` ridgepole --table-hash-options='{ "id": { "unsigned": true }, "charset": "utf8mb4" }' ```

table options are parsed charset/collation in 6.1.0. related: rails/rails#39365 ``` // before // < 6.1.0 create_table "users", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci" do |t| end // after // >= 6.1.0 create_table "users", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci" do |t| end ``` If `dump_without_table_options` is disabled, ridgepole doesn't dump charset/collation.

In 6.1, table options are parsed to charset/collation. rails/rails#39365 The following cases should be same result. ``` create_table "items", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci" do |t| end create_table "items", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci" do |t| end ``` To solve this I handled that if new-style `charset/collation` given, it converts to old-style `options`. By doing so, I solved the problem by a little changes. https://github.com/rails/rails/pull/39365/files#diff-868f1dccfcbed26a288bf9f3fd8a39c863a4413ab0075e12b6805d9798f556d1R427

rails/rails#39365 introduced new-style id options like the followings. ``` // < 6.1 old-style create_table(:items, id: :bigint, unsigned: true) {} // >= 6.1 new-style create_table(:items, id: { type: :bigint, unsigned: true }) {} ``` To compare those styles to get diff, I handled that I normalizes id options from those styles.

Unary plus operator is faster than `dup`.

winebarrel · 2021-01-02T14:52:37Z

The test code is redundant, why don't you modify it as follows?

https://github.com/winebarrel/ridgepole/blob/0.9/spec/erb_helper.rb#L16

ERBh.define_method(:cond) do |conds, m, e = nil|
  if conds.is_a?(Hash)
    return conds.find do |c, _|
      condition(*Array(c))
    end&.last || m
  end

  if condition(*Array(conds))
    m
  else
    e || (begin
      m.class.new
    rescue StandardError
      nil
    end)
  end
end

usage

        create_table "employee_clubs", <%= i cond(">= 5.1, < 6.1" => {id: :bigint}, ">= 6.1" => { id: { type: :bigint, unsigned: true } }, { unsigned: true }) %>, force: :cascade do |t|

related: ridgepole#323 (comment)

alpaca-tc · 2021-01-03T12:14:02Z

@winebarrel all done 🎉

alpaca-tc · 2021-01-03T12:19:04Z

spec/erb_helper.rb

 end

 ERBh.define_method(:cond) do |conds, m, e = nil|
-  if condition(*Array(conds))


Currently conds is only Numeric or String so *Array() is not needed.

winebarrel

LGTM
Thank you!!!

winebarrel · 2021-01-03T17:15:58Z

v0.9.0.beta has been released

alpaca-tc force-pushed the support_rails6_1 branch 2 times, most recently from a3b47f5 to 4f8edc7 Compare December 27, 2020 15:37

alpaca-tc changed the title ~~Support AR 6.1.0~~ [WIP] Support AR 6.1.0 Dec 27, 2020

alpaca-tc force-pushed the support_rails6_1 branch 4 times, most recently from 4296a73 to 87bdb2f Compare December 27, 2020 16:39

alpaca-tc changed the title ~~[WIP] Support AR 6.1.0~~ Support AR 6.1.0 Dec 27, 2020

kamipo reviewed Dec 27, 2020

View reviewed changes

lib/ridgepole/diff.rb Outdated Show resolved Hide resolved

winebarrel reviewed Dec 28, 2020

View reviewed changes

lib/ridgepole/default_limit.rb Outdated Show resolved Hide resolved

winebarrel reviewed Dec 28, 2020

View reviewed changes

lib/ridgepole/external_sql_executer.rb Outdated Show resolved Hide resolved

alpaca-tc force-pushed the support_rails6_1 branch from 87bdb2f to a023667 Compare December 28, 2020 02:58

winebarrel reviewed Dec 28, 2020

View reviewed changes

alpaca-tc changed the base branch from 0.8 to 0.9 December 28, 2020 23:58

winebarrel mentioned this pull request Dec 31, 2020

Remove --mysql-use-alter option #330

Merged

alpaca-tc mentioned this pull request Jan 1, 2021

Support hash table options with --table-hash-options #331

Merged

alpaca-tc force-pushed the support_rails6_1 branch from a023667 to 43ab113 Compare January 1, 2021 14:42

alpaca-tc force-pushed the support_rails6_1 branch from c5bb5e1 to a028f00 Compare January 1, 2021 16:53

alpaca-tc force-pushed the support_rails6_1 branch from a028f00 to 0e18f30 Compare January 1, 2021 16:55

alpaca-tc added 9 commits January 2, 2021 23:18

ActiveRecord::Base.connection_config is deprecated in 6.1.0

8e0906c

Fixed specs

07aeff5

Don't normalize table options

17b9f3a

Replace the pk method with simple way

b1b3e7b

Update changelog

d057683

Use unary plus operator to unfreeze string

f4f7f77

Unary plus operator is faster than `dup`.

alpaca-tc force-pushed the support_rails6_1 branch from fc7df19 to eaf9fa0 Compare January 2, 2021 14:30

Remove version checking

e2dbafe

alpaca-tc force-pushed the support_rails6_1 branch from eaf9fa0 to e2dbafe Compare January 2, 2021 14:42

alpaca-tc added a commit to alpaca-tc/ridgepole that referenced this pull request Jan 3, 2021

cond supports hash conditions

67ffbb5

related: ridgepole#323 (comment)

alpaca-tc added a commit to alpaca-tc/ridgepole that referenced this pull request Jan 3, 2021

cond supports hash conditions

ee90f98

related: ridgepole#323 (comment)

alpaca-tc force-pushed the support_rails6_1 branch from 67ffbb5 to ee90f98 Compare January 3, 2021 11:41

cond supports hash conditions

10c64a3

related: ridgepole#323 (comment)

alpaca-tc force-pushed the support_rails6_1 branch 2 times, most recently from f4d2db0 to 1121e64 Compare January 3, 2021 12:03

Testing with rails 6.1

fd3e271

alpaca-tc force-pushed the support_rails6_1 branch from 1121e64 to fd3e271 Compare January 3, 2021 12:09

alpaca-tc commented Jan 3, 2021

View reviewed changes

winebarrel approved these changes Jan 3, 2021

View reviewed changes

winebarrel merged commit 8dc9862 into ridgepole:0.9 Jan 3, 2021

alpaca-tc deleted the support_rails6_1 branch January 3, 2021 12:55

winebarrel mentioned this pull request Jan 3, 2021

Add notice for AR 6.1 #333

Merged

winebarrel mentioned this pull request Jun 26, 2021

Creating a string Primary Key not called "id" #360

Closed

kmasuda-aiming mentioned this pull request Jan 4, 2022

Unexpected behavior in applying with DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP statement #375

Closed

Comments

Conversation

alpaca-tc commented Dec 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Affected changes in 6.1.0

Deprecate connection_config

Default engine ENGINE=InnoDB is no longer dumped to make schema more agnostic

Refactor index creation to use index definition visitor

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

winebarrel commented Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

winebarrel Dec 28, 2020

Choose a reason for hiding this comment

Uh oh!

winebarrel Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

winebarrel commented Dec 28, 2020

Uh oh!

alpaca-tc commented Dec 29, 2020

Uh oh!

alpaca-tc commented Dec 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

winebarrel commented Dec 31, 2020

Uh oh!

winebarrel commented Jan 2, 2021

Uh oh!

alpaca-tc commented Jan 3, 2021

Uh oh!

alpaca-tc Jan 3, 2021

Choose a reason for hiding this comment

Uh oh!

winebarrel left a comment

Choose a reason for hiding this comment

Uh oh!

winebarrel commented Jan 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alpaca-tc commented Dec 27, 2020 •

edited

Loading

Deprecate `connection_config`

Default engine `ENGINE=InnoDB` is no longer dumped to make schema more agnostic

winebarrel commented Dec 28, 2020 •

edited

Loading

alpaca-tc commented Dec 29, 2020 •

edited

Loading