diff options
11 files changed, 79 insertions, 69 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index f3f1618473..6add697eeb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -53,9 +53,8 @@ module ActiveRecord statements.concat(o.foreign_keys.map { |to_table, options| foreign_key_in_create(o.name, to_table, options) }) end - create_sql << "(#{statements.join(', ')}) " if statements.present? + create_sql << "(#{statements.join(', ')})" if statements.present? add_table_options!(create_sql, table_options(o)) - create_sql << "#{o.options}" create_sql << " AS #{@conn.to_sql(o.as)}" if o.as create_sql end @@ -84,13 +83,16 @@ module ActiveRecord end def table_options(o) - options = {} - options[:comment] = o.comment - options + table_options = {} + table_options[:comment] = o.comment + table_options[:options] = o.options + table_options end - def add_table_options!(sql, _options) - sql + def add_table_options!(create_sql, options) + if options_sql = options[:options] + create_sql << " #{options_sql}" + end end def column_options(o) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 1603c38e35..9860f6e189 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -209,7 +209,7 @@ module ActiveRecord attr_accessor :indexes attr_reader :name, :temporary, :options, :as, :foreign_keys, :comment - def initialize(name, temporary, options, as = nil, comment = nil) + def initialize(name, temporary = false, options = nil, as = nil, comment: nil) @columns_hash = {} @indexes = {} @foreign_keys = [] 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 ca2539f845..4f361fed6b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -18,7 +18,7 @@ module ActiveRecord nil end - # Returns comment associated with given table in database + # Returns the table comment that's stored in database metadata. def table_comment(table_name) nil end @@ -259,8 +259,8 @@ module ActiveRecord # SELECT * FROM orders INNER JOIN line_items ON order_id=orders.id # # See also TableDefinition#column for details on how to create columns. - def create_table(table_name, options = {}) - td = create_table_definition table_name, options[:temporary], options[:options], options[:as], options[:comment] + def create_table(table_name, comment: nil, **options) + td = create_table_definition table_name, options[:temporary], options[:options], options[:as], comment: comment if options[:id] != false && !options[:as] pk = options.fetch(:primary_key) do @@ -270,7 +270,7 @@ module ActiveRecord if pk.is_a?(Array) td.primary_keys pk else - td.primary_key pk, options.fetch(:id, :primary_key), options.except(:comment) + td.primary_key pk, options.fetch(:id, :primary_key), options end end @@ -289,7 +289,8 @@ module ActiveRecord end if supports_comments? && !supports_comments_in_create? - change_table_comment(table_name, options[:comment]) if options[:comment] + change_table_comment(table_name, comment) if comment + td.columns.each do |column| change_column_comment(table_name, column.name, column.comment) if column.comment end @@ -1096,10 +1097,10 @@ module ActiveRecord Table.new(table_name, base) end - def add_index_options(table_name, column_name, options = {}) #:nodoc: + def add_index_options(table_name, column_name, comment: nil, **options) #:nodoc: column_names = Array(column_name) - options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type, :comment) + options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type) index_type = options[:type].to_s if options.key?(:type) index_type ||= options[:unique] ? "UNIQUE" : "" @@ -1127,8 +1128,6 @@ module ActiveRecord end index_columns = quoted_columns_for_index(column_names, options).join(", ") - comment = options[:comment] if options.key?(:comment) - [index_name, index_type, index_columns, index_options, algorithm, using, comment] end @@ -1136,14 +1135,14 @@ module ActiveRecord options.include?(:default) && !(options[:null] == false && options[:default].nil?) end - # Adds comment for given table or drops it if +nil+ given + # Changes the comment for a table or removes it if +nil+. def change_table_comment(table_name, comment) - raise NotImplementedError, "change_table_comment is not implemented" + raise NotImplementedError, "#{self.class} does not support changing table comments" end - # Adds comment for given table column or drops it if +nil+ given + # Changes the comment for a column or removes it if +nil+. def change_column_comment(table_name, column_name, comment) #:nodoc: - raise NotImplementedError, "change_column_comment is not implemented" + raise NotImplementedError, "#{self.class} does not support changing column comments" end protected @@ -1227,8 +1226,8 @@ module ActiveRecord end private - def create_table_definition(name, temporary = false, options = nil, as = nil, comment = nil) - TableDefinition.new(name, temporary, options, as, comment) + def create_table_definition(*args) + TableDefinition.new(*args) end def create_alter_table(name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index b753c348de..c9716c9ca9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -278,7 +278,7 @@ module ActiveRecord false end - # Does adapter supports comments on database objects (tables, columns, indexes)? + # Does this adapter support metadata comments on database objects (tables, columns, indexes)? def supports_comments? false end 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 cea5a04006..6fc286f2be 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -160,8 +160,8 @@ module ActiveRecord raise NotImplementedError end - def new_column(field, default, sql_type_metadata, null, table_name, default_function = nil, collation = nil, comment = nil) # :nodoc: - MySQL::Column.new(field, default, sql_type_metadata, null, table_name, default_function, collation, comment) + def new_column(*args) #:nodoc: + MySQL::Column.new(*args) end # Must return the MySQL error number from the exception, if the exception has an @@ -394,20 +394,20 @@ module ActiveRecord else default, default_function = field[:Default], nil end - new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], field[:Comment].presence) + new_column(field[:Field], default, type_metadata, field[:Null] == "YES", table_name, default_function, field[:Collation], comment: field[:Comment].presence) end end - def table_comment(table_name) + def table_comment(table_name) # :nodoc: select_value(<<-SQL.strip_heredoc, 'SCHEMA') SELECT table_comment - FROM INFORMATION_SCHEMA.TABLES - WHERE table_name=#{quote(table_name)}; + FROM information_schema.tables + WHERE table_name=#{quote(table_name)} SQL end - def create_table(table_name, options = {}) #:nodoc: - super(table_name, options.reverse_merge(:options => "ENGINE=InnoDB")) + def create_table(table_name, **options) #:nodoc: + super(table_name, options: 'ENGINE=InnoDB', **options) end def bulk_change_table(table_name, operations) #:nodoc: @@ -881,8 +881,8 @@ module ActiveRecord create_table_info_cache[table_name] ||= select_one("SHOW CREATE TABLE #{quote_table_name(table_name)}")["Create Table"] end - def create_table_definition(name, temporary = false, options = nil, as = nil, comment = nil) # :nodoc: - MySQL::TableDefinition.new(name, temporary, options, as, comment) + def create_table_definition(*args) # :nodoc: + MySQL::TableDefinition.new(*args) end def integer_to_sql(limit) # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 1c798d99f2..ce2f9f1925 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -15,7 +15,7 @@ module ActiveRecord # +default+ is the type-casted default value, such as +new+ in <tt>sales_stage varchar(20) default 'new'</tt>. # +sql_type_metadata+ is various information about the type of the column # +null+ determines if this column allows +NULL+ values. - def initialize(name, default, sql_type_metadata = nil, null = true, table_name = nil, default_function = nil, collation = nil, comment = nil) + def initialize(name, default, sql_type_metadata = nil, null = true, table_name = nil, default_function = nil, collation = nil, comment: nil) @name = name.freeze @table_name = table_name @sql_type_metadata = sql_type_metadata diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb index 5ab81640e8..0384079da2 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb @@ -25,10 +25,11 @@ module ActiveRecord add_column_position!(change_column_sql, column_options(o.column)) end - def add_table_options!(sql, options) + def add_table_options!(create_sql, options) super - if options[:comment] - sql << "COMMENT #{quote(options[:comment])} " + + if comment = options[:comment] + create_sql << " COMMENT #{quote(comment)}" end end @@ -39,17 +40,21 @@ module ActiveRecord end def add_column_options!(sql, options) - if options[:charset] - sql << " CHARACTER SET #{options[:charset]}" + if charset = options[:charset] + sql << " CHARACTER SET #{charset}" end - if options[:collation] - sql << " COLLATE #{options[:collation]}" + + if collation = options[:collation] + sql << " COLLATE #{collation}" end - new_sql = super - if options[:comment] - new_sql << " COMMENT #{quote(options[:comment])}" + + super + + if comment = options[:comment] + sql << " COMMENT #{quote(comment)}" end - new_sql + + sql end def add_column_position!(sql, options) @@ -58,6 +63,7 @@ module ActiveRecord elsif options[:after] sql << " AFTER #{quote_column_name(options[:after])}" end + sql end 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 36331204f7..4a66b82cbb 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -225,27 +225,27 @@ module ActiveRecord type_metadata = fetch_type_metadata(column_name, type, oid, fmod) default_value = extract_value_from_default(default) default_function = extract_default_function(default_value, default) - new_column(column_name, default_value, type_metadata, !notnull, table_name, default_function, collation, comment) + new_column(column_name, default_value, type_metadata, !notnull, table_name, default_function, collation, comment: comment) end end - def new_column(name, default, sql_type_metadata, null, table_name, default_function = nil, collation = nil, comment = nil) # :nodoc: - PostgreSQLColumn.new(name, default, sql_type_metadata, null, table_name, default_function, collation, comment) + def new_column(*args) # :nodoc: + PostgreSQLColumn.new(*args) end # Returns a comment stored in database for given table def table_comment(table_name) # :nodoc: name = Utils.extract_schema_qualified_name(table_name.to_s) - return nil unless name.identifier - - select_value(<<-SQL.strip_heredoc, 'SCHEMA') - SELECT pg_catalog.obj_description(c.oid, 'pg_class') - FROM pg_catalog.pg_class c - LEFT JOIN pg_namespace n ON n.oid = c.relnamespace - WHERE c.relname = '#{name.identifier}' - AND c.relkind IN ('r') -- (r)elation/table - AND n.nspname = #{name.schema ? "'#{name.schema}'" : 'ANY (current_schemas(false))'} - SQL + if name.identifier + select_value(<<-SQL.strip_heredoc, 'SCHEMA') + SELECT pg_catalog.obj_description(c.oid, 'pg_class') + FROM pg_catalog.pg_class c + LEFT JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relname = #{quote(name.identifier)} + AND c.relkind IN ('r') -- (r)elation/table + AND n.nspname = #{name.schema ? quote(name.schema) : 'ANY (current_schemas(false))'} + SQL + end end # Returns the current database name. @@ -536,9 +536,9 @@ module ActiveRecord def add_index(table_name, column_name, options = {}) #:nodoc: index_name, index_type, index_columns, index_options, index_algorithm, index_using, comment = add_index_options(table_name, column_name, options) - result = execute "CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns})#{index_options}" - execute "COMMENT ON INDEX #{quote_column_name(index_name)} IS #{quote(comment)}" if comment - result # Result of execute is used in tests in activerecord/test/cases/adapters/postgresql/active_schema_test.rb + execute("CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns})#{index_options}").tap do + execute "COMMENT ON INDEX #{quote_column_name(index_name)} IS #{quote(comment)}" if comment + end end def remove_index(table_name, options = {}) #:nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index cf04f32206..061a9f66d4 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -163,6 +163,10 @@ module ActiveRecord true end + def supports_comments_in_create? + false + end + def index_algorithms { concurrently: 'CONCURRENTLY' } end @@ -751,8 +755,8 @@ module ActiveRecord $1.strip if $1 end - def create_table_definition(name, temporary = false, options = nil, as = nil, comment = nil) # :nodoc: - PostgreSQL::TableDefinition.new(name, temporary, options, as, comment) + def create_table_definition(*args) # :nodoc: + PostgreSQL::TableDefinition.new(*args) end def can_perform_case_insensitive_comparison_for?(column) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index bc4adb4ed7..b4229cba04 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -138,8 +138,9 @@ HEADER table_options = @connection.table_options(table) tbl.print ", options: #{table_options.inspect}" unless table_options.blank? - comment = @connection.table_comment(table) - tbl.print ", comment: #{comment.inspect}" if comment + if comment = @connection.table_comment(table) + tbl.print ", comment: #{comment.inspect}" + end tbl.puts " do |t|" diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 53a490cf21..cb6f07c925 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -26,8 +26,7 @@ class CommentTest < ActiveRecord::TestCase @connection.drop_table 'commenteds', if_exists: true end - if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) - + if ActiveRecord::Base.connection.supports_comments? def test_column_created_in_block Commented.reset_column_information column = Commented.columns_hash['name'] @@ -86,6 +85,5 @@ class CommentTest < ActiveRecord::TestCase assert_match %r[add_index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output assert_match %r[add_index\s+.+\s+name: "idx_obvious",.+\s+comment: "We need to see obvious comments"], output end - end end |