From f2099361da05dd6890c733e5aecdce4cd52efdff Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 20 Aug 2017 23:31:34 +0900 Subject: [PATCH 1/2] Remove deprecated `#migration_keys` --- activerecord/CHANGELOG.md | 4 ++++ .../connection_adapters/abstract/schema_dumper.rb | 6 ------ .../connection_adapters/mysql/schema_dumper.rb | 4 ---- .../connection_adapters/postgresql/schema_dumper.rb | 5 ----- activerecord/test/cases/migration_test.rb | 4 ---- 5 files changed, 4 insertions(+), 19 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4a840a57b5..1a1a3a8092 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Remove deprecated `#migration_keys`. + + *Ryuta Kamizono* + * Automatically guess the inverse associations for STI. *Yuichiro Kaneko* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index f17f1d35e2..cd58365b3d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -22,12 +22,6 @@ module ActiveRecord spec end - # Lists the valid migration options - def migration_keys # :nodoc: - column_options_keys - end - deprecate :migration_keys - private def prepare_column_options(column) spec = {} diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index 81f7dce562..d5198a6dba 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -4,10 +4,6 @@ module ActiveRecord module ConnectionAdapters module MySQL module ColumnDumper # :nodoc: - def migration_keys - super + [:unsigned] - end - private def prepare_column_options(column) spec = super diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb index 5031605c2a..14394bbaf5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb @@ -4,11 +4,6 @@ module ActiveRecord module ConnectionAdapters module PostgreSQL module ColumnDumper # :nodoc: - # Adds +:array+ as a valid migration key - def migration_keys - super + [:array] - end - private def prepare_column_options(column) spec = super diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 0fa43583ac..07afa89779 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -1112,10 +1112,6 @@ class CopyMigrationsTest < ActiveRecord::TestCase assert_deprecated { ActiveRecord::Base.connection.initialize_internal_metadata_table } end - def test_deprecate_migration_keys - assert_deprecated { ActiveRecord::Base.connection.migration_keys } - end - def test_deprecate_supports_migrations assert_deprecated { ActiveRecord::Base.connection.supports_migrations? } end From 815dd11bc5b6988b55573530644de93aefefa1b4 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 20 Aug 2017 23:32:41 +0900 Subject: [PATCH 2/2] Refactor `SchemaDumper` to make it possible to adapter specific customization Currently `SchemaDumper` is only customizable for column options. But 3rd party connection adapters (oracle-enhanced etc) need to customizable for table or index dumping also. To make it possible, I introduced adapter specific `SchemaDumper` classes for that. --- .../abstract/schema_dumper.rb | 37 +++++++++---------- .../abstract/schema_statements.rb | 4 ++ .../connection_adapters/abstract_adapter.rb | 1 - .../abstract_mysql_adapter.rb | 1 - .../mysql/schema_dumper.rb | 21 ++++++----- .../mysql/schema_statements.rb | 4 ++ .../postgresql/schema_dumper.rb | 2 +- .../postgresql/schema_statements.rb | 4 ++ .../connection_adapters/postgresql_adapter.rb | 1 - .../sqlite3/schema_dumper.rb | 3 +- .../sqlite3/schema_statements.rb | 4 ++ .../connection_adapters/sqlite3_adapter.rb | 1 - .../lib/active_record/schema_dumper.rb | 6 +-- 13 files changed, 51 insertions(+), 38 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb index cd58365b3d..1926603474 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb @@ -4,25 +4,24 @@ require "active_support/core_ext/hash/compact" module ActiveRecord module ConnectionAdapters # :nodoc: - # The goal of this module is to move Adapter specific column - # definitions to the Adapter instead of having it in the schema - # dumper itself. This code represents the normal case. - # We can then redefine how certain data types may be handled in the schema dumper on the - # Adapter level by over-writing this code inside the database specific adapters - module ColumnDumper - def column_spec(column) - [schema_type_with_virtual(column), prepare_column_options(column)] - end - - def column_spec_for_primary_key(column) - return {} if default_primary_key?(column) - spec = { id: schema_type(column).inspect } - spec.merge!(prepare_column_options(column).except!(:null)) - spec[:default] ||= "nil" if explicit_primary_key_default?(column) - spec + class SchemaDumper < SchemaDumper # :nodoc: + def self.create(connection, options) + new(connection, options) end private + def column_spec(column) + [schema_type_with_virtual(column), prepare_column_options(column)] + end + + def column_spec_for_primary_key(column) + return {} if default_primary_key?(column) + spec = { id: schema_type(column).inspect } + spec.merge!(prepare_column_options(column).except!(:null)) + spec[:default] ||= "nil" if explicit_primary_key_default?(column) + spec + end + def prepare_column_options(column) spec = {} spec[:limit] = schema_limit(column) @@ -45,7 +44,7 @@ module ActiveRecord end def schema_type_with_virtual(column) - if supports_virtual_columns? && column.virtual? + if @connection.supports_virtual_columns? && column.virtual? :virtual else schema_type(column) @@ -62,7 +61,7 @@ module ActiveRecord def schema_limit(column) limit = column.limit unless column.bigint? - limit.inspect if limit && limit != native_database_types[column.type][:limit] + limit.inspect if limit && limit != @connection.native_database_types[column.type][:limit] end def schema_precision(column) @@ -75,7 +74,7 @@ module ActiveRecord def schema_default(column) return unless column.has_default? - type = lookup_cast_type_from_column(column) + type = @connection.lookup_cast_type_from_column(column) default = type.deserialize(column.default) if default.nil? schema_expression(column) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 3c9b25e411..f36c1f111a 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1174,6 +1174,10 @@ module ActiveRecord raise NotImplementedError, "#{self.class} does not support changing column comments" end + def create_schema_dumper(options) # :nodoc: + SchemaDumper.create(self, options) + end + private def column_options_keys [:limit, :precision, :scale, :default, :null, :collation, :comment] diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 7645cf7825..47881e3305 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -72,7 +72,6 @@ module ActiveRecord include Quoting, DatabaseStatements, SchemaStatements include DatabaseLimits include QueryCache - include ColumnDumper include Savepoints SIMPLE_INT = /\A\d+\z/ diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index e647389514..9f5efda97c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -17,7 +17,6 @@ module ActiveRecord module ConnectionAdapters class AbstractMysqlAdapter < AbstractAdapter include MySQL::Quoting - include MySQL::ColumnDumper include MySQL::SchemaStatements ## diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb index d5198a6dba..95eb77aea4 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb @@ -3,13 +3,13 @@ module ActiveRecord module ConnectionAdapters module MySQL - module ColumnDumper # :nodoc: + class SchemaDumper < ConnectionAdapters::SchemaDumper # :nodoc: private def prepare_column_options(column) spec = super spec[:unsigned] = "true" if column.unsigned? - if supports_virtual_columns? && column.virtual? + if @connection.supports_virtual_columns? && column.virtual? spec[:as] = extract_expression_for_virtual_column(column) spec[:stored] = "true" if /\b(?:STORED|PERSISTENT)\b/.match?(column.extra) spec = { type: schema_type(column).inspect }.merge!(spec) @@ -44,24 +44,27 @@ module ActiveRecord def schema_collation(column) if column.collation && table_name = column.table_name @table_collation_cache ||= {} - @table_collation_cache[table_name] ||= exec_query("SHOW TABLE STATUS LIKE #{quote(table_name)}", "SCHEMA").first["Collation"] + @table_collation_cache[table_name] ||= + @connection.exec_query("SHOW TABLE STATUS LIKE #{@connection.quote(table_name)}", "SCHEMA").first["Collation"] column.collation.inspect if column.collation != @table_collation_cache[table_name] end end def extract_expression_for_virtual_column(column) - if mariadb? && version < "10.2.5" - create_table_info = create_table_info(column.table_name) - if %r/#{quote_column_name(column.name)} #{Regexp.quote(column.sql_type)}(?: COLLATE \w+)? AS \((?.+?)\) #{column.extra}/ =~ create_table_info + if @connection.mariadb? && @connection.version < "10.2.5" + create_table_info = @connection.send(:create_table_info, column.table_name) + column_name = @connection.quote_column_name(column.name) + if %r/#{column_name} #{Regexp.quote(column.sql_type)}(?: COLLATE \w+)? AS \((?.+?)\) #{column.extra}/ =~ create_table_info $~[:expression].inspect end else - scope = quoted_scope(column.table_name) + scope = @connection.send(:quoted_scope, column.table_name) + column_name = @connection.quote(column.name) sql = "SELECT generation_expression FROM information_schema.columns" \ " WHERE table_schema = #{scope[:schema]}" \ " AND table_name = #{scope[:name]}" \ - " AND column_name = #{quote(column.name)}" - query_value(sql, "SCHEMA").inspect + " AND column_name = #{column_name}" + @connection.query_value(sql, "SCHEMA").inspect end end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 7bebe82065..759493e3bd 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -66,6 +66,10 @@ module ActiveRecord MySQL::Table.new(table_name, base) end + def create_schema_dumper(options) + MySQL::SchemaDumper.create(self, options) + end + private CHARSETS_OF_4BYTES_MAXLEN = ["utf8mb4", "utf16", "utf16le", "utf32"] diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb index 14394bbaf5..c0dbb166b7 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb @@ -3,7 +3,7 @@ module ActiveRecord module ConnectionAdapters module PostgreSQL - module ColumnDumper # :nodoc: + class SchemaDumper < ConnectionAdapters::SchemaDumper # :nodoc: private def prepare_column_options(column) spec = super diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index f258203a43..9e2f61e6ce 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -590,6 +590,10 @@ module ActiveRecord PostgreSQL::Table.new(table_name, base) end + def create_schema_dumper(options) # :nodoc: + PostgreSQL::SchemaDumper.create(self, options) + end + private def schema_creation PostgreSQL::SchemaCreation.new(self) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 9d92f7ca70..4d37a292d6 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -121,7 +121,6 @@ module ActiveRecord include PostgreSQL::ReferentialIntegrity include PostgreSQL::SchemaStatements include PostgreSQL::DatabaseStatements - include PostgreSQL::ColumnDumper def supports_index_sort_order? true diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_dumper.rb index ab057c73f1..621678ec65 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_dumper.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_dumper.rb @@ -3,9 +3,8 @@ module ActiveRecord module ConnectionAdapters module SQLite3 - module ColumnDumper # :nodoc: + class SchemaDumper < ConnectionAdapters::SchemaDumper # :nodoc: private - def default_primary_key?(column) schema_type(column) == :integer end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index ee84646214..a512702b7b 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -43,6 +43,10 @@ module ActiveRecord SQLite3::Table.new(table_name, base) end + def create_schema_dumper(options) + SQLite3::SchemaDumper.create(self, options) + end + private def schema_creation SQLite3::SchemaCreation.new(self) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 107b7a9e1d..d6f357bcc3 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -57,7 +57,6 @@ module ActiveRecord ADAPTER_NAME = "SQLite".freeze include SQLite3::Quoting - include SQLite3::ColumnDumper include SQLite3::SchemaStatements NATIVE_DATABASE_TYPES = { diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index fd644225ca..7b2894086e 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -19,7 +19,7 @@ module ActiveRecord class << self def dump(connection = ActiveRecord::Base.connection, stream = STDOUT, config = ActiveRecord::Base) - new(connection, generate_options(config)).dump(stream) + connection.create_schema_dumper(generate_options(config)).dump(stream) stream end @@ -123,7 +123,7 @@ HEADER when String tbl.print ", primary_key: #{pk.inspect}" unless pk == "id" pkcol = columns.detect { |c| c.name == pk } - pkcolspec = @connection.column_spec_for_primary_key(pkcol) + pkcolspec = column_spec_for_primary_key(pkcol) if pkcolspec.present? tbl.print ", #{format_colspec(pkcolspec)}" end @@ -145,7 +145,7 @@ HEADER columns.each do |column| raise StandardError, "Unknown type '#{column.sql_type}' for column '#{column.name}'" unless @connection.valid_type?(column.type) next if column.name == pk - type, colspec = @connection.column_spec(column) + type, colspec = column_spec(column) tbl.print " t.#{type} #{column.name.inspect}" tbl.print ", #{format_colspec(colspec)}" if colspec.present? tbl.puts