diff options
author | Jeffrey Guenther <guenther.jeffrey@gmail.com> | 2017-12-08 13:25:54 -0800 |
---|---|---|
committer | Jeffrey Guenther <guenther.jeffrey@gmail.com> | 2017-12-08 13:25:54 -0800 |
commit | a822287cefc38b9b8b3be38ffd775cd3d511b7c3 (patch) | |
tree | d69c6ea1fcc4299caa11bcbef2ce5520347a4f46 /activerecord | |
parent | 08fab27db52aa375df85a23e89799600f785b9d4 (diff) | |
parent | da8e0ba03cbae33857954c0c1a228bd6dae562da (diff) | |
download | rails-a822287cefc38b9b8b3be38ffd775cd3d511b7c3.tar.gz rails-a822287cefc38b9b8b3be38ffd775cd3d511b7c3.tar.bz2 rails-a822287cefc38b9b8b3be38ffd775cd3d511b7c3.zip |
Merge branch 'master' into activestorage-guide
Diffstat (limited to 'activerecord')
35 files changed, 647 insertions, 186 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 60ceffac5e..89a12d4223 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Add support for PostgreSQL operator classes to `add_index`. + + Example: + + add_index :users, :name, using: :gist, opclass: { name: :gist_trgm_ops } + + *Greg Navis* + * Don't allow scopes to be defined which conflict with instance methods on `Relation`. Fixes #31120. @@ -72,7 +80,7 @@ *bogdanvlviv* * Fixed a bug where column orders for an index weren't written to - db/schema.rb when using the sqlite adapter. + `db/schema.rb` when using the sqlite adapter. Fixes #30902. 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 be2f625d74..0594b4b485 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -6,7 +6,7 @@ module ActiveRecord # this type are typically created and returned by methods in database # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes class IndexDefinition # :nodoc: - attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment + attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :comment def initialize( table, name, @@ -14,6 +14,7 @@ module ActiveRecord columns = [], lengths: {}, orders: {}, + opclasses: {}, where: nil, type: nil, using: nil, @@ -23,13 +24,23 @@ module ActiveRecord @name = name @unique = unique @columns = columns - @lengths = lengths - @orders = orders + @lengths = concise_options(lengths) + @orders = concise_options(orders) + @opclasses = concise_options(opclasses) @where = where @type = type @using = using @comment = comment end + + private + def concise_options(options) + if columns.size == options.size && options.values.uniq.size == 1 + options.values.first + else + options + end + end end # Abstract representation of a column definition. Instances of this type @@ -85,6 +96,11 @@ module ActiveRecord options[:primary_key] != default_primary_key end + def validate? + options.fetch(:validate, true) + end + alias validated? validate? + def defined_for?(to_table_ord = nil, to_table: nil, **options) if to_table_ord self.to_table == to_table_ord.to_s @@ -204,6 +220,7 @@ module ActiveRecord :decimal, :float, :integer, + :json, :string, :text, :time, 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 9b7345f7c3..4f58b0242c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -600,7 +600,7 @@ module ActiveRecord # to provide these in a migration's +change+ method so it can be reverted. # In that case, +type+ and +options+ will be used by #add_column. def remove_column(table_name, column_name, type = nil, options = {}) - execute "ALTER TABLE #{quote_table_name(table_name)} DROP #{quote_column_name(column_name)}" + execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, options)}" end # Changes the column's definition according to the new options. @@ -738,6 +738,28 @@ module ActiveRecord # # Note: only supported by PostgreSQL and MySQL # + # ====== Creating an index with a specific operator class + # + # add_index(:developers, :name, using: 'gist', opclass: :gist_trgm_ops) + # + # generates: + # + # CREATE INDEX developers_on_name ON developers USING gist (name gist_trgm_ops) -- PostgreSQL + # + # add_index(:developers, [:name, :city], using: 'gist', opclass: { city: :gist_trgm_ops }) + # + # generates: + # + # CREATE INDEX developers_on_name_and_city ON developers USING gist (name, city gist_trgm_ops) -- PostgreSQL + # + # add_index(:developers, [:name, :city], using: 'gist', opclass: :gist_trgm_ops) + # + # generates: + # + # CREATE INDEX developers_on_name_and_city ON developers USING gist (name gist_trgm_ops, city gist_trgm_ops) -- PostgreSQL + # + # Note: only supported by PostgreSQL + # # ====== Creating an index with a specific type # # add_index(:developers, :name, type: :fulltext) @@ -942,6 +964,8 @@ module ActiveRecord # Action that happens <tt>ON DELETE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+ # [<tt>:on_update</tt>] # Action that happens <tt>ON UPDATE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+ + # [<tt>:validate</tt>] + # (Postgres only) Specify whether or not the constraint should be validated. Defaults to +true+. def add_foreign_key(from_table, to_table, options = {}) return unless supports_foreign_keys? @@ -1120,7 +1144,7 @@ module ActiveRecord def add_index_options(table_name, column_name, comment: nil, **options) # :nodoc: column_names = index_column_names(column_name) - options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type) + options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type, :opclass) index_type = options[:type].to_s if options.key?(:type) index_type ||= options[:unique] ? "UNIQUE" : "" @@ -1173,20 +1197,22 @@ module ActiveRecord end def add_index_sort_order(quoted_columns, **options) - if order = options[:order] - case order - when Hash - order = order.symbolize_keys - quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? } - when String - quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? } - end + orders = options_for_index_columns(options[:order]) + quoted_columns.each do |name, column| + column << " #{orders[name].upcase}" if orders[name].present? end + end - quoted_columns + def options_for_index_columns(options) + if options.is_a?(Hash) + options.symbolize_keys + else + Hash.new { |hash, column| hash[column] = options } + end end - # Overridden by the MySQL adapter for supporting index lengths + # Overridden by the MySQL adapter for supporting index lengths and by + # the PostgreSQL adapter for supporting operator classes. def add_options_for_index_columns(quoted_columns, **options) if supports_index_sort_order? quoted_columns = add_index_sort_order(quoted_columns, options) @@ -1340,6 +1366,20 @@ module ActiveRecord options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty? end + def add_column_for_alter(table_name, column_name, type, options = {}) + td = create_table_definition(table_name) + cd = td.new_column_definition(column_name, type, options) + schema_creation.accept(AddColumnDefinition.new(cd)) + end + + def remove_column_for_alter(table_name, column_name, type = nil, options = {}) + "DROP COLUMN #{quote_column_name(column_name)}" + end + + def remove_columns_for_alter(table_name, *column_names) + column_names.map { |column_name| remove_column_for_alter(table_name, column_name) } + end + def insert_versions_sql(versions) sm_table = quote_table_name(ActiveRecord::SchemaMigration.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 8993c517a6..fc80d332f9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -272,6 +272,11 @@ module ActiveRecord false end + # Does this adapter support creating invalid constraints? + def supports_validate_constraints? + false + end + # Does this adapter support creating foreign key constraints # in the same statement as creating the table? def supports_foreign_keys_in_create? 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 ede8a9c1e2..479131caad 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -299,7 +299,7 @@ module ActiveRecord def bulk_change_table(table_name, operations) #:nodoc: sqls = operations.flat_map do |command, args| table, arguments = args.shift, args - method = :"#{command}_sql" + method = :"#{command}_for_alter" if respond_to?(method, true) send(method, table, *arguments) @@ -372,11 +372,11 @@ module ActiveRecord end def change_column(table_name, column_name, type, options = {}) #:nodoc: - execute("ALTER TABLE #{quote_table_name(table_name)} #{change_column_sql(table_name, column_name, type, options)}") + execute("ALTER TABLE #{quote_table_name(table_name)} #{change_column_for_alter(table_name, column_name, type, options)}") end def rename_column(table_name, column_name, new_column_name) #:nodoc: - execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_sql(table_name, column_name, new_column_name)}") + execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_for_alter(table_name, column_name, new_column_name)}") rename_column_indexes(table_name, column_name, new_column_name) end @@ -605,25 +605,6 @@ module ActiveRecord end end - def add_index_length(quoted_columns, **options) - if length = options[:length] - case length - when Hash - length = length.symbolize_keys - quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? } - when Integer - quoted_columns.each { |name, column| column << "(#{length})" } - end - end - - quoted_columns - end - - def add_options_for_index_columns(quoted_columns, **options) - quoted_columns = add_index_length(quoted_columns, options) - super - end - # See https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html ER_DUP_ENTRY = 1062 ER_NOT_NULL_VIOLATION = 1048 @@ -671,13 +652,7 @@ module ActiveRecord end end - def add_column_sql(table_name, column_name, type, options = {}) - td = create_table_definition(table_name) - cd = td.new_column_definition(column_name, type, options) - schema_creation.accept(AddColumnDefinition.new(cd)) - end - - def change_column_sql(table_name, column_name, type, options = {}) + def change_column_for_alter(table_name, column_name, type, options = {}) column = column_for(table_name, column_name) type ||= column.sql_type @@ -698,7 +673,7 @@ module ActiveRecord schema_creation.accept(ChangeColumnDefinition.new(cd, column.name)) end - def rename_column_sql(table_name, column_name, new_column_name) + def rename_column_for_alter(table_name, column_name, new_column_name) column = column_for(table_name, column_name) options = { default: column.default, @@ -712,31 +687,23 @@ module ActiveRecord schema_creation.accept(ChangeColumnDefinition.new(cd, column.name)) end - def remove_column_sql(table_name, column_name, type = nil, options = {}) - "DROP #{quote_column_name(column_name)}" - end - - def remove_columns_sql(table_name, *column_names) - column_names.map { |column_name| remove_column_sql(table_name, column_name) } - end - - def add_index_sql(table_name, column_name, options = {}) + def add_index_for_alter(table_name, column_name, options = {}) index_name, index_type, index_columns, _, index_algorithm, index_using = add_index_options(table_name, column_name, options) index_algorithm[0, 0] = ", " if index_algorithm.present? "ADD #{index_type} INDEX #{quote_column_name(index_name)} #{index_using} (#{index_columns})#{index_algorithm}" end - def remove_index_sql(table_name, options = {}) + def remove_index_for_alter(table_name, options = {}) index_name = index_name_for_remove(table_name, options) "DROP INDEX #{quote_column_name(index_name)}" end - def add_timestamps_sql(table_name, options = {}) - [add_column_sql(table_name, :created_at, :datetime, options), add_column_sql(table_name, :updated_at, :datetime, options)] + def add_timestamps_for_alter(table_name, options = {}) + [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)] end - def remove_timestamps_sql(table_name, options = {}) - [remove_column_sql(table_name, :updated_at), remove_column_sql(table_name, :created_at)] + def remove_timestamps_for_alter(table_name, options = {}) + [remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)] end # MySQL is too stupid to create a temporary table for use subquery, so we have diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index da25e4863c..2ed4ad16ae 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -32,10 +32,6 @@ module ActiveRecord args.each { |name| column(name, :longtext, options) } end - def json(*args, **options) - args.each { |name| column(name, :json, options) } - end - def unsigned_integer(*args, **options) args.each { |name| column(name, :unsigned_integer, options) } 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 a15c7d1787..ce50590651 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -22,23 +22,26 @@ module ActiveRecord index_using = mysql_index_type end - indexes << IndexDefinition.new( + indexes << [ row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, + [], + lengths: {}, + orders: {}, type: index_type, using: index_using, comment: row[:Index_comment].presence - ) + ] end - indexes.last.columns << row[:Column_name] - indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] - indexes.last.orders.merge!(row[:Column_name] => :desc) if row[:Collation] == "D" + indexes.last[-2] << row[:Column_name] + indexes.last[-1][:lengths].merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] + indexes.last[-1][:orders].merge!(row[:Column_name] => :desc) if row[:Collation] == "D" end end - indexes + indexes.map { |index| IndexDefinition.new(*index) } end def remove_column(table_name, column_name, type = nil, options = {}) @@ -103,6 +106,18 @@ module ActiveRecord super unless specifier == "RESTRICT" end + def add_index_length(quoted_columns, **options) + lengths = options_for_index_columns(options[:length]) + quoted_columns.each do |name, column| + column << "(#{lengths[name]})" if lengths[name].present? + end + end + + def add_options_for_index_columns(quoted_columns, **options) + quoted_columns = add_index_length(quoted_columns, options) + super + end + def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb index 59f661da25..8e381a92cf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb @@ -5,6 +5,18 @@ module ActiveRecord module PostgreSQL class SchemaCreation < AbstractAdapter::SchemaCreation # :nodoc: private + def visit_AlterTable(o) + super << o.constraint_validations.map { |fk| visit_ValidateConstraint fk }.join(" ") + end + + def visit_AddForeignKey(o) + super.dup.tap { |sql| sql << " NOT VALID" unless o.validate? } + end + + def visit_ValidateConstraint(name) + "VALIDATE CONSTRAINT #{quote_column_name(name)}" + end + def add_column_options!(sql, options) if options[:collation] sql << " COLLATE \"#{options[:collation]}\"" diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index 75622eb304..6047217fcd 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -95,10 +95,6 @@ module ActiveRecord args.each { |name| column(name, :int8range, options) } end - def json(*args, **options) - args.each { |name| column(name, :json, options) } - end - def jsonb(*args, **options) args.each { |name| column(name, :jsonb, options) } end @@ -192,6 +188,19 @@ module ActiveRecord class Table < ActiveRecord::ConnectionAdapters::Table include ColumnMethods end + + class AlterTable < ActiveRecord::ConnectionAdapters::AlterTable + attr_reader :constraint_validations + + def initialize(td) + super + @constraint_validations = [] + end + + def validate_constraint(name) + @constraint_validations << name + end + end end end 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 846e721983..bf5fbb30e1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -87,10 +87,7 @@ module ActiveRecord result = query(<<-SQL, "SCHEMA") SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid, - pg_catalog.obj_description(i.oid, 'pg_class') AS comment, - (SELECT COUNT(*) FROM pg_opclass o - JOIN (SELECT unnest(string_to_array(d.indclass::text, ' '))::int oid) c - ON o.oid = c.oid WHERE o.opcdefault = 'f') + pg_catalog.obj_description(i.oid, 'pg_class') AS comment FROM pg_class t INNER JOIN pg_index d ON t.oid = d.indrelid INNER JOIN pg_class i ON d.indexrelid = i.oid @@ -109,11 +106,13 @@ module ActiveRecord inddef = row[3] oid = row[4] comment = row[5] - opclass = row[6] using, expressions, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: WHERE (.+))?\z/).flatten - if indkey.include?(0) || opclass > 0 + orders = {} + opclasses = {} + + if indkey.include?(0) columns = expressions else columns = Hash[query(<<-SQL.strip_heredoc, "SCHEMA")].values_at(*indkey).compact @@ -123,10 +122,12 @@ module ActiveRecord AND a.attnum IN (#{indkey.join(",")}) SQL - # add info on sort order for columns (only desc order is explicitly specified, asc is the default) - orders = Hash[ - expressions.scan(/(\w+) DESC/).flatten.map { |order_column| [order_column, :desc] } - ] + # add info on sort order (only desc order is explicitly specified, asc is the default) + # and non-default opclasses + expressions.scan(/(\w+)(?: (?!DESC)(\w+))?(?: (DESC))?/).each do |column, opclass, desc| + opclasses[column] = opclass.to_sym if opclass + orders[column] = :desc if desc + end end IndexDefinition.new( @@ -135,6 +136,7 @@ module ActiveRecord unique, columns, orders: orders, + opclasses: opclasses, where: where, using: using.to_sym, comment: comment.presence @@ -392,50 +394,23 @@ module ActiveRecord def change_column(table_name, column_name, type, options = {}) #:nodoc: clear_cache! - quoted_table_name = quote_table_name(table_name) - quoted_column_name = quote_column_name(column_name) - sql_type = type_to_sql(type, options) - sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}".dup - if options[:collation] - sql << " COLLATE \"#{options[:collation]}\"" - end - if options[:using] - sql << " USING #{options[:using]}" - elsif options[:cast_as] - cast_as_type = type_to_sql(options[:cast_as], options) - sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" - end - execute sql - - change_column_default(table_name, column_name, options[:default]) if options.key?(:default) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + sqls, procs = change_column_for_alter(table_name, column_name, type, options) + execute "ALTER TABLE #{quote_table_name(table_name)} #{sqls.join(", ")}" + procs.each(&:call) end # Changes the default value of a table column. def change_column_default(table_name, column_name, default_or_changes) # :nodoc: - clear_cache! - column = column_for(table_name, column_name) - return unless column - - default = extract_new_default_value(default_or_changes) - alter_column_query = "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} %s" - if default.nil? - # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will - # cast the default to the columns type, which leaves us with a default like "default NULL::character varying". - execute alter_column_query % "DROP DEFAULT" - else - execute alter_column_query % "SET DEFAULT #{quote_default_expression(default, column)}" - end + execute "ALTER TABLE #{quote_table_name(table_name)} #{change_column_default_for_alter(table_name, column_name, default_or_changes)}" end def change_column_null(table_name, column_name, null, default = nil) #:nodoc: clear_cache! unless null || default.nil? column = column_for(table_name, column_name) - execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(default, column)} WHERE #{quote_column_name(column_name)} IS NULL") if column + execute "UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(default, column)} WHERE #{quote_column_name(column_name)} IS NULL" if column end - execute("ALTER TABLE #{quote_table_name(table_name)} ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL") + execute "ALTER TABLE #{quote_table_name(table_name)} #{change_column_null_for_alter(table_name, column_name, null, default)}" end # Adds comment for given table column or drops it if +comment+ is a +nil+ @@ -458,8 +433,8 @@ module ActiveRecord end 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) - 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 + index_name, index_type, index_columns_and_opclasses, index_options, index_algorithm, index_using, comment = add_index_options(table_name, column_name, options) + execute("CREATE #{index_type} INDEX #{index_algorithm} #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} #{index_using} (#{index_columns_and_opclasses})#{index_options}").tap do execute "COMMENT ON INDEX #{quote_column_name(index_name)} IS #{quote(comment)}" if comment end end @@ -499,7 +474,7 @@ module ActiveRecord def foreign_keys(table_name) scope = quoted_scope(table_name) fk_info = exec_query(<<-SQL.strip_heredoc, "SCHEMA") - SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete + SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid FROM pg_constraint c JOIN pg_class t1 ON c.conrelid = t1.oid JOIN pg_class t2 ON c.confrelid = t2.oid @@ -521,6 +496,7 @@ module ActiveRecord options[:on_delete] = extract_foreign_key_action(row["on_delete"]) options[:on_update] = extract_foreign_key_action(row["on_update"]) + options[:validate] = row["valid"] ForeignKeyDefinition.new(table_name, row["to_table"], options) end @@ -581,6 +557,43 @@ module ActiveRecord PostgreSQL::SchemaDumper.create(self, options) end + # Validates the given constraint. + # + # Validates the constraint named +constraint_name+ on +accounts+. + # + # validate_constraint :accounts, :constraint_name + def validate_constraint(table_name, constraint_name) + return unless supports_validate_constraints? + + at = create_alter_table table_name + at.validate_constraint constraint_name + + execute schema_creation.accept(at) + end + + # Validates the given foreign key. + # + # Validates the foreign key on +accounts.branch_id+. + # + # validate_foreign_key :accounts, :branches + # + # Validates the foreign key on +accounts.owner_id+. + # + # validate_foreign_key :accounts, column: :owner_id + # + # Validates the foreign key named +special_fk_name+ on the +accounts+ table. + # + # validate_foreign_key :accounts, name: :special_fk_name + # + # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key. + def validate_foreign_key(from_table, options_or_to_table = {}) + return unless supports_validate_constraints? + + fk_name_to_validate = foreign_key_for!(from_table, options_or_to_table).name + + validate_constraint from_table, fk_name_to_validate + end + private def schema_creation PostgreSQL::SchemaCreation.new(self) @@ -590,6 +603,10 @@ module ActiveRecord PostgreSQL::TableDefinition.new(*args) end + def create_alter_table(name) + PostgreSQL::AlterTable.new create_table_definition(name) + end + def new_column_from_field(table_name, field) column_name, type, default, notnull, oid, fmod, collation, comment = field type_metadata = fetch_type_metadata(column_name, type, oid.to_i, fmod.to_i) @@ -629,6 +646,66 @@ module ActiveRecord end end + def change_column_sql(table_name, column_name, type, options = {}) + quoted_column_name = quote_column_name(column_name) + sql_type = type_to_sql(type, options) + sql = "ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}".dup + if options[:collation] + sql << " COLLATE \"#{options[:collation]}\"" + end + if options[:using] + sql << " USING #{options[:using]}" + elsif options[:cast_as] + cast_as_type = type_to_sql(options[:cast_as], options) + sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" + end + + sql + end + + def change_column_for_alter(table_name, column_name, type, options = {}) + sqls = [change_column_sql(table_name, column_name, type, options)] + procs = [] + sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) + sqls << change_column_null_for_alter(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + procs << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment) + + [sqls, procs] + end + + + # Changes the default value of a table column. + def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc: + column = column_for(table_name, column_name) + return unless column + + default = extract_new_default_value(default_or_changes) + alter_column_query = "ALTER COLUMN #{quote_column_name(column_name)} %s" + if default.nil? + # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will + # cast the default to the columns type, which leaves us with a default like "default NULL::character varying". + alter_column_query % "DROP DEFAULT" + else + alter_column_query % "SET DEFAULT #{quote_default_expression(default, column)}" + end + end + + def change_column_null_for_alter(table_name, column_name, null, default = nil) #:nodoc: + "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL" + end + + def add_index_opclass(quoted_columns, **options) + opclasses = options_for_index_columns(options[:opclass]) + quoted_columns.each do |name, column| + column << " #{opclasses[name]}" if opclasses[name].present? + end + end + + def add_options_for_index_columns(quoted_columns, **options) + quoted_columns = add_index_opclass(quoted_columns, options) + super + end + def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) scope[:type] ||= "'r','v','m'" # (r)elation/table, (v)iew, (m)aterialized view diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 27011bfe92..23fc69d649 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -142,6 +142,10 @@ module ActiveRecord true end + def supports_validate_constraints? + true + end + def supports_views? true end @@ -386,7 +390,6 @@ module ActiveRecord end private - # See https://www.postgresql.org/docs/current/static/errcodes-appendix.html VALUE_LIMIT_VIOLATION = "22001" NUMERIC_VALUE_OUT_OF_RANGE = "22003" diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index daece2bffd..c72db15ce3 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -70,7 +70,8 @@ module ActiveRecord time: { name: "time" }, date: { name: "date" }, binary: { name: "blob" }, - boolean: { name: "boolean" } + boolean: { name: "boolean" }, + json: { name: "json" }, } ## @@ -134,6 +135,10 @@ module ActiveRecord true end + def supports_json? + true + end + def supports_multi_insert? sqlite_version >= "3.7.11" end @@ -369,6 +374,10 @@ module ActiveRecord end private + def initialize_type_map(m = type_map) + super + register_class_with_limit m, %r(int)i, SQLite3Integer + end def table_structure(table_name) structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA") @@ -398,18 +407,21 @@ module ActiveRecord options[:id] = false create_table(to, options) do |definition| @definition = definition - @definition.primary_key(from_primary_key) if from_primary_key.present? + if from_primary_key.is_a?(Array) + @definition.primary_keys from_primary_key + end columns(from).each do |column| column_name = options[:rename] ? (options[:rename][column.name] || options[:rename][column.name.to_sym] || column.name) : column.name - next if column_name == from_primary_key @definition.column(column_name, column.type, limit: column.limit, default: column.default, precision: column.precision, scale: column.scale, - null: column.null, collation: column.collation) + null: column.null, collation: column.collation, + primary_key: column_name == from_primary_key + ) end yield @definition if block_given? end @@ -422,6 +434,9 @@ module ActiveRecord def copy_table_indexes(from, to, rename = {}) indexes(from).each do |index| name = index.name + # indexes sqlite creates for internal use start with `sqlite_` and + # don't need to be copied + next if name.starts_with?("sqlite_") if to == "a#{from}" name = "t#{name}" elsif from == "a#{to}" @@ -524,6 +539,17 @@ module ActiveRecord def configure_connection execute("PRAGMA foreign_keys = ON", "SCHEMA") end + + class SQLite3Integer < Type::Integer # :nodoc: + private + def _limit + # INTEGER storage class can be stored 8 bytes value. + # See https://www.sqlite.org/datatype3.html#storage_classes_and_datatypes + limit || 8 + end + end + + ActiveRecord::Type.register(:integer, SQLite3Integer, adapter: :sqlite3) end ActiveSupport.run_load_hooks(:active_record_sqlite3adapter, SQLite3Adapter) end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 15e9c09ffb..5c10d4ff24 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1045,7 +1045,7 @@ module ActiveRecord new(:up, migrations(migrations_paths), nil) end - def get_all_versions(connection = Base.connection) + def get_all_versions if SchemaMigration.table_exists? SchemaMigration.all_versions.map(&:to_i) else @@ -1053,12 +1053,13 @@ module ActiveRecord end end - def current_version(connection = Base.connection) - get_all_versions(connection).max || 0 + def current_version(connection = nil) + get_all_versions.max || 0 + rescue ActiveRecord::NoDatabaseError end - def needs_migration?(connection = Base.connection) - (migrations(migrations_paths).collect(&:version) - get_all_versions(connection)).size > 0 + def needs_migration?(connection = nil) + (migrations(migrations_paths).collect(&:version) - get_all_versions).size > 0 end def any_migrations? diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index c979aaf0a0..bd8c054c28 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -16,6 +16,18 @@ module ActiveRecord V5_2 = Current class V5_1 < V5_2 + def change_column(table_name, column_name, type, options = {}) + if adapter_name == "PostgreSQL" + clear_cache! + sql = connection.send(:change_column_sql, table_name, column_name, type, options) + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql}" + change_column_default(table_name, column_name, options[:default]) if options.key?(:default) + change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + else + super + end + end end class V5_0 < V5_1 diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 4e1b05dbf6..a13b0d0181 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -99,7 +99,9 @@ module ActiveRecord # for updating all records in a single query. def update(id = :all, attributes) if id.is_a?(Array) - id.map.with_index { |one_id, idx| update(one_id, attributes[idx]) }.compact + id.map { |one_id| find(one_id) }.each_with_index { |object, idx| + object.update(attributes[idx]) + } elsif id == :all all.each { |record| record.update(attributes) } else @@ -112,7 +114,6 @@ module ActiveRecord object.update(attributes) object end - rescue RecordNotFound end # Destroy an object (or multiple objects) that has the given id. The object is instantiated first, @@ -136,11 +137,10 @@ module ActiveRecord # Todo.destroy(todos) def destroy(id) if id.is_a?(Array) - id.map { |one_id| destroy(one_id) }.compact + find(id).each(&:destroy) else find(id).destroy end - rescue RecordNotFound end # Deletes the row with a primary key matching the +id+ argument, using a diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 9ee8425e1b..4538ed6a5f 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -90,12 +90,15 @@ module ActiveRecord filename = File.join(app.config.paths["db"].first, "schema_cache.yml") if File.file?(filename) + current_version = ActiveRecord::Migrator.current_version + next if current_version.nil? + cache = YAML.load(File.read(filename)) - if cache.version == ActiveRecord::Migrator.current_version + if cache.version == current_version connection.schema_cache = cache connection_pool.schema_cache = cache.dup else - warn "Ignoring db/schema_cache.yml because it has expired. The current schema version is #{ActiveRecord::Migrator.current_version}, but the one in the cache is #{cache.version}." + warn "Ignoring db/schema_cache.yml because it has expired. The current schema version is #{current_version}, but the one in the cache is #{cache.version}." end end end diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 752bb38481..a502713e56 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -47,7 +47,7 @@ module ActiveRecord end def to_h(table_name = nil) - equalities = predicates.grep(Arel::Nodes::Equality) + equalities = equalities(predicates) if table_name equalities = equalities.select do |node| node.left.relation.name == table_name @@ -90,6 +90,20 @@ module ActiveRecord end private + def equalities(predicates) + equalities = [] + + predicates.each do |node| + case node + when Arel::Nodes::Equality + equalities << node + when Arel::Nodes::And + equalities.concat equalities(node.children) + end + end + + equalities + end def predicates_unreferenced_by(other) predicates.reject do |n| @@ -121,7 +135,7 @@ module ActiveRecord end def except_predicates(columns) - self.predicates.reject do |node| + predicates.reject do |node| case node when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual subrelation = (node.left.kind_of?(Arel::Attributes::Attribute) ? node.left : node.right) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 66f7d29886..16ccba6b6c 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -184,8 +184,9 @@ HEADER "name: #{index.name.inspect}", ] index_parts << "unique: true" if index.unique - index_parts << "length: { #{format_options(index.lengths)} }" if index.lengths.present? - index_parts << "order: { #{format_options(index.orders)} }" if index.orders.present? + index_parts << "length: #{format_index_parts(index.lengths)}" if index.lengths.present? + index_parts << "order: #{format_index_parts(index.orders)}" if index.orders.present? + index_parts << "opclass: #{format_index_parts(index.opclasses)}" if index.opclasses.present? index_parts << "where: #{index.where.inspect}" if index.where index_parts << "using: #{index.using.inspect}" if !@connection.default_index_type?(index) index_parts << "type: #{index.type.inspect}" if index.type @@ -231,6 +232,14 @@ HEADER options.map { |key, value| "#{key}: #{value.inspect}" }.join(", ") end + def format_index_parts(options) + if options.is_a?(Hash) + "{ #{format_options(options)} }" + else + options.inspect + end + end + def remove_prefix_and_suffix(table) prefix = Regexp.escape(@options[:table_name_prefix].to_s) suffix = Regexp.escape(@options[:table_name_suffix].to_s) diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index e61c70848a..13b4096671 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -174,10 +174,10 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase assert_equal "SCHEMA", @subscriber.logged[0][1] end - def test_logs_name_rename_column_sql + def test_logs_name_rename_column_for_alter @connection.execute "CREATE TABLE `bar_baz` (`foo` varchar(255))" @subscriber.logged.clear - @connection.send(:rename_column_sql, "bar_baz", "foo", "foo2") + @connection.send(:rename_column_for_alter, "bar_baz", "foo", "foo2") assert_equal "SCHEMA", @subscriber.logged[0][1] ensure @connection.execute "DROP TABLE `bar_baz`" diff --git a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb index 9929237546..99c53dadeb 100644 --- a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb @@ -59,6 +59,9 @@ class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase assert_equal expected, add_index(:people, "lower(last_name)", using: type, unique: true) end + expected = %(CREATE INDEX "index_people_on_last_name" ON "people" USING gist ("last_name" bpchar_pattern_ops)) + assert_equal expected, add_index(:people, :last_name, using: :gist, opclass: { last_name: :bpchar_pattern_ops }) + assert_raise ArgumentError do add_index(:people, :last_name, algorithm: :copy) end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index f199519d86..1951230c8a 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -248,12 +248,12 @@ module ActiveRecord def test_index_with_opclass with_example_table do - @connection.add_index "ex", "data varchar_pattern_ops" - index = @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data_varchar_pattern_ops" } - assert_equal "data varchar_pattern_ops", index.columns + @connection.add_index "ex", "data", opclass: "varchar_pattern_ops" + index = @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data" } + assert_equal ["data"], index.columns - @connection.remove_index "ex", "data varchar_pattern_ops" - assert_not @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data_varchar_pattern_ops" } + @connection.remove_index "ex", "data" + assert_not @connection.indexes("ex").find { |idx| idx.name == "index_ex_on_data" } end end diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 5a64da028b..1126908761 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -459,7 +459,7 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase assert_equal :btree, index_d.using assert_equal :gin, index_e.using - assert_equal :desc, index_d.orders[INDEX_D_COLUMN] + assert_equal :desc, index_d.orders end end @@ -500,6 +500,38 @@ class SchemaForeignKeyTest < ActiveRecord::PostgreSQLTestCase end end +class SchemaIndexOpclassTest < ActiveRecord::PostgreSQLTestCase + include SchemaDumpingHelper + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table "trains" do |t| + t.string :name + t.text :description + end + end + + teardown do + @connection.drop_table "trains", if_exists: true + end + + def test_string_opclass_is_dumped + @connection.execute "CREATE INDEX trains_name_and_description ON trains USING btree(name text_pattern_ops, description text_pattern_ops)" + + output = dump_table_schema "trains" + + assert_match(/opclass: :text_pattern_ops/, output) + end + + def test_non_default_opclass_is_dumped + @connection.execute "CREATE INDEX trains_name_and_description ON trains USING btree(name, description text_pattern_ops)" + + output = dump_table_schema "trains" + + assert_match(/opclass: \{ description: :text_pattern_ops \}/, output) + end +end + class DefaultsUsingMultipleSchemasAndDomainTest < ActiveRecord::PostgreSQLTestCase setup do @connection = ActiveRecord::Base.connection diff --git a/activerecord/test/cases/adapters/sqlite3/json_test.rb b/activerecord/test/cases/adapters/sqlite3/json_test.rb index 568a524058..6f247fcd22 100644 --- a/activerecord/test/cases/adapters/sqlite3/json_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/json_test.rb @@ -9,8 +9,8 @@ class SQLite3JSONTest < ActiveRecord::SQLite3TestCase def setup super @connection.create_table("json_data_type") do |t| - t.column "payload", :json, default: {} - t.column "settings", :json + t.json "payload", default: {} + t.json "settings" end end diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb index 1f057fe5c6..1357719422 100644 --- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb @@ -360,6 +360,51 @@ module ActiveRecord end end + class Barcode < ActiveRecord::Base + self.primary_key = "code" + end + + def test_copy_table_with_existing_records_have_custom_primary_key + connection = Barcode.connection + connection.create_table(:barcodes, primary_key: "code", id: :string, limit: 42, force: true) do |t| + t.text :other_attr + end + code = "214fe0c2-dd47-46df-b53b-66090b3c1d40" + Barcode.create!(code: code, other_attr: "xxx") + + connection.change_table "barcodes" do |t| + connection.remove_column("barcodes", "other_attr") + end + + assert_equal code, Barcode.first.id + ensure + Barcode.reset_column_information + end + + def test_copy_table_with_composite_primary_keys + connection = Barcode.connection + connection.create_table(:barcodes, primary_key: ["region", "code"], force: true) do |t| + t.string :region + t.string :code + t.text :other_attr + end + region = "US" + code = "214fe0c2-dd47-46df-b53b-66090b3c1d40" + Barcode.create!(region: region, code: code, other_attr: "xxx") + + connection.change_table "barcodes" do |t| + connection.remove_column("barcodes", "other_attr") + end + + assert_equal ["region", "code"], connection.primary_keys("barcodes") + + barcode = Barcode.first + assert_equal region, barcode.region + assert_equal code, barcode.code + ensure + Barcode.reset_column_information + end + def test_supports_extensions assert_not @conn.supports_extensions?, "does not support extensions" end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d79afa2ee9..3497f6aae4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -891,11 +891,9 @@ class BasicsTest < ActiveRecord::TestCase assert_equal 2147483648, company.rating end - unless current_adapter?(:SQLite3Adapter) - def test_bignum_pk - company = Company.create!(id: 2147483648, name: "foo") - assert_equal company, Company.find(company.id) - end + def test_bignum_pk + company = Company.create!(id: 2147483648, name: "foo") + assert_equal company, Company.find(company.id) end # TODO: extend defaults tests to other databases! diff --git a/activerecord/test/cases/json_attribute_test.rb b/activerecord/test/cases/json_attribute_test.rb index 63f3c77fc3..afc39d0420 100644 --- a/activerecord/test/cases/json_attribute_test.rb +++ b/activerecord/test/cases/json_attribute_test.rb @@ -19,14 +19,14 @@ class JsonAttributeTest < ActiveRecord::TestCase def setup super @connection.create_table("json_data_type") do |t| - t.text "payload" - t.text "settings" + t.string "payload" + t.string "settings" end end private def column_type - :text + :string end def klass diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb index a71485982c..b0c0f2c283 100644 --- a/activerecord/test/cases/json_shared_test_cases.rb +++ b/activerecord/test/cases/json_shared_test_cases.rb @@ -23,25 +23,23 @@ module JSONSharedTestCases def test_column column = klass.columns_hash["payload"] assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type + assert_type_match column_type, column.sql_type type = klass.type_for_attribute("payload") assert_not type.binary? end def test_change_table_supports_json - skip unless @connection.supports_json? @connection.change_table("json_data_type") do |t| t.public_send column_type, "users" end klass.reset_column_information column = klass.columns_hash["users"] assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type + assert_type_match column_type, column.sql_type end def test_schema_dumping - skip unless @connection.supports_json? output = dump_table_schema("json_data_type") assert_match(/t\.#{column_type}\s+"settings"/, output) end @@ -68,26 +66,26 @@ module JSONSharedTestCases end def test_rewrite - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + @connection.execute(insert_statement_per_database('{"k":"v"}')) x = klass.first x.payload = { '"a\'' => "b" } assert x.save! end def test_select - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + @connection.execute(insert_statement_per_database('{"k":"v"}')) x = klass.first assert_equal({ "k" => "v" }, x.payload) end def test_select_multikey - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')|) + @connection.execute(insert_statement_per_database('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')) x = klass.first assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload) end def test_null_json - @connection.execute("insert into json_data_type (payload) VALUES(null)") + @connection.execute(insert_statement_per_database("null")) x = klass.first assert_nil(x.payload) end @@ -109,13 +107,13 @@ module JSONSharedTestCases end def test_select_array_json_value - @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + @connection.execute(insert_statement_per_database('["v0",{"k1":"v1"}]')) x = klass.first assert_equal(["v0", { "k1" => "v1" }], x.payload) end def test_rewrite_array_json_value - @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + @connection.execute(insert_statement_per_database('["v0",{"k1":"v1"}]')) x = klass.first x.payload = ["v1", { "k2" => "v2" }, "v3"] assert x.save! @@ -255,4 +253,17 @@ module JSONSharedTestCases def klass JsonDataType end + + def assert_type_match(type, sql_type) + native_type = ActiveRecord::Base.connection.native_database_types[type][:name] + assert_match %r(\A#{native_type}\b), sql_type + end + + def insert_statement_per_database(values) + if current_adapter?(:OracleAdapter) + "insert into json_data_type (id, payload) VALUES (json_data_type_seq.nextval, '#{values}')" + else + "insert into json_data_type (payload) VALUES ('#{values}')" + end + end end diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index 7b0644e9c0..38a906c8f5 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -264,19 +264,18 @@ module ActiveRecord t.column :foo, :timestamp end - klass = Class.new(ActiveRecord::Base) - klass.table_name = "testings" + column = connection.columns(:testings).find { |c| c.name == "foo" } - assert_equal :datetime, klass.columns_hash["foo"].type + assert_equal :datetime, column.type if current_adapter?(:PostgreSQLAdapter) - assert_equal "timestamp without time zone", klass.columns_hash["foo"].sql_type + assert_equal "timestamp without time zone", column.sql_type elsif current_adapter?(:Mysql2Adapter) - assert_equal "timestamp", klass.columns_hash["foo"].sql_type + assert_equal "timestamp", column.sql_type elsif current_adapter?(:OracleAdapter) - assert_equal "TIMESTAMP(6)", klass.columns_hash["foo"].sql_type + assert_equal "TIMESTAMP(6)", column.sql_type else - assert_equal klass.connection.type_to_sql("datetime"), klass.columns_hash["foo"].sql_type + assert_equal connection.type_to_sql("datetime"), column.sql_type end end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 2fef2f796e..cc2391f349 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -16,7 +16,7 @@ module ActiveRecord ActiveRecord::Migration.verbose = false connection.create_table :testings do |t| - t.column :foo, :string, limit: 100 + t.column :foo, :string, limit: 5 t.column :bar, :string, limit: 100 end end @@ -126,6 +126,25 @@ module ActiveRecord end assert_match(/LegacyMigration < ActiveRecord::Migration\[4\.2\]/, e.message) end + + if current_adapter?(:PostgreSQLAdapter) + class Testing < ActiveRecord::Base + end + + def test_legacy_change_column_with_null_executes_update + migration = Class.new(ActiveRecord::Migration[5.1]) { + def migrate(x) + change_column :testings, :foo, :string, limit: 10, null: false, default: "foobar" + end + }.new + + Testing.create! + ActiveRecord::Migrator.new(:up, [migration]).migrate + assert_equal ["foobar"], Testing.all.map(&:foo) + ensure + ActiveRecord::Base.clear_cache! + end + end end end end diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index 499d072de5..079be04946 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -227,6 +227,74 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end end + if ActiveRecord::Base.connection.supports_validate_constraints? + def test_add_invalid_foreign_key + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + refute fk.validated? + end + + def test_validate_foreign_key_infers_column + @connection.add_foreign_key :astronauts, :rockets, validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, :rockets + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_column + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, column: "rocket_id" + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_symbol_column + @connection.add_foreign_key :astronauts, :rockets, column: :rocket_id, validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, column: :rocket_id + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_name + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk", validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, name: "fancy_named_fk" + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_non_existing_foreign_key_raises + assert_raises ArgumentError do + @connection.validate_foreign_key :astronauts, :rockets + end + end + + def test_validate_constraint_by_name + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk", validate: false + + @connection.validate_constraint :astronauts, "fancy_named_fk" + assert @connection.foreign_keys("astronauts").first.validated? + end + else + # Foreign key should still be created, but should not be invalid + def test_add_invalid_foreign_key + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert fk.validated? + end + end + def test_schema_dumping @connection.add_foreign_key :astronauts, :rockets output = dump_table_schema "astronauts" diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index f088c064f5..4cc66a2e49 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -70,7 +70,7 @@ class PersistenceTest < ActiveRecord::TestCase end def test_update_many - topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, nil => {} } + topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } } updated = Topic.update(topic_data.keys, topic_data.values) assert_equal [1, 2], updated.map(&:id) @@ -78,10 +78,33 @@ class PersistenceTest < ActiveRecord::TestCase assert_equal "2 updated", Topic.find(2).content end + def test_update_many_with_duplicated_ids + updated = Topic.update([1, 1, 2], [ + { "content" => "1 duplicated" }, { "content" => "1 updated" }, { "content" => "2 updated" } + ]) + + assert_equal [1, 1, 2], updated.map(&:id) + assert_equal "1 updated", Topic.find(1).content + assert_equal "2 updated", Topic.find(2).content + end + + def test_update_many_with_invalid_id + topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, 99999 => {} } + + assert_raise(ActiveRecord::RecordNotFound) do + Topic.update(topic_data.keys, topic_data.values) + end + + assert_not_equal "1 updated", Topic.find(1).content + assert_not_equal "2 updated", Topic.find(2).content + end + def test_class_level_update_is_affected_by_scoping topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } } - assert_equal [], Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) } + assert_raise(ActiveRecord::RecordNotFound) do + Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) } + end assert_not_equal "1 updated", Topic.find(1).content assert_not_equal "2 updated", Topic.find(2).content @@ -175,15 +198,25 @@ class PersistenceTest < ActiveRecord::TestCase end def test_destroy_many - clients = Client.all.merge!(order: "id").find([2, 3]) + clients = Client.find([2, 3]) assert_difference("Client.count", -2) do - destroyed = Client.destroy([2, 3, nil]).sort_by(&:id) + destroyed = Client.destroy([2, 3]) assert_equal clients, destroyed assert destroyed.all?(&:frozen?), "destroyed clients should be frozen" end end + def test_destroy_many_with_invalid_id + clients = Client.find([2, 3]) + + assert_raise(ActiveRecord::RecordNotFound) do + Client.destroy([2, 3, 99999]) + end + + assert_equal clients, Client.find([2, 3]) + end + def test_becomes assert_kind_of Reply, topics(:first).becomes(Reply) assert_equal "The First Topic", topics(:first).becomes(Reply).title @@ -473,10 +506,18 @@ class PersistenceTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { Topic.find(topic.id) } end - def test_record_not_found_exception + def test_find_raises_record_not_found_exception assert_raise(ActiveRecord::RecordNotFound) { Topic.find(99999) } end + def test_update_raises_record_not_found_exception + assert_raise(ActiveRecord::RecordNotFound) { Topic.update(99999, approved: true) } + end + + def test_destroy_raises_record_not_found_exception + assert_raise(ActiveRecord::RecordNotFound) { Topic.destroy(99999) } + end + def test_update_all assert_equal Topic.count, Topic.update_all("content = 'bulk updated!'") assert_equal "bulk updated!", Topic.find(1).content @@ -938,7 +979,9 @@ class PersistenceTest < ActiveRecord::TestCase should_not_be_destroyed_reply = Reply.create("title" => "hello", "content" => "world") Topic.find(1).replies << should_not_be_destroyed_reply - assert_nil Topic.where("1=0").scoping { Topic.destroy(1) } + assert_raise(ActiveRecord::RecordNotFound) do + Topic.where("1=0").scoping { Topic.destroy(1) } + end assert_nothing_raised { Topic.find(1) } assert_nothing_raised { Reply.find(should_not_be_destroyed_reply.id) } diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index a71d8de521..b424ca91de 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -68,7 +68,7 @@ module ActiveRecord relation = Relation.new(Post, Post.arel_table, Post.predicate_builder) left = relation.table[:id].eq(10) right = relation.table[:id].eq(10) - combine = left.and right + combine = left.or(right) relation.where! combine assert_equal({}, relation.where_values_hash) end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 50ad1d5b26..675aafabda 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1189,6 +1189,15 @@ class RelationTest < ActiveRecord::TestCase assert_equal "hen", hen.name end + def test_create_with_polymorphic_association + author = authors(:david) + post = posts(:welcome) + comment = Comment.where(post: post, author: author).create!(body: "hello") + + assert_equal author, comment.author + assert_equal post, comment.post + end + def test_first_or_create parrot = Bird.where(color: "green").first_or_create(name: "parrot") assert_kind_of Bird, parrot diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index ac5092f1c1..a612ce9bb2 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -177,14 +177,14 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dumps_index_columns_in_right_order index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*company_index/).first.strip - if current_adapter?(:PostgreSQLAdapter, :SQLite3Adapter) - assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition - elsif current_adapter?(:Mysql2Adapter) + if current_adapter?(:Mysql2Adapter) if ActiveRecord::Base.connection.supports_index_sort_order? assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }, order: { rating: :desc }', index_definition else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }', index_definition end + elsif ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index"', index_definition end @@ -199,6 +199,24 @@ class SchemaDumperTest < ActiveRecord::TestCase end end + def test_schema_dumps_index_sort_order + index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_rating/).first.strip + if ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["name", "rating"], name: "index_companies_on_name_and_rating", order: :desc', index_definition + else + assert_equal 't.index ["name", "rating"], name: "index_companies_on_name_and_rating"', index_definition + end + end + + def test_schema_dumps_index_length + index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_description/).first.strip + if current_adapter?(:Mysql2Adapter) + assert_equal 't.index ["name", "description"], name: "index_companies_on_name_and_description", length: 10', index_definition + else + assert_equal 't.index ["name", "description"], name: "index_companies_on_name_and_description"', index_definition + end + end + def test_schema_dump_should_honor_nonstandard_primary_keys output = standard_dump match = output.match(%r{create_table "movies"(.*)do}) diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index a4505a4892..bf66846840 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -205,6 +205,8 @@ ActiveRecord::Schema.define do t.bigint :rating, default: 1 t.integer :account_id t.string :description, default: "" + t.index [:name, :rating], order: :desc + t.index [:name, :description], length: 10 t.index [:firm_id, :type, :rating], name: "company_index", length: { type: 10 }, order: { rating: :desc } t.index [:firm_id, :type], name: "company_partial_index", where: "(rating > 10)" t.index :name, name: "company_name_index", using: :btree |