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

make change_column_comment and change_table_comment invertible #35970

Merged
merged 1 commit into from Apr 15, 2019

Conversation

yskkin
Copy link
Contributor

@yskkin yskkin commented Apr 14, 2019

We can revert migrations using change_column_comment or
change_table_comment at current master.
However, results are not what we expect: comments are remained in new
status.
This change tells previous comment to these methods in a way like
change_column_default.

@yskkin yskkin force-pushed the reversible_comment branch 2 times, most recently from 0e48e87 to cedd4cd Compare April 14, 2019 13:45
We can revert migrations using `change_column_comment` or
`change_table_comment` at current master.
However, results are not what we expect: comments are remained in new
status.
This change tells previous comment to these methods in a way like
`change_column_default`.
@kamipo kamipo merged commit 3a5b124 into rails:master Apr 15, 2019
@yskkin yskkin deleted the reversible_comment branch April 15, 2019 00:29
koic added a commit to koic/oracle-enhanced that referenced this pull request Apr 15, 2019
Follow up rails/rails#35970.

This PR fixes the following `TypeError: can't quote Hash`.

```consle
% cd path/to/rails/activerecord
% ARCONN=oracle be ruby -w -Itest test/cases/invertible_migration_test.rb -n test_migrate_revert_change_table_comment
Using oracle
Run options: -n test_migrate_revert_change_table_comment --seed 42508

# Running:

E

Error:
ActiveRecord::InvertibleMigrationTest#test_migrate_revert_change_table_comment:
TypeError: can't quote Hash
    /home/vagrant/src/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb:178:in
    `_quote'
    /home/vagrant/src/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/quoting.rb:93:in
    `_quote'
    /home/vagrant/src/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb:18:in
    `quote'
    /home/vagrant/src/oracle-enhanced/lib/active_record/connection_adapters/oracle_enhanced/schema_statements.rb:485:in
    `change_table_comment'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:875:in
    `block in method_missing'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:843:in
    `block in say_with_time'
    /home/vagrant/.rvm/rubies/ruby-2.6.2/lib/ruby/2.6.0/benchmark.rb:293:in
    `measure'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:843:in
    `say_with_time'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:864:in
    `method_missing'
    test/cases/invertible_migration_test.rb:128:in `change'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:813:in
    `exec_migration'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:797:in
    `block (2 levels) in migrate'
    /home/vagrant/.rvm/rubies/ruby-2.6.2/lib/ruby/2.6.0/benchmark.rb:293:in
    `measure'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:796:in
    `block in migrate'
    /home/vagrant/src/rails/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:416:in
    `with_connection'
    /home/vagrant/src/rails/activerecord/lib/active_record/migration.rb:795:in
    `migrate'
    test/cases/invertible_migration_test.rb:356:in
    `test_migrate_revert_change_table_comment'

rails test test/cases/invertible_migration_test.rb:349

Finished in 0.106411s, 9.3975 runs/s, 9.3975 assertions/s.
1 runs, 1 assertions, 0 failures, 1 errors, 0 skips
```
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

2 participants