Deprecate `supports_migrations?` on connection adapters #28172

Merged
merged 1 commit into from Feb 26, 2017

Conversation

Projects
None yet
3 participants
@kamipo
Member

kamipo commented Feb 26, 2017

supports_migrations? was added at 4160b51 to determine if schema
statements (create_table, drop_table, etc) are implemented in the
adapter. But all tested databases has been supported migrations since
a4fc93c at least.

@pixeltrix

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Feb 26, 2017

Member

@kamipo are we sure that all the major third-party adapters support migrations too? Also are there any schema-less databases with AR adapters that might make use of this?

Member

pixeltrix commented Feb 26, 2017

@kamipo are we sure that all the major third-party adapters support migrations too? Also are there any schema-less databases with AR adapters that might make use of this?

@kamipo

This comment has been minimized.

Show comment
Hide comment
@kamipo

kamipo Feb 26, 2017

Member

are we sure that all the major third-party adapters support migrations too?

Most adapters (firebird, frontbase, mysql, oracle, postgresql, sqlite, sqlserver, sybase) were true at least as of Rails 1.2.

https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/firebird_adapter.rb#L286-L288
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/frontbase_adapter.rb#L270-L272
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb#L159-L161
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/oracle_adapter.rb#L133-L135
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L106-L108
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb#L101-L103
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb#L215-L217
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb#L340-L342

db2 and openbase adapters were false, but looks like that create_table, drop_table, add_column, remove_column, add_index, and remove_index works at least.

supports_migrations? was used for two purposes.

  1. to determine if schema statements (create_table, drop_table, etc) are implemented in the adapter in tests. But since a4fc93c any adapter never be able to run tests without create_table and drop_table are implemented at least (and supports_migrations? condition in migration_test.rb was removed at 2158a70).

  2. to determine to allow migration (rake db:migrate) to the adapter. I think that disallowing migration is unneeded because not only schema statements can be used in migration.

Member

kamipo commented Feb 26, 2017

are we sure that all the major third-party adapters support migrations too?

Most adapters (firebird, frontbase, mysql, oracle, postgresql, sqlite, sqlserver, sybase) were true at least as of Rails 1.2.

https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/firebird_adapter.rb#L286-L288
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/frontbase_adapter.rb#L270-L272
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb#L159-L161
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/oracle_adapter.rb#L133-L135
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L106-L108
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb#L101-L103
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/sqlserver_adapter.rb#L215-L217
https://github.com/rails/rails/blob/1-2-stable/activerecord/lib/active_record/connection_adapters/sybase_adapter.rb#L340-L342

db2 and openbase adapters were false, but looks like that create_table, drop_table, add_column, remove_column, add_index, and remove_index works at least.

supports_migrations? was used for two purposes.

  1. to determine if schema statements (create_table, drop_table, etc) are implemented in the adapter in tests. But since a4fc93c any adapter never be able to run tests without create_table and drop_table are implemented at least (and supports_migrations? condition in migration_test.rb was removed at 2158a70).

  2. to determine to allow migration (rake db:migrate) to the adapter. I think that disallowing migration is unneeded because not only schema statements can be used in migration.

Deprecate `supports_migrations?` on connection adapters
`supports_migrations?` was added at 4160b51 to determine if schema
statements (`create_table`, `drop_table`, etc) are implemented in the
adapter. But all tested databases has been supported migrations since
a4fc93c at least.
@pixeltrix

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Feb 26, 2017

Member

I think that disallowing migration is unneeded because not only schema statements can be used in migration.

Definitely - you could write all your migrations use execute

Member

pixeltrix commented Feb 26, 2017

I think that disallowing migration is unneeded because not only schema statements can be used in migration.

Definitely - you could write all your migrations use execute

@pixeltrix pixeltrix merged commit 7888f4f into rails:master Feb 26, 2017

2 checks passed

codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pixeltrix

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Feb 26, 2017

Member

Thanks for the additional research @kamipo 👍

Member

pixeltrix commented Feb 26, 2017

Thanks for the additional research @kamipo 👍

@kamipo kamipo deleted the kamipo:deprecate_supports_migrations branch Feb 26, 2017

@maclover7 maclover7 removed the needs work label Feb 26, 2017

koic added a commit to koic/oracle-enhanced that referenced this pull request Feb 27, 2017

@koic koic referenced this pull request in rsim/oracle-enhanced Feb 27, 2017

Merged

Deprecate `supports_migrations?` on connection adapters #1209

Edouard-chin added a commit to Edouard-chin/activerecord-session_store that referenced this pull request Dec 1, 2017

Remove the `supports_migrations?` check:
- This method is deprecated as of Rails 5.1. All tested databases seems to supports migrations since at least rails/rails@a4fc93c
- Ref rails/rails#28172

@Edouard-chin Edouard-chin referenced this pull request in rails/activerecord-session_store Dec 1, 2017

Open

Remove the `supports_migrations?` check: #119

bgeuken added a commit to bgeuken/open-build-service that referenced this pull request May 23, 2018

[frontend] Rails 5.2: Update database rake task
The supports_migrations? method got deprecated in Rails 5.1 and now got
removed.
Since OBS only supports the MySQL adapter and we know that this one
supports migration dumps, we don't need this check anyway.

See rails/rails#28172 for details.

bgeuken added a commit to bgeuken/open-build-service that referenced this pull request May 23, 2018

[frontend] Rails 5.2: Update database rake task
The supports_migrations? method got deprecated in Rails 5.1 and now got
removed.
Since OBS only supports the MySQL adapter and we know that this one
supports migration dumps, we don't need this check anyway.

See rails/rails#28172 for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment