Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  John Whitley September 12th, 2008 @ 11:01 PM

  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.

• You flagged this item as spam.
  

  Michael Hartl October 16th, 2008 @ 04:17 AM

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

• You flagged this item as spam.
  

  Blim December 6th, 2008 @ 03:01 AM

  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.

• 

  Pratik March 8th, 2009 @ 05:19 PM

  • Assigned user set to “Tarmo Tänav”
  • State changed from “new” to “incomplete”

  Is this still an issue ? Will reopen once we have a fix or a failing test.

  http://guides.rails.info/contrib... might come handy if you wish to submit a failing test case :)

• You flagged this item as spam.
  

  Michael Hartl March 12th, 2009 @ 10:48 PM

  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.

• You flagged this item as spam.
  

  Joe Rafaniello April 3rd, 2009 @ 05:32 PM

  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
  
  

  • tests.log 391 KB

• You flagged this item as spam.
  

  Joe Rafaniello April 3rd, 2009 @ 05:33 PM

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

  • migration_test.rb 57.3 KB

• You flagged this item as spam.
  

  Joe Rafaniello April 3rd, 2009 @ 05:50 PM

  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"

• You flagged this item as spam.
  

  Joe Rafaniello April 6th, 2009 @ 07:16 PM

  • Title changed from “ActiveRecord 2.1.1 breaks formerly legit postgres migrations” to “[Patch] ActiveRecord 2.1.1 breaks formerly legit postgres migrations”

  Attached is the unit test diff which fails on Edge rails with 8.3.6 PG

  • 0001-Added-test-case-showing-change_column-from-string-t.diff 1.6 KB
• 

  Tarmo Tänav April 6th, 2009 @ 07:25 PM

  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'.

• You flagged this item as spam.
  

  Joe Rafaniello April 6th, 2009 @ 07:27 PM

  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.

  • 0004-Adapt-change_column-to-use-USING-CAST-syntax-for-Pos.diff 1.9 KB
• 

  Tarmo Tänav April 6th, 2009 @ 07:37 PM

  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.

• You flagged this item as spam.
  

  Joe Rafaniello April 6th, 2009 @ 08:06 PM

  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.

• 

  Tarmo Tänav April 6th, 2009 @ 08:21 PM

  • State changed from “incomplete” to “open”

  Implicit casts were first removed in 8.3, theoretically there are a few more issues in Rails due to this. You no longer can compare a text column with an integer value, though you can compare a integer column with a text value, so thankfully it's only the less common case that no longer works (basically you can't do "User.find_by_first_name(1)" and there is no activerecord-level solution either as conditions can be built in many ways and in many cases the column type information may not be readily available).

• You flagged this item as spam.
  

  Joe Rafaniello April 24th, 2009 @ 09:23 PM

  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.

  • using_cast_bug.diff 2.6 KB

• You flagged this item as spam.
  

  felipekk December 29th, 2009 @ 11:48 PM

  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.

• You flagged this item as spam.
  

  Joe Rafaniello January 27th, 2010 @ 04:24 PM

  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.

• 

  Jeremy Kemper May 4th, 2010 @ 06:48 PM

  • Milestone changed from “2.x” to “3.x”

  [bulk edit]

• You flagged this item as spam.
  

  Łukasz Strzałkowski November 17th, 2010 @ 09:46 AM

  • Importance changed from “” to “”

  Why this ticket is still not resolved. It was posted 2 years ago (!) and have working patch.

  As a Postgres user I've run into this issue few times in serveral projects :(

• You flagged this item as spam.
  

  Uwe Kubosch November 20th, 2010 @ 11:30 PM

  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?

• You flagged this item as spam.
  

  Harold Giménez December 1st, 2010 @ 01:44 AM

  +1, apply this patch.

• 

  rails April 14th, 2011 @ 01:00 AM

  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.

• 

  rails April 14th, 2011 @ 01:00 AM

  • State changed from “open” to “stale”