#71 √ resolved
Jeff Dean

[patch] change_table block

Reported by Jeff Dean | April 30th, 2008 @ 09:06 PM

I created a fork that adds change_table to migrations like so: 

change_table :videos do |t|
  t.add_timestamps
  t.add_belongs_to :goat
  t.add_string :name, :email, :limit => 20
  t.remove_column :name, :email
  t.string :some_string
  t.rename :some_new_name
  t.drop
end

This brings Sexy Migrations to the change table helpers as well, in the same style as create_table.

It contains wrappers for all of the migration functions, tests and docs. Some have metioned that "rename" and "drop" are confusing, but I find the alternatives confusing. I'd be open to removing "rename" and "drop" and updating the tests and docs if that's necessary.

This also makes remove_column take an array, so that the sexy migration "t.string :name, :email" has a counterpart "remove_column(s) :name, :email" 

$ git remote add zilkey git://github.com/zilkey/rails.git
$ git checkout -b zilkey/master
$ git pull zilkey

(or however you normally pull requests)

I'd be happy to provide and upload a patch as well, but now that github exists, I can't see any reason to (pulling seems much faster and easier). But if not having a patch is a blocker, I'll upload one.

Feedback so far has been tepid, but I think this is a strong and useful addition, one that I use regularly now in production. It's a big feature to add, but it's very well tested and documented.

Comments and changes to this ticket

  • david (at loudthinking)

    david (at loudthinking) April 30th, 2008 @ 10:40 PM

    Thoughts: Make the add_ implicit, just like in create_table, so you do t.string :column to add a column. Then you can drop column from remote for just doing t.remove :column. I don't think drop and rename fit well. Otherwise, I like it!

  • david (at loudthinking)

    david (at loudthinking) April 30th, 2008 @ 11:27 PM

    Also, pulling is actually a big hassle. Please do upload a patch.

  • Jeff Dean

    Jeff Dean May 1st, 2008 @ 04:13 AM

    I agree that removing add_ makes it much clearer. However, removing those makes the naming a bit less clear to me. I've made those changes and provided 2 patches here so that you can choose the style you like the most:

    • Regular Version which omits the word "column" and looks more like create_table with:

    ** t.change

    ** t.change_default

    ** t.rename

    • Verbose Version which retains the word "column" and looks more like regular change helpers with:

    ** t.change_column

    ** t.change_column_default

    ** t.rename_column

    Each patch has appropriate passing tests and documentation and are identical except for the 3 methods mentioned above.

    The regular patch produces all of the following methods: 

    change_table :table do |t|
 t.column
 t.index
 t.timestamps
 t.change
 t.change_default
 t.rename
 t.references
 t.belongs_to
 t.string 
 t.text 
 t.integer 
 t.float 
 t.decimal 
 t.datetime 
 t.timestamp 
 t.time 
 t.date 
 t.binary 
 t.boolean
 t.remove
 t.remove_references
 t.remove_belongs_to
 t.remove_index
 t.remove_timestamps
end

    The verbose patch produces all of the following methods: 

    change_table :table do |t|
  t.column
  t.index
  t.timestamps
  t.change_column
  t.change_column_default
  t.rename_column
  t.references
  t.belongs_to
  t.string 
  t.text 
  t.integer 
  t.float 
  t.decimal 
  t.datetime 
  t.timestamp 
  t.time 
  t.date 
  t.binary 
  t.boolean
  t.remove
  t.remove_references
  t.remove_belongs_to
  t.remove_index
  t.remove_timestamps
end

    I've removed the helpers for "drop_table" and "rename_table". In both patches, "remove_column" now takes an array of field names, with updated tests.

  • Jeff Dean

    Jeff Dean May 1st, 2008 @ 04:14 AM

    (the list formatting got eaten on that last post...)

    Here's the verbose patch.

  • david (at loudthinking)

    david (at loudthinking) May 1st, 2008 @ 05:45 PM

    Getting the following failures on rake test_sqlite3 in AR after applying:

    1) Failure:

    test_integer_creates_integer_column(ChangeTableMigrationsTest)

    [(eval):1:in `add_column'

    (eval):16:in `integer'

    (eval):5:in `each'

    (eval):5:in `integer'

    ./test/cases/migration_test.rb:1253:in `test_integer_creates_integer_column'

    ./test/cases/migration_test.rb:1345:in `with_change_table'

    ./test/cases/../../lib/active_record/connection_adapters/abstract/schema_statements.rb:165:in `change_table'

    ./test/cases/migration_test.rb:1344:in `with_change_table'

    ./test/cases/migration_test.rb:1250:in `test_integer_creates_integer_column'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `__send__'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `run']:

    #.add_column(:delete_me, :foo, 'integer', {}) - expected calls: 0, actual calls: 1

    Similar expectations:

    add_column(:delete_me, :foo, 'int(11)', {})

    add_column(:delete_me, :bar, 'int(11)', {})

    2) Failure:

    test_add_remove_single_field_using_an_array_of_arguments(MigrationTest)

    [./test/cases/migration_test.rb:471:in `test_add_remove_single_field_using_an_array_of_arguments'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `__send__'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `run']:

    is not true.

    3) Failure:

    test_add_remove_single_field_using_multiple_string_arguments(MigrationTest)

    [./test/cases/migration_test.rb:423:in `test_add_remove_single_field_using_multiple_string_arguments'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `__send__'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `run']:

    is not true.

    4) Failure:

    test_add_remove_single_field_using_multiple_symbol_arguments(MigrationTest)

    [./test/cases/migration_test.rb:441:in `test_add_remove_single_field_using_multiple_symbol_arguments'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `__send__'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `run']:

    is not true.

    5) Failure:

    test_remove_columns_works_like_remove_column(MigrationTest)

    [./test/cases/migration_test.rb:477:in `test_remove_columns_works_like_remove_column'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `__send__'

    ./test/cases/../../lib/../../activesupport/lib/active_support/testing/setup_and_teardown.rb:59:in `run']:

    is not true.

  • Jeff Dean

    Jeff Dean May 1st, 2008 @ 10:58 PM

      • → Title changed from “Add change_table to core?” to “[patch] change_table block”

    Sorry to have taken your time with that - I've updated the patches to include tests that run in:

    • MySQL
    • PostgreSQL
    • SQLite3

    I don't have any of the other databases installed, so I can't vouch for those, but SQLite should work as well.

  • Repository

    Repository May 3rd, 2008 @ 11:30 AM

      • → State changed from “new” to “resolved”

    (from [96980bd561d79824b6cb6efbcbecdcbf8785d452]) Added change_table for migrations (Jeff Dean) [#71 state:resolved]

    http://github.com/rails/rails/co...

Please Login or create a free account to add a new comment.

You can update this ticket by sending an email to from your email client. (help)

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Source available from github

The Git repository resides at http://github.com/rails

Check out the current development trunk (Edge Rails) with:

git clone git://github.com/rails/rails.git

The latest development for the 1.2.x and 2.0.x releases are on the 1-2-stable and 2-0-stable branches.

Creating a bug report

When creating a bug report, be sure to include as much relevant information as possible. Post the code sample that causes the problem. Preferably, alter the unit tests and show through either changed or added tests how the expected behavior is not occuring.

Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use trac for reporting security vulnerabilities. All content in trac is publicly available as soon as it is posted.

Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kinda bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to sign on to with a "I'm having this problem too".

Shared Ticket Bins

Attachments

People watching this ticket

Tags