From 1dca75c2c8f930b58d86cd2216af5e14307b3e53 Mon Sep 17 00:00:00 2001
From: Greg Navis <contact@gregnavis.com>
Date: Sat, 13 May 2017 03:09:58 +0200
Subject: Add support for PostgreSQL operator classes to add_index

Add support for specifying non-default operator classes in PostgreSQL
indexes. An example CREATE INDEX query that becomes possible is:

    CREATE INDEX users_name ON users USING gist (name gist_trgm_ops);

Previously it was possible to specify the `gist` index but not the
custom operator class. The `add_index` call for the above query is:

    add_index :users, :name, using: :gist, opclasses: {name: :gist_trgm_ops}
---
 activerecord/CHANGELOG.md                          |  8 ++++++
 .../abstract/schema_definitions.rb                 |  4 ++-
 .../abstract/schema_statements.rb                  | 28 +++++++++++++++++--
 .../postgresql/schema_statements.rb                | 32 ++++++++++++++--------
 .../connection_adapters/postgresql_adapter.rb      | 17 ++++++++++++
 activerecord/lib/active_record/schema_dumper.rb    |  1 +
 .../adapters/postgresql/active_schema_test.rb      |  3 ++
 .../adapters/postgresql/postgresql_adapter_test.rb | 10 +++----
 .../test/cases/adapters/postgresql/schema_test.rb  | 32 ++++++++++++++++++++++
 9 files changed, 115 insertions(+), 20 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 60ceffac5e..048a1eeb2f 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.
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..b66009bdc6 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, :where, :type, :using, :opclass, :comment
 
       def initialize(
         table, name,
@@ -17,6 +17,7 @@ module ActiveRecord
         where: nil,
         type: nil,
         using: nil,
+        opclass: {},
         comment: nil
       )
         @table = table
@@ -28,6 +29,7 @@ module ActiveRecord
         @where = where
         @type = type
         @using = using
+        @opclass = opclass
         @comment = comment
       end
     end
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..2a335e8f2b 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -738,6 +738,29 @@ 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)
@@ -1120,7 +1143,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" : ""
@@ -1186,7 +1209,8 @@ module ActiveRecord
           quoted_columns
         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)
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..15a1dfd102 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,10 @@ 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
+            if indkey.include?(0)
               columns = expressions
             else
               columns = Hash[query(<<-SQL.strip_heredoc, "SCHEMA")].values_at(*indkey).compact
@@ -123,10 +119,21 @@ 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] }
-              ]
+              orders = {}
+              opclasses = {}
+
+              # 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 if opclass
+                orders[column] = :desc if desc
+              end
+
+              # Use a string for the opclass description (instead of a hash) when all columns
+              # have the same opclass specified.
+              if columns.count == opclasses.count && opclasses.values.uniq.count == 1
+                opclasses = opclasses.values.first
+              end
             end
 
             IndexDefinition.new(
@@ -137,6 +144,7 @@ module ActiveRecord
               orders: orders,
               where: where,
               using: using.to_sym,
+              opclass: opclasses,
               comment: comment.presence
             )
           end
@@ -458,8 +466,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
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 7b27f6b7a0..eee61780b7 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -388,6 +388,23 @@ module ActiveRecord
 
       private
 
+        def add_index_opclass(column_names, options = {})
+          opclass =
+            if options[:opclass].is_a?(Hash)
+              options[:opclass].symbolize_keys
+            else
+              Hash.new { |hash, column| hash[column] = options[:opclass].to_s }
+            end
+          column_names.each do |name, column|
+            column << " #{opclass[name]}" if opclass[name].present?
+          end
+        end
+
+        def add_options_for_index_columns(quoted_columns, **options)
+          quoted_columns = add_index_opclass(quoted_columns, options)
+          super
+        end
+
         # 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/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 66f7d29886..8cb2851557 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -189,6 +189,7 @@ HEADER
         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
+        index_parts << "opclass: #{index.opclass.inspect}" if index.opclass.present?
         index_parts << "comment: #{index.comment.inspect}" if index.comment
         index_parts
       end
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..a6ae18a826 100644
--- a/activerecord/test/cases/adapters/postgresql/schema_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb
@@ -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
-- 
cgit v1.2.3


From 0dc3b6535dfb8127f568bb4abc2fb09118ce4b96 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 1 Dec 2017 11:49:36 +0900
Subject: Maintain raising `RecordNotFound` for class level `update` and`
 destroy`

In 35836019, class level `update` and `destroy` suppressed
`RecordNotFound` to ensure returning affected objects. But
`RecordNotFound` is a common exception caught by a `rescue_from`
handler. So changing the behavior when a typical `params[:id]` is passed
has a compatibility problem. The previous behavior should not be
changed.

Fixes #31301.
---
 activerecord/lib/active_record/persistence.rb | 13 +++++++++----
 activerecord/test/cases/persistence_test.rb   | 14 ++++++++++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 4e1b05dbf6..17459f2505 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -99,7 +99,11 @@ 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.with_index { |one_id, idx|
+            object = find_by(primary_key => one_id)
+            object.update(attributes[idx]) if object
+            object
+          }.compact
         elsif id == :all
           all.each { |record| record.update(attributes) }
         else
@@ -112,7 +116,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 +139,13 @@ module ActiveRecord
       #   Todo.destroy(todos)
       def destroy(id)
         if id.is_a?(Array)
-          id.map { |one_id| destroy(one_id) }.compact
+          id.map { |one_id|
+            object = find_by(primary_key => one_id)
+            object.destroy if object
+          }.compact
         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/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index f088c064f5..643032e7cf 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -473,10 +473,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 +946,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) }
-- 
cgit v1.2.3


From 695ec1fef0b7fdb3125a3e2e4364a7f449265c1b Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 1 Dec 2017 18:21:21 +0900
Subject: Class level `update` and `destroy` checks all the records exist
 before making changes (#31306)

It makes more sense than ignoring invalid IDs.
---
 activerecord/lib/active_record/persistence.rb | 13 +++------
 activerecord/test/cases/persistence_test.rb   | 41 ++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 13 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index 17459f2505..a13b0d0181 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -99,11 +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|
-            object = find_by(primary_key => one_id)
-            object.update(attributes[idx]) if object
-            object
-          }.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
@@ -139,10 +137,7 @@ module ActiveRecord
       #   Todo.destroy(todos)
       def destroy(id)
         if id.is_a?(Array)
-          id.map { |one_id|
-            object = find_by(primary_key => one_id)
-            object.destroy if object
-          }.compact
+          find(id).each(&:destroy)
         else
           find(id).destroy
         end
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 643032e7cf..4aea275d51 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.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
+  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
-- 
cgit v1.2.3


From 05d1e9e413a87bee5b0c1c200fa9f84dba0e1d15 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 1 Dec 2017 18:28:31 +0900
Subject: Remove unnecessary scoping

---
 activerecord/test/cases/persistence_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 4aea275d51..4cc66a2e49 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -92,7 +92,7 @@ class PersistenceTest < ActiveRecord::TestCase
     topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, 99999 => {} }
 
     assert_raise(ActiveRecord::RecordNotFound) do
-      Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) }
+      Topic.update(topic_data.keys, topic_data.values)
     end
 
     assert_not_equal "1 updated", Topic.find(1).content
-- 
cgit v1.2.3


From 6b16f6db279bdbd95e8b295c9bab6760a6ff6af0 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 1 Dec 2017 20:42:04 +0900
Subject: Tweaks CHANGELOGs [ci skip]

---
 activerecord/CHANGELOG.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 048a1eeb2f..89a12d4223 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -2,7 +2,7 @@
 
     Example:
 
-        add_index :users, :name, using: :gist, opclass: name: :gist_trgm_ops
+        add_index :users, :name, using: :gist, opclass: { name: :gist_trgm_ops }
 
     *Greg Navis*
 
@@ -80,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.
 
-- 
cgit v1.2.3


From 32516e8862946da963e4ba9256553156ca321596 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 1 Dec 2017 20:48:29 +0900
Subject: Remove unpaired `}` [ci skip]

---
 .../active_record/connection_adapters/abstract/schema_statements.rb    | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

(limited to 'activerecord')

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 2a335e8f2b..7ae32d0ced 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -752,13 +752,12 @@ module ActiveRecord
       #
       #   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 })
+      #   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
-- 
cgit v1.2.3


From 8203482a9ef9bd60d5014745fd7af868d9954b1d Mon Sep 17 00:00:00 2001
From: Travis Hunter <travis.c.hunter@gmail.com>
Date: Fri, 20 Jan 2017 16:18:46 -0500
Subject: Add support for invalid foreign keys in Postgres

Add validate_constraint and update naming
---
 .../abstract/schema_definitions.rb                 |  5 ++
 .../abstract/schema_statements.rb                  |  2 +
 .../connection_adapters/abstract_adapter.rb        |  5 ++
 .../postgresql/schema_creation.rb                  | 12 ++++
 .../postgresql/schema_definitions.rb               | 13 +++++
 .../postgresql/schema_statements.rb                | 44 +++++++++++++-
 .../connection_adapters/postgresql_adapter.rb      |  4 ++
 .../test/cases/migration/foreign_key_test.rb       | 68 ++++++++++++++++++++++
 8 files changed, 152 insertions(+), 1 deletion(-)

(limited to 'activerecord')

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 b66009bdc6..88ef8e0819 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -87,6 +87,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
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 7ae32d0ced..09ccf0c193 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -964,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?
 
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/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..a3bb66bf3e 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
@@ -192,6 +192,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 15a1dfd102..38102b2e7a 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -507,7 +507,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
@@ -529,6 +529,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
@@ -589,6 +590,43 @@ module ActiveRecord
           PostgreSQL::SchemaDumper.create(self, options)
         end
 
+        # Validates the given constraint.
+        #
+        # Validates the constraint named +constraint_name+ on +accounts+.
+        #
+        #   validate_foreign_key :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)
@@ -598,6 +636,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)
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 9397481c4c..68b79f777e 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
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"
-- 
cgit v1.2.3


From 70c96b4423abacecb1116184c8ea4d4662b809f8 Mon Sep 17 00:00:00 2001
From: "yuuji.yaginuma" <yuuji.yaginuma@gmail.com>
Date: Sat, 2 Dec 2017 11:18:38 +0900
Subject: Fix method name in `validate_constraint` doc [ci skip]

---
 .../active_record/connection_adapters/postgresql/schema_statements.rb   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

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 38102b2e7a..af13a9e16f 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -594,7 +594,7 @@ module ActiveRecord
         #
         # Validates the constraint named +constraint_name+ on +accounts+.
         #
-        #   validate_foreign_key :accounts, :constraint_name
+        #   validate_constraint :accounts, :constraint_name
         def validate_constraint(table_name, constraint_name)
           return unless supports_validate_constraints?
 
-- 
cgit v1.2.3


From dd6338a0699f2301d4b2fc8653688b4c4183cee5 Mon Sep 17 00:00:00 2001
From: Dinah Shi <dinahshi28@gmail.com>
Date: Sat, 2 Dec 2017 14:30:10 +0800
Subject: Extract sql fragment generators for alter table from PostgreSQL
 adapter

---
 .../abstract/schema_statements.rb                  | 16 +++-
 .../connection_adapters/abstract_mysql_adapter.rb  | 36 +++------
 .../postgresql/schema_statements.rb                | 86 +++++++++++++---------
 .../lib/active_record/migration/compatibility.rb   |  9 +++
 .../test/cases/adapters/mysql2/connection_test.rb  |  4 +-
 .../test/cases/migration/compatibility_test.rb     | 17 +++++
 6 files changed, 107 insertions(+), 61 deletions(-)

(limited to 'activerecord')

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..f0c6ad63f9 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.
@@ -1340,6 +1340,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_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 0e552dba95..2926366e10 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
 
@@ -668,13 +668,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
 
@@ -695,7 +689,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,
@@ -709,31 +703,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/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 846e721983..371597031c 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -391,51 +391,24 @@ module ActiveRecord
         end
 
         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 #{quoted_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+
@@ -629,6 +602,53 @@ module ActiveRecord
             end
           end
 
+          def change_column_for_alter(table_name, column_name, type, options = {})
+            clear_cache!
+            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
+
+            sqls = [sql]
+            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:
+            clear_cache!
+            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:
+            clear_cache!
+            "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL"
+          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/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index c979aaf0a0..da307c1723 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -16,6 +16,15 @@ module ActiveRecord
       V5_2 = Current
 
       class V5_1 < V5_2
+        if adapter_name == "PostgreSQL"
+          def change_column(table_name, column_name, type, options = {})
+            unless options[:null] || options[:default].nil?
+              column = connection.send(:column_for, table_name, column_name)
+              connection.execute("UPDATE #{connection.quote_table_name(table_name)} SET #{connection.quote_column_name(column_name)}=#{connection.quote_default_expression(options[:default], column)} WHERE #{connection.quote_column_name(column_name)} IS NULL") if column
+            end
+            connection.change_column(table_name, column_name, type, options)
+          end
+        end
       end
 
       class V5_0 < V5_1
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/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index 2fef2f796e..0fd55e6ae4 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -126,6 +126,23 @@ 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, null: false, default: "foobar"
+            end
+          }.new
+
+          t = Testing.create!
+          ActiveRecord::Migrator.new(:up, [migration]).migrate
+          assert_equal ["foobar"], Testing.all.map(&:foo)
+        end
+      end
     end
   end
 end
-- 
cgit v1.2.3


From 01fb0907b2dafcecf63eedb80408d4cbff5f4d23 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 04:57:04 +0900
Subject: Refactor `length`, `order`, and `opclass` index options dumping

---
 .../abstract/schema_definitions.rb                 | 19 ++++++++++++-----
 .../abstract/schema_statements.rb                  |  2 +-
 .../connection_adapters/abstract_mysql_adapter.rb  |  2 +-
 .../connection_adapters/mysql/schema_statements.rb | 15 ++++++++------
 .../postgresql/schema_statements.rb                | 16 +++++----------
 activerecord/lib/active_record/schema_dumper.rb    | 14 ++++++++++---
 .../test/cases/adapters/postgresql/schema_test.rb  |  6 +++---
 activerecord/test/cases/schema_dumper_test.rb      | 24 +++++++++++++++++++---
 activerecord/test/schema/schema.rb                 |  2 ++
 9 files changed, 67 insertions(+), 33 deletions(-)

(limited to 'activerecord')

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 88ef8e0819..86406d95ad 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, :opclass, :comment
+      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :comment
 
       def initialize(
         table, name,
@@ -14,24 +14,33 @@ module ActiveRecord
         columns = [],
         lengths: {},
         orders: {},
+        opclasses: {},
         where: nil,
         type: nil,
         using: nil,
-        opclass: {},
         comment: nil
       )
         @table = table
         @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
-        @opclass = opclass
         @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
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 fac9e1cca2..efc934c34c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1202,7 +1202,7 @@ module ActiveRecord
             when Hash
               order = order.symbolize_keys
               quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? }
-            when String
+            else
               quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? }
             end
           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 143dc6d08d..41c628302d 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -611,7 +611,7 @@ module ActiveRecord
             when Hash
               length = length.symbolize_keys
               quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? }
-            when Integer
+            else
               quoted_columns.each { |name, column| column << "(#{length})" }
             end
           end
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
index a15c7d1787..d5b2c18ee6 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 = {})
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 64f08e0ee1..7b4d5711cb 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -109,6 +109,9 @@ module ActiveRecord
 
             using, expressions, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: WHERE (.+))?\z/).flatten
 
+            orders = {}
+            opclasses = {}
+
             if indkey.include?(0)
               columns = expressions
             else
@@ -119,21 +122,12 @@ module ActiveRecord
                 AND a.attnum IN (#{indkey.join(",")})
               SQL
 
-              orders = {}
-              opclasses = {}
-
               # 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 if opclass
+                opclasses[column] = opclass.to_sym if opclass
                 orders[column] = :desc if desc
               end
-
-              # Use a string for the opclass description (instead of a hash) when all columns
-              # have the same opclass specified.
-              if columns.count == opclasses.count && opclasses.values.uniq.count == 1
-                opclasses = opclasses.values.first
-              end
             end
 
             IndexDefinition.new(
@@ -142,9 +136,9 @@ module ActiveRecord
               unique,
               columns,
               orders: orders,
+              opclasses: opclasses,
               where: where,
               using: using.to_sym,
-              opclass: opclasses,
               comment: comment.presence
             )
           end
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 8cb2851557..16ccba6b6c 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -184,12 +184,12 @@ 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
-        index_parts << "opclass: #{index.opclass.inspect}" if index.opclass.present?
         index_parts << "comment: #{index.comment.inspect}" if index.comment
         index_parts
       end
@@ -232,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/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb
index a6ae18a826..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
 
@@ -520,7 +520,7 @@ class SchemaIndexOpclassTest < ActiveRecord::PostgreSQLTestCase
 
     output = dump_table_schema "trains"
 
-    assert_match(/opclass: "text_pattern_ops"/, output)
+    assert_match(/opclass: :text_pattern_ops/, output)
   end
 
   def test_non_default_opclass_is_dumped
@@ -528,7 +528,7 @@ class SchemaIndexOpclassTest < ActiveRecord::PostgreSQLTestCase
 
     output = dump_table_schema "trains"
 
-    assert_match(/opclass: \{"description"=>"text_pattern_ops"\}/, output)
+    assert_match(/opclass: \{ description: :text_pattern_ops \}/, output)
   end
 end
 
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
-- 
cgit v1.2.3


From 3040446cece8e7a6d9e29219e636e13f180a1e03 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 06:33:56 +0900
Subject: Fix warning: assigned but unused variable - t

---
 activerecord/test/cases/migration/compatibility_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index 0fd55e6ae4..f1defe1a8a 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -138,7 +138,7 @@ module ActiveRecord
             end
           }.new
 
-          t = Testing.create!
+          Testing.create!
           ActiveRecord::Migrator.new(:up, [migration]).migrate
           assert_equal ["foobar"], Testing.all.map(&:foo)
         end
-- 
cgit v1.2.3


From 33a3f7123bc4cc49b99b01a40bbfd463b2e73f76 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 07:08:30 +0900
Subject: Extract duplicated index column options normalization as
 `options_for_index_columns`

And placed `add_options_for_index_columns` in `schema_statements.rb`
consistently to ease to find related code.
---
 .../connection_adapters/abstract/schema_statements.rb | 19 ++++++++++---------
 .../connection_adapters/abstract_mysql_adapter.rb     | 19 -------------------
 .../connection_adapters/mysql/schema_statements.rb    | 12 ++++++++++++
 .../postgresql/schema_statements.rb                   | 12 ++++++++++++
 .../connection_adapters/postgresql_adapter.rb         | 18 ------------------
 5 files changed, 34 insertions(+), 46 deletions(-)

(limited to 'activerecord')

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 efc934c34c..4f58b0242c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1197,17 +1197,18 @@ 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? }
-            else
-              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 and by
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 41c628302d..479131caad 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -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? }
-            else
-              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
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 d5b2c18ee6..ce50590651 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb
@@ -106,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_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index 7b4d5711cb..2e9e74403b 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -693,6 +693,18 @@ module ActiveRecord
             "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 68b79f777e..23fc69d649 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -390,24 +390,6 @@ module ActiveRecord
       end
 
       private
-
-        def add_index_opclass(column_names, options = {})
-          opclass =
-            if options[:opclass].is_a?(Hash)
-              options[:opclass].symbolize_keys
-            else
-              Hash.new { |hash, column| hash[column] = options[:opclass].to_s }
-            end
-          column_names.each do |name, column|
-            column << " #{opclass[name]}" if opclass[name].present?
-          end
-        end
-
-        def add_options_for_index_columns(quoted_columns, **options)
-          quoted_columns = add_index_opclass(quoted_columns, options)
-          super
-        end
-
         # See https://www.postgresql.org/docs/current/static/errcodes-appendix.html
         VALUE_LIMIT_VIOLATION = "22001"
         NUMERIC_VALUE_OUT_OF_RANGE = "22003"
-- 
cgit v1.2.3


From 5358f2b67bd6fb12d708527a4a70dcab65513c5e Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 07:55:34 +0900
Subject: Fix `test_add_column_with_timestamp_type` failure

This test failed due to dirty schema cache. It is needed to call
`clear_cache!` when using same named table with different definition.

https://travis-ci.org/rails/rails/jobs/310627767#L769-L772
---
 activerecord/test/cases/migration/change_schema_test.rb | 13 ++++++-------
 activerecord/test/cases/migration/compatibility_test.rb |  2 ++
 2 files changed, 8 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb
index 7b0644e9c0..43aac5da75 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 klass.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 f1defe1a8a..e6ca252369 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -141,6 +141,8 @@ module ActiveRecord
           Testing.create!
           ActiveRecord::Migrator.new(:up, [migration]).migrate
           assert_equal ["foobar"], Testing.all.map(&:foo)
+        ensure
+          ActiveRecord::Base.clear_cache!
         end
       end
     end
-- 
cgit v1.2.3


From 32516983a67f10d513370edd67b41ef0e011b175 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 08:05:32 +0900
Subject: Fix `s/klass.connection/connection/`

`klass` has removed in 5358f2b67bd6fb12d708527a4a70dcab65513c5e.
---
 activerecord/test/cases/migration/change_schema_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb
index 43aac5da75..38a906c8f5 100644
--- a/activerecord/test/cases/migration/change_schema_test.rb
+++ b/activerecord/test/cases/migration/change_schema_test.rb
@@ -275,7 +275,7 @@ module ActiveRecord
         elsif current_adapter?(:OracleAdapter)
           assert_equal "TIMESTAMP(6)", column.sql_type
         else
-          assert_equal klass.connection.type_to_sql("datetime"), column.sql_type
+          assert_equal connection.type_to_sql("datetime"), column.sql_type
         end
       end
 
-- 
cgit v1.2.3


From dbee80bca0ef504120219e6c7686437456511060 Mon Sep 17 00:00:00 2001
From: "yuuji.yaginuma" <yuuji.yaginuma@gmail.com>
Date: Fri, 1 Dec 2017 18:16:06 +0900
Subject: Make `Migrator.current_version` work without a current database

This is necessary in order to make the processing dependent on
`Migrator.current_version` work even without database.

Context: https://github.com/rails/rails/pull/31135#issuecomment-348404326
---
 activerecord/lib/active_record/migration.rb | 10 +++++++++-
 activerecord/lib/active_record/railtie.rb   |  7 +++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 15e9c09ffb..4d4b0dc67a 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -1053,7 +1053,15 @@ module ActiveRecord
         end
       end
 
-      def current_version(connection = Base.connection)
+      def current_version(connection = nil)
+        if connection.nil?
+          begin
+            connection = Base.connection
+          rescue ActiveRecord::NoDatabaseError
+            return nil
+          end
+        end
+
         get_all_versions(connection).max || 0
       end
 
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
-- 
cgit v1.2.3


From d0f5dce492696019ddf409892829f89bee5f45ef Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 15:00:05 +0900
Subject: `change_column_default` should be executed after type changing

If do not execute a type changing first, filling in default value may be
failed.

```
% ARCONN=postgresql be ruby -w -Itest test/cases/migration/compatibility_test.rb -n test_legacy_change_column_with_null_executes_update
Using postgresql
Run options: -n test_legacy_change_column_with_null_executes_update --seed 20459

E

Error:
ActiveRecord::Migration::CompatibilityTest#test_legacy_change_column_with_null_executes_update:
StandardError: An error has occurred, this and all later migrations canceled:

PG::StringDataRightTruncation: ERROR:  value too long for type character varying(5)
: UPDATE "testings" SET "foo"='foobar' WHERE "foo" IS NULL
```
---
 .../connection_adapters/postgresql/schema_statements.rb | 15 ++++++++-------
 .../lib/active_record/migration/compatibility.rb        | 17 ++++++++++-------
 activerecord/test/cases/migration/compatibility_test.rb |  4 ++--
 3 files changed, 20 insertions(+), 16 deletions(-)

(limited to 'activerecord')

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 2e9e74403b..bf5fbb30e1 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -393,9 +393,9 @@ module ActiveRecord
         end
 
         def change_column(table_name, column_name, type, options = {}) #:nodoc:
-          quoted_table_name = quote_table_name(table_name)
+          clear_cache!
           sqls, procs = change_column_for_alter(table_name, column_name, type, options)
-          execute "ALTER TABLE #{quoted_table_name} #{sqls.join(", ")}"
+          execute "ALTER TABLE #{quote_table_name(table_name)} #{sqls.join(", ")}"
           procs.each(&:call)
         end
 
@@ -646,8 +646,7 @@ module ActiveRecord
             end
           end
 
-          def change_column_for_alter(table_name, column_name, type, options = {})
-            clear_cache!
+          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
@@ -661,7 +660,11 @@ module ActiveRecord
               sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})"
             end
 
-            sqls = [sql]
+            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)
@@ -673,7 +676,6 @@ module ActiveRecord
 
           # Changes the default value of a table column.
           def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc:
-            clear_cache!
             column = column_for(table_name, column_name)
             return unless column
 
@@ -689,7 +691,6 @@ module ActiveRecord
           end
 
           def change_column_null_for_alter(table_name, column_name, null, default = nil) #:nodoc:
-            clear_cache!
             "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL"
           end
 
diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index da307c1723..bd8c054c28 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -16,13 +16,16 @@ module ActiveRecord
       V5_2 = Current
 
       class V5_1 < V5_2
-        if adapter_name == "PostgreSQL"
-          def change_column(table_name, column_name, type, options = {})
-            unless options[:null] || options[:default].nil?
-              column = connection.send(:column_for, table_name, column_name)
-              connection.execute("UPDATE #{connection.quote_table_name(table_name)} SET #{connection.quote_column_name(column_name)}=#{connection.quote_default_expression(options[:default], column)} WHERE #{connection.quote_column_name(column_name)} IS NULL") if column
-            end
-            connection.change_column(table_name, column_name, type, options)
+        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
diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index e6ca252369..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
@@ -134,7 +134,7 @@ module ActiveRecord
         def test_legacy_change_column_with_null_executes_update
           migration = Class.new(ActiveRecord::Migration[5.1]) {
             def migrate(x)
-              change_column :testings, :foo, :string, null: false, default: "foobar"
+              change_column :testings, :foo, :string, limit: 10, null: false, default: "foobar"
             end
           }.new
 
-- 
cgit v1.2.3


From 70fa9e9ab7fd89589664ecd7ee367448ef45f9d8 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 15:28:10 +0900
Subject: Emulate JSON types for SQLite3 adapter (#29664)

Actually SQLite3 doesn't have JSON storage class (so it is stored as a
TEXT like Date and Time). But emulating JSON types is convinient for
making database agnostic migrations.
---
 .../connection_adapters/abstract/schema_definitions.rb             | 1 +
 .../active_record/connection_adapters/mysql/schema_definitions.rb  | 4 ----
 .../connection_adapters/postgresql/schema_definitions.rb           | 4 ----
 .../lib/active_record/connection_adapters/sqlite3_adapter.rb       | 7 ++++++-
 activerecord/test/cases/adapters/sqlite3/json_test.rb              | 4 ++--
 activerecord/test/cases/json_shared_test_cases.rb                  | 2 --
 6 files changed, 9 insertions(+), 13 deletions(-)

(limited to 'activerecord')

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 86406d95ad..0594b4b485 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -220,6 +220,7 @@ module ActiveRecord
         :decimal,
         :float,
         :integer,
+        :json,
         :string,
         :text,
         :time,
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/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
index a3bb66bf3e..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
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index daece2bffd..ff63f63bf7 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
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/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb
index a71485982c..56ec8c8a82 100644
--- a/activerecord/test/cases/json_shared_test_cases.rb
+++ b/activerecord/test/cases/json_shared_test_cases.rb
@@ -30,7 +30,6 @@ module JSONSharedTestCases
   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
@@ -41,7 +40,6 @@ module JSONSharedTestCases
   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
-- 
cgit v1.2.3


From 9b823c02b62a91216ef1ad0c9d4fca095377afb6 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Sun, 3 Dec 2017 15:45:40 +0900
Subject: SQLite3 valid integer value should be 8 bytes (64-bit signed integer)
 (#28379)

This is a regression since Rails 4.2.

SQLite3 integer is stored in 1, 2, 3, 4, 6, or 8 bytes depending on the
magnitude of the value. Assuming default valid value as 4 bytes caused
that actual valid value in INTEGER storage class cannot be stored and
existing value cannot be found.

https://www.sqlite.org/datatype3.html

We should allow valid value in INTEGER storage class in SQLite3 to fix
the regression.

Fixes #22594.
---
 .../active_record/connection_adapters/sqlite3_adapter.rb  | 15 +++++++++++++++
 activerecord/test/cases/base_test.rb                      |  8 +++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index ff63f63bf7..574a7b4572 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -374,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")
@@ -529,6 +533,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/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!
-- 
cgit v1.2.3


From 07788c7ad8bad797ec97cba038e37e007f343afa Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 4 Dec 2017 08:18:31 +0900
Subject: `current_version` should catch `NoDatabaseError` from
 `get_all_versions`

`get_all_versions` doesn't use passed `connection`. So it should be
caught `NoDatabaseError` from `SchemaMigration.table_exists?`.
---
 activerecord/lib/active_record/migration.rb | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 4d4b0dc67a..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
@@ -1054,19 +1054,12 @@ module ActiveRecord
       end
 
       def current_version(connection = nil)
-        if connection.nil?
-          begin
-            connection = Base.connection
-          rescue ActiveRecord::NoDatabaseError
-            return nil
-          end
-        end
-
-        get_all_versions(connection).max || 0
+        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?
-- 
cgit v1.2.3


From 29d081c47a43cc5d979836541cb73a816c95981d Mon Sep 17 00:00:00 2001
From: Yasuo Honda <yasuo.honda@gmail.com>
Date: Tue, 5 Dec 2017 19:16:11 +0000
Subject: Execute `JsonAttributeTest` only if `supports_json?` returns `true`

Oracle enhanced adapter does not fully support JSON datatype then `supports_json?` returns `false`.
I wanted to skip known failures and errors when tested with Oracle enhanced adapter.
---
 activerecord/test/cases/json_attribute_test.rb | 46 ++++++++++++++------------
 1 file changed, 24 insertions(+), 22 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/json_attribute_test.rb b/activerecord/test/cases/json_attribute_test.rb
index 63f3c77fc3..a6fd4f34dc 100644
--- a/activerecord/test/cases/json_attribute_test.rb
+++ b/activerecord/test/cases/json_attribute_test.rb
@@ -3,33 +3,35 @@
 require "cases/helper"
 require "cases/json_shared_test_cases"
 
-class JsonAttributeTest < ActiveRecord::TestCase
-  include JSONSharedTestCases
-  self.use_transactional_tests = false
+if ActiveRecord::Base.connection.supports_json?
+  class JsonAttributeTest < ActiveRecord::TestCase
+    include JSONSharedTestCases
+    self.use_transactional_tests = false
 
-  class JsonDataTypeOnText < ActiveRecord::Base
-    self.table_name = "json_data_type"
+    class JsonDataTypeOnText < ActiveRecord::Base
+      self.table_name = "json_data_type"
 
-    attribute :payload,  :json
-    attribute :settings, :json
+      attribute :payload,  :json
+      attribute :settings, :json
 
-    store_accessor :settings, :resolution
-  end
-
-  def setup
-    super
-    @connection.create_table("json_data_type") do |t|
-      t.text "payload"
-      t.text "settings"
+      store_accessor :settings, :resolution
     end
-  end
 
-  private
-    def column_type
-      :text
+    def setup
+      super
+      @connection.create_table("json_data_type") do |t|
+        t.text "payload"
+        t.text "settings"
+      end
     end
 
-    def klass
-      JsonDataTypeOnText
-    end
+    private
+      def column_type
+        :text
+      end
+
+      def klass
+        JsonDataTypeOnText
+      end
+  end
 end
-- 
cgit v1.2.3


From d2321aacc7451f6b4f154d894adcb76820abba3c Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 6 Dec 2017 08:30:27 +0900
Subject: Revert "Merge pull request #31341 from
 yahonda/skip_json_attribute_test"

This reverts commit 23226d04f921b79f0077ba38c5a5a923b6d43f89, reversing
changes made to 7544cf7603959f25100b21f70b5e70354bed7e45.
---
 activerecord/test/cases/json_attribute_test.rb | 46 ++++++++++++--------------
 1 file changed, 22 insertions(+), 24 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/json_attribute_test.rb b/activerecord/test/cases/json_attribute_test.rb
index a6fd4f34dc..63f3c77fc3 100644
--- a/activerecord/test/cases/json_attribute_test.rb
+++ b/activerecord/test/cases/json_attribute_test.rb
@@ -3,35 +3,33 @@
 require "cases/helper"
 require "cases/json_shared_test_cases"
 
-if ActiveRecord::Base.connection.supports_json?
-  class JsonAttributeTest < ActiveRecord::TestCase
-    include JSONSharedTestCases
-    self.use_transactional_tests = false
+class JsonAttributeTest < ActiveRecord::TestCase
+  include JSONSharedTestCases
+  self.use_transactional_tests = false
 
-    class JsonDataTypeOnText < ActiveRecord::Base
-      self.table_name = "json_data_type"
+  class JsonDataTypeOnText < ActiveRecord::Base
+    self.table_name = "json_data_type"
 
-      attribute :payload,  :json
-      attribute :settings, :json
+    attribute :payload,  :json
+    attribute :settings, :json
 
-      store_accessor :settings, :resolution
-    end
+    store_accessor :settings, :resolution
+  end
 
-    def setup
-      super
-      @connection.create_table("json_data_type") do |t|
-        t.text "payload"
-        t.text "settings"
-      end
+  def setup
+    super
+    @connection.create_table("json_data_type") do |t|
+      t.text "payload"
+      t.text "settings"
     end
+  end
 
-    private
-      def column_type
-        :text
-      end
+  private
+    def column_type
+      :text
+    end
 
-      def klass
-        JsonDataTypeOnText
-      end
-  end
+    def klass
+      JsonDataTypeOnText
+    end
 end
-- 
cgit v1.2.3


From d721344280346f67c7d2dbffa9eaf9341e73673d Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Wed, 6 Dec 2017 08:33:16 +0900
Subject: Use `:string` instead of `:text` for `JsonAttributeTest`

Since CLOB data type has many limitations in Oracle SELECT WHERE clause.
---
 activerecord/test/cases/json_attribute_test.rb    | 6 +++---
 activerecord/test/cases/json_shared_test_cases.rb | 9 +++++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

(limited to 'activerecord')

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 56ec8c8a82..012aaffde1 100644
--- a/activerecord/test/cases/json_shared_test_cases.rb
+++ b/activerecord/test/cases/json_shared_test_cases.rb
@@ -23,7 +23,7 @@ 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?
@@ -36,7 +36,7 @@ module JSONSharedTestCases
     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
@@ -253,4 +253,9 @@ 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
 end
-- 
cgit v1.2.3


From b34838cbe29434212eb81ca1e31bb0ab0765e32a Mon Sep 17 00:00:00 2001
From: Yasuo Honda <yasuo.honda@gmail.com>
Date: Wed, 6 Dec 2017 13:09:51 +0000
Subject: Address `ActiveRecord::NotNullViolation: OCIError: ORA-01400` for
 Oracle database which requires primary key value mentioned in insert
 statement explicitly.

---
 activerecord/test/cases/json_shared_test_cases.rb | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb
index 012aaffde1..b0c0f2c283 100644
--- a/activerecord/test/cases/json_shared_test_cases.rb
+++ b/activerecord/test/cases/json_shared_test_cases.rb
@@ -66,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
@@ -107,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!
@@ -258,4 +258,12 @@ module JSONSharedTestCases
       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
-- 
cgit v1.2.3


From 5ac4f4d2563e7f9c5ffaecce4be4b9e2c5b0c081 Mon Sep 17 00:00:00 2001
From: Ashley Ellis Pierce <anellis12@gmail.com>
Date: Mon, 4 Dec 2017 16:44:33 -0500
Subject: Fix sqlite migrations with custom primary keys

Previously, if a record was created with a custom primary key, that
table could not be migrated using sqlite. While attempting to copy the
table, the type of the primary key was ignored.

Once that was corrected, copying the indexes would fail because custom
primary keys are autoindexed by sqlite by default.

To correct that, this skips copying the index if the index name begins
with "sqlite_". This is a reserved word that indicates that the
index is an internal schema object. SQLite prohibits applications from
creating objects whose names begin with "sqlite_", so this string should
be safe to use as a check.
ref https://www.sqlite.org/fileformat2.html#intschema
---
 .../connection_adapters/sqlite3_adapter.rb             |  6 +++++-
 .../cases/adapters/sqlite3/sqlite3_adapter_test.rb     | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index daece2bffd..3197b522b3 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -395,10 +395,11 @@ module ActiveRecord
 
         def copy_table(from, to, options = {})
           from_primary_key = primary_key(from)
+          from_primary_key_column = columns(from).select { |column| column.name == from_primary_key }.first
           options[:id] = false
           create_table(to, options) do |definition|
             @definition = definition
-            @definition.primary_key(from_primary_key) if from_primary_key.present?
+            @definition.primary_key(from_primary_key, from_primary_key_column.type) if from_primary_key.present?
             columns(from).each do |column|
               column_name = options[:rename] ?
                 (options[:rename][column.name] ||
@@ -422,6 +423,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}"
diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
index 1f057fe5c6..14f4997d5b 100644
--- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
@@ -360,6 +360,24 @@ module ActiveRecord
         end
       end
 
+      class Barcode < ActiveRecord::Base
+      end
+
+      def test_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
+      end
+
       def test_supports_extensions
         assert_not @conn.supports_extensions?, "does not support extensions"
       end
-- 
cgit v1.2.3


From 6a8ce7416d6615a13ee5c4b9f6bcd91cc5adef4d Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 8 Dec 2017 02:37:02 +0900
Subject: Fix `scope_for_create` to do not lose polymorphic associations

This regression was caused at 213796fb due to polymorphic predicates are
combined by `Arel::Nodes::And`. But I'd like to keep that combined
because it would help inverting polymorphic predicates correctly
(e9ba12f7), and we can collect equality nodes regardless of combined by
`Arel::Nodes::And` (`a AND (b AND c) AND d` == `a AND b AND c AND d`).
This change fixes the regression to collect equality nodes in
`Arel::Nodes::And` as well.

Fixes #31338.
---
 .../lib/active_record/relation/where_clause.rb         | 18 ++++++++++++++++--
 activerecord/test/cases/relation_test.rb               |  2 +-
 activerecord/test/cases/relations_test.rb              |  9 +++++++++
 3 files changed, 26 insertions(+), 3 deletions(-)

(limited to 'activerecord')

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/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
-- 
cgit v1.2.3


From 131cc6eab64eeae6c7f508dca4176183144cf3a6 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 8 Dec 2017 10:49:25 +0900
Subject: SQLite: Fix `copy_table` with composite primary keys

`connection.primary_key` also return composite primary keys, so
`from_primary_key_column` may not be found even if `from_primary_key` is
presented.

```
% ARCONN=sqlite3 be ruby -w -Itest
test/cases/adapters/sqlite3/sqlite3_adapter_test.rb -n
test_copy_table_with_composite_primary_keys
Using sqlite3
Run options: -n test_copy_table_with_composite_primary_keys --seed 19041

# Running:

E

Error:
ActiveRecord::ConnectionAdapters::SQLite3AdapterTest#test_copy_table_with_composite_primary_keys:
NoMethodError: undefined method `type' for nil:NilClass
    /path/to/rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb:411:in
    `block in copy_table'
```

This change fixes `copy_table` to do not lose composite primary keys.
---
 .../connection_adapters/sqlite3_adapter.rb         | 10 ++++---
 .../cases/adapters/sqlite3/sqlite3_adapter_test.rb | 31 ++++++++++++++++++++--
 2 files changed, 35 insertions(+), 6 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 65aba20052..c72db15ce3 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -404,22 +404,24 @@ module ActiveRecord
 
         def copy_table(from, to, options = {})
           from_primary_key = primary_key(from)
-          from_primary_key_column = columns(from).select { |column| column.name == from_primary_key }.first
           options[:id] = false
           create_table(to, options) do |definition|
             @definition = definition
-            @definition.primary_key(from_primary_key, from_primary_key_column.type) 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
diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
index 14f4997d5b..1357719422 100644
--- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
@@ -361,21 +361,48 @@ module ActiveRecord
       end
 
       class Barcode < ActiveRecord::Base
+        self.primary_key = "code"
       end
 
-      def test_existing_records_have_custom_primary_key
+      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"
+        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
-- 
cgit v1.2.3