Merge pull request #18916 from kamipo/handle_array_option_in_type_to_sql

Handle array option in `type_to_sql`

activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb

@@ -18,6 +18,9 @@ def visit_AddColumn(o)
           "ADD #{accept(o)}"
         end
 
+        delegate :quote_column_name, :quote_table_name, :quote_default_expression, :type_to_sql, to: :@conn
+        private :quote_column_name, :quote_table_name, :quote_default_expression, :type_to_sql
+
         private
 
           def visit_AlterTable(o)
@@ -70,18 +73,6 @@ def column_options(o)
             column_options
           end
 
-          def quote_column_name(name)
-            @conn.quote_column_name name
-          end
-
-          def quote_table_name(name)
-            @conn.quote_table_name name
-          end
-
-          def type_to_sql(type, limit, precision, scale)
-            @conn.type_to_sql type.to_sym, limit, precision, scale
-          end
-
           def add_column_options!(sql, options)
             sql << " DEFAULT #{quote_default_expression(options[:default], options[:column])}" if options_include_default?(options)
             # must explicitly check for :null to allow change_column to work on migrations
@@ -97,10 +88,6 @@ def add_column_options!(sql, options)
             sql
           end
 
-          def quote_default_expression(value, column)
-            @conn.quote_default_expression(value, column)
-          end
-
           def options_include_default?(options)
             options.include?(:default) && !(options[:null] == false && options[:default].nil?)
           end

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

@@ -5,8 +5,7 @@ class SchemaCreation < AbstractAdapter::SchemaCreation
         private
 
         def visit_ColumnDefinition(o)
-          o.sql_type = type_to_sql(o.type, o.limit, o.precision, o.scale)
-          o.sql_type << '[]' if o.array
+          o.sql_type = type_to_sql(o.type, o.limit, o.precision, o.scale, o.array)
           super
         end
       end
@@ -407,12 +406,14 @@ def add_column(table_name, column_name, type, options = {})
         def change_column(table_name, column_name, type, options = {})
           clear_cache!
           quoted_table_name = quote_table_name(table_name)
-          sql_type = type_to_sql(type, options[:limit], options[:precision], options[:scale])
-          sql_type << "[]" if options[:array]
-          sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{sql_type}"
-          sql << " USING #{options[:using]}" if options[:using]
-          if options[:cast_as]
-            sql << " USING CAST(#{quote_column_name(column_name)} AS #{type_to_sql(options[:cast_as], options[:limit], options[:precision], options[:scale])})"
+          quoted_column_name = quote_column_name(column_name)
+          sql_type = type_to_sql(type, options[:limit], options[:precision], options[:scale], options[:array])
+          sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}"
+          if options[:using]
+            sql << " USING #{options[:using]}"
+          elsif options[:cast_as]
+            cast_as_type = type_to_sql(options[:cast_as], options[:limit], options[:precision], options[:scale], options[:array])
+            sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})"
           end
           execute sql
 
@@ -509,8 +510,8 @@ def index_name_length
         end
 
         # Maps logical Rails types to PostgreSQL-specific data types.
-        def type_to_sql(type, limit = nil, precision = nil, scale = nil)
-          case type.to_s
+        def type_to_sql(type, limit = nil, precision = nil, scale = nil, array = nil)
+          sql = case type.to_s
           when 'binary'
             # PostgreSQL doesn't support limits on binary (bytea) columns.
             # The hard limit is 1Gb, because of a 32-bit size field, and TOAST.
@@ -526,24 +527,24 @@ def type_to_sql(type, limit = nil, precision = nil, scale = nil)
             else raise(ActiveRecordError, "The limit on text can be at most 1GB - 1byte.")
             end
           when 'integer'
-            return 'integer' unless limit
-
             case limit
-              when 1, 2; 'smallint'
-              when 3, 4; 'integer'
-              when 5..8; 'bigint'
-              else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with precision 0 instead.")
+            when 1, 2; 'smallint'
+            when nil, 3, 4; 'integer'
+            when 5..8; 'bigint'
+            else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with precision 0 instead.")
             end
           when 'datetime'
-            return super unless precision
-
             case precision
-              when 0..6; "timestamp(#{precision})"
-              else raise(ActiveRecordError, "No timestamp type has precision of #{precision}. The allowed range of precision is from 0 to 6")
+            when nil; super(type, limit, precision, scale)
+            when 0..6; "timestamp(#{precision})"
+            else raise(ActiveRecordError, "No timestamp type has precision of #{precision}. The allowed range of precision is from 0 to 6")
             end
           else
-            super
+            super(type, limit, precision, scale)
           end
+
+          sql << '[]' if array && type != :primary_key
+          sql
         end
 
         # PostgreSQL requires the ORDER BY columns in the select list for distinct queries, and

activerecord/test/cases/adapters/postgresql/change_schema_test.rb

@@ -26,6 +26,13 @@ def test_change_type_with_symbol
         connection.change_column :strings, :somedate, :timestamp, cast_as: :timestamp
         assert_equal :datetime, connection.columns(:strings).find { |c| c.name == 'somedate' }.type
       end
+
+      def test_change_type_with_array
+        connection.change_column :strings, :somedate, :timestamp, array: true, cast_as: :timestamp
+        column = connection.columns(:strings).find { |c| c.name == 'somedate' }
+        assert_equal :datetime, column.type
+        assert column.array?
+      end
     end
   end
 end