:limit for primary key columns

Thinking about it, I suppose the primary key could always be given explicitly in the create_table block. That would make the underlying code a bit simpler. That doesn't solve the problem though.

Sorry, one more fix. Changed the default :limit from 11 to 4, otherwise it'll add :limit in the schema when it doesn't need to since 4 is returned by int, not 11.

change_column ignores the limit option for primary keys so that doesn't work.

Although the patch looks a little big, it actually simplifies the existing code by making primary keys less of a special case. I think this is ideally what you want.

The change in NATIVE_DATABASE_TYPES isn't really a change. Remember that the byte sizes in MySQL get translated to string lengths by the type_to_sql function. 4 is the equivalent byte size for a string length of 11.

I used this patch a lot in a previous project and never had a single problem with it.

Actually, to expand on what I just said about change_column not working, it's not merely a case of allowing the limit option to take effect because output from the schema dumper still doesn't reflect the new size.

Alright. So this patch should probably make change_column work too, if it's not too much work.

Also, I'm not yet so sure about changing schema dumper to always have :id => false. I wonder if it's possible to do that only if the supplied :limit, :name are different from the default

But the rest looks good.

Thanks !

The patch already fixes change_column. There is still a " type != :primary_key" condition in the base definition of type_to_sql but the MySQL definition deals with primary keys before this line is reached. The condition should be left there for the other databases because I don't think they allow you to set the size of the primary key like MySQL does.

Preventing :id => false except when necessary makes it a little messier but very similar to how it was originally so it's no big deal. Here's an updated patch with that included against current edge.

Just realised the call to "compact" in the schema dumper can now be removed.

Sorry but I'm concerned that this will get overlooked unless it is reopened.

This adds the behavior and now passes with all tests.

(from [0cb3311d06c02649fb7444c34b6fdf2214ab85f5]) Revert "primary_key now supports :limit. [#876 state:resolved]" since it broke AR test suite.

This reverts commit 41e5c7ed44fedb95636ef9b7a792c46ea03309bd.
http://github.com/rails/rails/commit/0cb3311d06c02649fb7444c34b6fdf...

This is the failing test:

1) Failure: test_mysql_schema_dump_should_honor_primary_keys_limits(SchemaDumperTest)

[./test/cases/schema_dumper_test.rb:176:in `test_mysql_schema_dump_should_honor_primary_keys_limits'
 /Users/jose/Work/github/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:64:in `__send__'
 /Users/jose/Work/github/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:64:in `run'
 /Users/jose/Work/github/rails/activesupport/lib/active_support/callbacks.rb:412:in `_run_setup_callbacks'
 /Users/jose/Work/github/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:62:in `run']:

limit option not found on primary key.
<"|t|\n t.string "nick", :null => false\n t.string "name"\n "> expected to be =~
</t.primary_key\s+"id",\s+:limit => \d+$/>.

Strange, I tested this and it was passing. :(

Are you on Ruby 1.9?

José, can you test this again please? You might need to recreate databases. Thanks.

The error appears when running the test suite with rake test_sqlite3. I recreated sqlite database, still get it.

Thanks, fix coming. :)

The tests now pass for the whole ActiveRecord suite. Thanks for the mention, José. :-)

Needless to say, this also fixes the parsing error on 1.9.

Sorry, I meant that will always be TRUE now.

Please also check comments here:

http://github.com/rails/rails/commit/41e5c7ed44fedb95636ef9b7a792c4...

That error was fixed in the second commit. I am still unsure what is going on. Will dive through a Sam Ruby app to test it. Thanks José and sorry for the troubles. :D

verified when cherry-picking the original commit to the current HEAD

there is an error when running the tests with mysql and fixtures and using a limit below 4. The reason: the automatically calculated ids for the fixtures which are usually above the range of limits between 1-3 (maximum of 8388607).

Is this patch intended to be used for lowering the limit? My use-case would probably always be increasing the limit to allow for bigger ids. In this case we could just disallow the use of limits below 4. If one would want limits below 4 we would have to adjust the automatically generated number for ids.

I might look at it again once I move up to Rails 3. I don't use MySQL anymore but I don't want to see the effort go to waste. To answer Michael's question, it was to increase the limit.