Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

change_column carries over default when it can't be carried

#3234

related to: http://dev.rubyonrails.org/ticket/10963

add_column :mytable, :data, :string
change_column :mytable, :data, :binary

Will fail (on MySQL anyway) with:

Mysql::Error: BLOB/TEXT column 'data' can't have a default value: ALTER TABLE `mytable` CHANGE `data` `data` blob DEFAULT '' NOT NULL

This is because the default from the string column, which defaults to '', is carried over to the new column.

Culprit:

def change_column(table_name, column_name, type, options = {}) #:nodoc:
  column = column_for(table_name, column_name)

  unless options_include_default?(options)
    options[:default] = column.default
  end

  unless options.has_key?(:null)
    options[:null] = column.null
  end

See how it takes column.default without considering the type of the new column?

Similar to has_default?

def has_default?
  return false if type == :binary || type == :text #mysql forbids defaults on blob and text columns
  super
end

It could check the type. Something like:

-  unless options_include_default?(options)
+  unless options_include_default?(options) or type == :binary or type == :text #mysql forbids defaults on blob and text
     options[:default] = column.default
   end

Reported by Greg Hazel · September 19th, 2009 @ 02:34 PM

State: open
Milestone: 2.3.10
Assigned to: Pratik Pratik
Importance: Low

Activity

  1. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from activerecord, active_record, migrate, migration, migrations, migrator, mysql to activerecord, active_record, bugmash, migrate, migration, migrations, migrator, mysql

    September 25th, 2009 @ 11:59 AM

  2. dira
    dira

    Not reproducible.

    Tested in 2.3.stable and on master, by running the migration on the dev app & also in tests. The column is changed correctly and there is no error.

    The attached file is a patch for the migration tests on stable; it shows that the error does not happen.

    September 27th, 2009 @ 10:37 AM

  3. Greg Hazel
    Greg Hazel

    What version of MySQL, and what sql_mode? I run 5.0.xx in TRADITIONAL mode. Can you see the ALTER TABLE line it generates?

    September 27th, 2009 @ 12:31 PM

  4. Elomar França
    Elomar França

    +1, verified on Mysql 5.0.81 in TRADITIONAL mode.

    I've attached a patch with a test based on the one from dira and a tiny fix. Applies on 2-3-stable and master.

    September 27th, 2009 @ 05:44 PM

  5. Elomar França
    Elomar França

    +1, verified on Mysql 5.0.81 in TRADITIONAL mode.

    I've attached a patch with a test based on the one from dira and a tiny fix. Applies on 2-3-stable and master.

    September 27th, 2009 @ 05:45 PM

  6. Prem Sichanugrist (sikachu)
    Prem Sichanugrist (sikachu)
    • State changed from new to verified
    • Milestone set to 2.3.6

    January 25th, 2010 @ 05:29 PM

  7. Prem Sichanugrist (sikachu)
  8. José Valim
  9. Prem Sichanugrist (sikachu)
  10. Rizwan Reza
    Rizwan Reza
    • Tag changed from activerecord, active_record, bugmash, migrate, migration, migrations, migrator, mysql to activerecord, active_record, migrate, migration, migrations, migrator, mysql

    February 12th, 2010 @ 12:46 PM

  11. Rizwan Reza
    Rizwan Reza
    • Tag changed from activerecord, active_record, migrate, migration, migrations, migrator, mysql to activerecord, active_record, bugmash, migrate, migration, migrations, migrator, mysql

    May 16th, 2010 @ 02:41 AM

  12. Rizwan Reza
    Rizwan Reza
    • Tag changed from activerecord, active_record, bugmash, migrate, migration, migrations, migrator, mysql to activerecord, active_record, bugmash-review, migrate, migration, migrations, migrator, mysql

    May 16th, 2010 @ 02:43 AM

  13. Repository
    Repository
    • State changed from verified to resolved

    May 16th, 2010 @ 09:45 AM

  14. Repository
  15. Rizwan Reza
    Rizwan Reza
    • Tag changed from activerecord, active_record, bugmash-review, migrate, migration, migrations, migrator, mysql to activerecord, active_record, migrate, migration, migrations, migrator, mysql

    May 16th, 2010 @ 03:06 PM

  16. Repository
    Repository
    • State changed from resolved to open

    (from [9d99e610be0da6e4b682ebb8cc410cdca1ca633a]) Revert "Don't carry default value when changing column for a binary type on MySQL"

    Broke mysql tests.

    This reverts commit ddadcc7cf8332415ba48b66904ee6136b43dda6b.

    Conflicts:

    activerecord/test/cases/migration_test.rb
    

    [#3234 change_column carries over default when it can't be carried state:open] http://github.com/rails/rails/commit/9d99e610be0da6e4b682ebb8cc410c...

    May 18th, 2010 @ 07:04 PM

  17. Repository
    Repository

    (from [223d6415d045e670610603665c21e93b06a01db7]) Revert "Don't carry default value when changing column for a binary type on MySQL"

    Broke mysql tests.

    This reverts commit edec1afe25014749f0e2df86d27477b45586a9e3.

    Conflicts:

    activerecord/test/cases/migration_test.rb
    

    [#3234 change_column carries over default when it can't be carried state:open] http://github.com/rails/rails/commit/223d6415d045e670610603665c21e9...

    May 18th, 2010 @ 07:04 PM

  18. Rizwan Reza
    Rizwan Reza
    • No changes were found…

    May 19th, 2010 @ 10:32 AM

  19. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.6 to 2.3.7

    May 23rd, 2010 @ 05:54 PM

  20. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.7 to 2.3.8

    May 24th, 2010 @ 09:40 AM

  21. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.8 to 2.3.9

    May 25th, 2010 @ 11:45 PM

  22. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.3.9 to 2.3.10
    • Importance changed from to Low

    August 30th, 2010 @ 02:28 AM

  23. Santiago Pastorino
    Santiago Pastorino

    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.

    February 2nd, 2011 @ 04:32 PM

  24. Santiago Pastorino
    Santiago Pastorino
    • State changed from open to stale

    February 2nd, 2011 @ 04:32 PM

  25. Greg Hazel
    Greg Hazel
    • State changed from stale to open

    [state:open]

    February 2nd, 2011 @ 07:23 PM

  26. Santiago Pastorino
    Santiago Pastorino

    Greg, glad you reopen the issue, can you make a patch that doesn't break mysql tests, for master, 3-0-stable and 2-3-stable.
    Thanks a lot :).

    February 2nd, 2011 @ 08:27 PM

  27. rails
    rails

    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.

    May 3rd, 2011 @ 01:00 AM

  28. rails
    rails
    • State changed from open to stale

    May 3rd, 2011 @ 01:00 AM

  29. Greg Hazel
    Greg Hazel
    • State changed from stale to open

    Oh no, not this again.

    [state:open]

    I would be happy to fix the patch, but I don't see how it failed. It just said "Broke mysql tests.". That's not very helpful.

    May 3rd, 2011 @ 01:06 AM