From a3b16b955f053663feed41d938ba0d42363b2097 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Tue, 28 Feb 2017 19:42:02 +0900
Subject: [PATCH] `valid_type?` should accept only supported types

`valid_type?` is used in schema dumper to determine if a type is
supported. So if `valid_type?(:foobar)` is true, it means that schema
dumper is allowed to create `t.foobar`. But it doesn't work. I think
that `valid_type?` should accept only supported types.

https://github.com/rails/rails/blob/v5.1.0.beta1/activerecord/lib/active_record/schema_dumper.rb#L135-L142

```ruby
  columns.each do |column|
    raise StandardError, "Unknown type '#{column.sql_type}' for column '#{column.name}'" unless @connection.valid_type?(column.type)
    next if column.name == pk
    type, colspec = @connection.column_spec(column)
    tbl.print "    t.#{type} #{column.name.inspect}"
    tbl.print ", #{format_colspec(colspec)}" if colspec.present?
    tbl.puts
  end
```
---
 .../connection_adapters/sqlite3_adapter.rb       |  4 ----
 activerecord/test/cases/adapter_test.rb          | 10 ++++++++++
 .../cases/adapters/mysql2/mysql2_adapter_test.rb | 11 -----------
 .../postgresql/postgresql_adapter_test.rb        | 11 -----------
 .../adapters/sqlite3/sqlite3_adapter_test.rb     | 16 ----------------
 5 files changed, 10 insertions(+), 42 deletions(-)

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 285b0ec243..8b627a6d4d 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -158,10 +158,6 @@ def supports_index_sort_order?
         true
       end
 
-      def valid_type?(type) # :nodoc:
-        true
-      end
-
       # Returns 62. SQLite supports index names up to 64
       # characters. The rest is used by Rails internally to perform
       # temporary rename operations
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index 0c9d1dff9d..070fca240f 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -30,6 +30,16 @@ def test_create_record_with_pk_as_zero
       assert_nothing_raised { Book.destroy(0) }
     end
 
+    def test_valid_column
+      @connection.native_database_types.each_key do |type|
+        assert @connection.valid_type?(type)
+      end
+    end
+
+    def test_invalid_column
+      assert_not @connection.valid_type?(:foobar)
+    end
+
     def test_tables
       tables = @connection.tables
       assert_includes tables, "accounts"
diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
index aab3dcb724..565130c38f 100644
--- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
@@ -17,17 +17,6 @@ def test_exec_query_nothing_raises_with_no_result_queries
     end
   end
 
-  def test_valid_column
-    with_example_table do
-      column = @conn.columns("ex").find { |col| col.name == "id" }
-      assert @conn.valid_type?(column.type)
-    end
-  end
-
-  def test_invalid_column
-    assert_not @conn.valid_type?(:foobar)
-  end
-
   def test_columns_for_distinct_zero_orders
     assert_equal "posts.id",
       @conn.columns_for_distinct("posts.id", [])
diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
index 3054f0271f..003e6e62e7 100644
--- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
@@ -21,17 +21,6 @@ def test_bad_connection
         end
       end
 
-      def test_valid_column
-        with_example_table do
-          column = @connection.columns("ex").find { |col| col.name == "id" }
-          assert @connection.valid_type?(column.type)
-        end
-      end
-
-      def test_invalid_column
-        assert_not @connection.valid_type?(:foobar)
-      end
-
       def test_primary_key
         with_example_table do
           assert_equal "id", @connection.primary_key("ex")
diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
index a6afb7816b..fdd7b0157a 100644
--- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
@@ -49,22 +49,6 @@ def test_connect_memory_with_url
         end
       end
 
-      def test_valid_column
-        with_example_table do
-          column = @conn.columns("ex").find { |col| col.name == "id" }
-          assert @conn.valid_type?(column.type)
-        end
-      end
-
-      # sqlite3 databases should be able to support any type and not just the
-      # ones mentioned in the native_database_types.
-      #
-      # Therefore test_invalid column should always return true even if the
-      # type is not valid.
-      def test_invalid_column
-        assert @conn.valid_type?(:foobar)
-      end
-
       def test_column_types
         owner = Owner.create!(name: "hello".encode("ascii-8bit"))
         owner.reload
-- 
GitLab