[Patch] ActiveRecord 2.1.1 breaks formerly legit postgres migrations

I'll further observe that the ALTER COLUMN needs a USING clause to work correctly in this use case, ala:

execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])} USING CAST(#{quote_column_name(column_name)} AS #{type_to_sql(type, options[:limit], options[:precision], options[:scale])})"

Note that the CAST() is identical to the one used in the PostgreSQL 7.x fallback code.

@John: have you submitted a patch? This is a problem for me, too.

In addition to the change mentioned by John, the change_column method needs to drop the default value of a column prior to changing it.

This is so that you don't get an error when the default value of a column is incompatible with the column type you are changing it to.

It appears that it's still broken in edge. I'd suggest looking at a diff between 2.1 and 2.1.1 for a hint about where to start, since that's where it broke.

I have recreated it with he gem: postgres (0.7.9.2008.01.28) and have PostgreSQL 8.3.6 installed on ubuntu 8.04.

I have uploaded the AR/test/cases/migration_test.rb to illustrate the problem migrating from a string column to an integer. Below is the diff of the test.

diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 16861f2..07197ee 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -712,6 +712,19 @@ if ActiveRecord::Base.connection.supports_migrations?

   assert_nothing_raised { Topic.connection.change_column :topics, :approved, :boolean, :default => true }
 end

• def test_change_column_requires_type_cast
• Person.connection.add_column 'people', 'age', :string
• old_columns = Person.connection.columns(Person.table_name, "#{name} Columns")
• assert old_columns.find { |c| c.name == 'age' and c.type == :string } +
• assert_nothing_raised { Person.connection.change_column "people", "age", :integer } +
• new_columns = Person.connection.columns(Person.table_name, "#{name} Columns")
• assert_nil new_columns.find { |c| c.name == 'age' and c.type == :string }
• assert new_columns.find { |c| c.name == 'age' and c.type == :integer }
• end + + def test_change_column_with_nil_default Person.connection.add_column "people", "contributor", :boolean, :default => true Person.reset_column_information

Here is the result, also attached(tests.log):

23) Failure: test_change_column_requires_type_cast(MigrationTest)

[./test/cases/migration_test.rb:720:in `test_change_column_requires_type_cast'
 ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in `__send__'
 ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in `run']:

Exception raised: Class: <ActiveRecord::StatementInvalid> Message: <"PGError: ERROR: column "age" cannot be cast to type "pg_catalog.int4"\n: ALTER TABLE "people" ALTER COLUMN "age" TYPE integer"> ---Backtrace--- ./test/cases/../../lib/active_record/connection_adapters/abstract_adapter.rb:212:in log' ./test/cases/../../lib/active_record/connection_adapters/postgresql_adapter.rb:507:inexecute_without_query_record' ./test/cases/helper.rb:37:in execute' ./test/cases/../../lib/active_record/connection_adapters/postgresql_adapter.rb:802:inchange_column' ./test/cases/migration_test.rb:720:in test_change_column_requires_type_cast' ./test/cases/migration_test.rb:720:intest_change_column_requires_type_cast' ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in __send__' ./test/cases/../../../activesupport/lib/active_support/testing/setup_and_teardown.rb:62:inrun'

The problem is in the change_column method, the execute fails and the raise e line (804) raises the exception seen above. Apparently the alter table works in earlier versions of postgres but not in 8.3+

    begin
      execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
    rescue ActiveRecord::StatementInvalid => e
      raise e if postgresql_version > 80000
      # This is PostgreSQL 7.x, so we have to use a more arcane way of doing it.
      begin
        begin_db_transaction
        tmp_column_name = "#{column_name}_ar_tmp"
        add_column(table_name, tmp_column_name, type, options)
        execute "UPDATE #{quoted_table_name} SET #{quote_column_name(tmp_column_name)} = CAST(#{quote_column_name(column_name)} AS #{type_to_sql(type, options[:limit], options[:precision], options[:scale])})"
        remove_column(table_name, column_name)
        rename_column(table_name, tmp_column_name, column_name)
        commit_db_transaction
      rescue
        rollback_db_transaction
      end
    end

Sorry, the unit test did not get attached... let's try that again.

Looks like the diff did not print properly... see the attached migration_test.rb, method: test_change_column_requires_type_cast

I updated the one in test/cases/migration_test.rb and ran it from rails/activerecord from edge rails. "rake test_postgresql"

Joe, unfortunately I don't currently have enough free time to try to solve this, but if you'd like to, I can point you in the right direction. What is needed here is for the "ALTER COLUMN" statement to include a "USING" argument that explicitly specifies the type conversion that (in previous versions of postgresql) happened manually. For example, for 'change_column "people", "age", :integer' it should be 'USING age::integer'.

To make the unit test pass, attached is a simple patch based on John Whitley's suggestion of "USING CAST" which was introduced in 8.0. This was needed because 8.x seems to complain about implicit type casts. Unfortunately, I don't have a pre-8.0 database to verify this patch works for it.

The patch looks good, only thing I'm unsure about is if the casting should be enabled for postgresql versions older than 8.3 as those really don't have a problem with implicit casts.

Good point... maybe others know when exactly Postgres started complaining about implicit type conversions. Currently, I'm adding the USING CAST for 8.0 and higher since this link indicates this syntax was added in 8.0:

http://www.postgresonline.com/jo...?/archives/29-How-to-convert-a-table-column-to-another-data-type.html

Feel free to error on the side of caution and use 8.3 since I can verify it certainly complains if there is no explicit type conversion for the source and destination types.

Submitting a "using_cast_bug.diff" typo fix on top of prior patch. In addition, I ran into a PG 8.2.6 that complained about implicit type casts so I am using USING CAST for 80206 and above.

I've ran in to a bug where my schema states:

t.text "description", :limit => 255

When trying to use the schema on a PG 8.3 database, I get:

PGError: ERROR: type modifier is not allowed for type "text"

The limit comes from the fact that this column used to be of type string.

Is there anything else needed for this ticket? The supplied patch with the follow-on patch seem to fix this for me. I'm hoping to get this moved upstream to avoid monkey patching my local version.

It would be great if you could apply this patch. I ran into this today.

Any chance you can do the changeon the 2.3 branch as well?

+1, apply this patch.

This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.