From 485e7f25f29ca1ca23bb214b802cf68840dabbb6 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremydaer@gmail.com>
Date: Sun, 17 Apr 2016 13:35:16 -0700
Subject: Database comments: switch to keyword args for new table options
* Switch to keyword args where we can without breaking compat.
* Use add_table_options! for :options, too.
* Some code polish.
---
.../abstract/schema_creation.rb | 16 ++++++-----
.../abstract/schema_definitions.rb | 2 +-
.../abstract/schema_statements.rb | 29 ++++++++++----------
.../connection_adapters/abstract_adapter.rb | 2 +-
.../connection_adapters/abstract_mysql_adapter.rb | 20 +++++++-------
.../active_record/connection_adapters/column.rb | 2 +-
.../connection_adapters/mysql/schema_creation.rb | 28 +++++++++++--------
.../postgresql/schema_statements.rb | 32 +++++++++++-----------
.../connection_adapters/postgresql_adapter.rb | 8 ++++--
activerecord/lib/active_record/schema_dumper.rb | 5 ++--
activerecord/test/cases/comment_test.rb | 4 +--
11 files changed, 79 insertions(+), 69 deletions(-)
(limited to 'activerecord')
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
--
cgit v1.2.3