Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7424 (closed defect: fixed)

Opened 2 years ago

Last modified 7 months ago

[PATCH] mysql schema dump doesn't decorate BLOB and TEXT fields with length limit option

Reported by: will.bryant Assigned to: core
Priority: normal Milestone: 2.x
Component: ActiveRecord Version: edge
Severity: normal Keywords: mysql blob longblob text longtext limit mediumtext verified
Cc: runako@onepercentsoftware.com, lantzt@businesslogic.com, gohanlon@gmail.com, yanowitz-railstrac-ticket7424@visiblecertainty.com

Description

The current mysql schema dumper maps all the mysql BLOB column types (blob, tinyblob, mediumblob, and largeblob) to an undecorated :binary column, and similarly all TEXT columns to an undecorated :text column.

Thus, even if you specify a bigger column, say using a :limit on a :binary column in a migration, when the database is dumped to schema and recreated, the recreated column will end up being a 'normal' mysql blob or text column, not the size that you specified. I hit this one all the time as all my test cases that need to store more than 64k into a binary field fail, since the test DB always ends up with these smaller columns.

Personally, I'd rather we just mapped :binary to longblob (rather than blob) in the first place - I can't see why anyone would mind the extra 2 bytes of storage space per BLOB, and longblob would match what happens with the connection adapters for other databases also. Plus, I think defaulting to a 64k column is a bit of an accident waiting to happen.

But David's response to ticket #4164 makes me think that he's cool with the current situation, so I've prepared this patch instead, which adds the :limit corresponding to the source column.

Cheers, Will

Attachments

decorate_mysql_blob_and_text_schema_dumps.diff (1.1 kB) - added by will.bryant on 01/29/07 11:25:26.
proposed patch
test_mysql_schema_dump_blob_column_lengths.diff (2.0 kB) - added by will.bryant on 02/21/08 04:42:52.
currently-failing test case (updated to apply to r8918)

Change History

01/29/07 11:25:26 changed by will.bryant

  • attachment decorate_mysql_blob_and_text_schema_dumps.diff added.

proposed patch

03/07/07 08:17:05 changed by runakog

  • cc set to runako@onepercentsoftware.com.

03/07/07 08:52:43 changed by runakog

I have testcases that fail on Rails 1.2.2 (AR 1.15.2) due to the binary column length issue described here. (This does not affect us in development or production modes at this time.) Applying this patch fixes the problem and allows us to test our mediumblob and longblob columns.

Thanks Will!

07/09/07 15:10:17 changed by tlantz

  • cc changed from runako@onepercentsoftware.com to runako@onepercentsoftware.com, lantzt@businesslogic.com.

09/14/07 00:44:04 changed by gohanlon

  • cc changed from runako@onepercentsoftware.com, lantzt@businesslogic.com to runako@onepercentsoftware.com, lantzt@businesslogic.com, gohanlon@gmail.com.

11/01/07 13:41:51 changed by yanowitz

  • cc changed from runako@onepercentsoftware.com, lantzt@businesslogic.com, gohanlon@gmail.com to runako@onepercentsoftware.com, lantzt@businesslogic.com, gohanlon@gmail.com, yanowitz-railstrac-ticket7424@visiblecertainty.com.

(follow-up: ↓ 8 ) 11/02/07 09:57:58 changed by will.bryant

Ppl who are adding CCs: if you're following this ticket because you want to see it merged, please consider doing the +1 thing so it shows up on Report 12 (http://dev.rubyonrails.org/report/12).

11/02/07 12:54:36 changed by yanowitz

  • keywords changed from mysql blob longblob text longtext limit to mysql blob longblob text longtext limit mediumtext.

+1 solves the problem without side-effects for me

(I wrapped it in a AR:CA:MysqlColmn decl and dropped it into config/initializers)

The optimal solution would be to make ActiveRecord aware of more column types and preserve the real limit, but this is way better than nothing. At least with this, my code can work.

Thanks!

(in reply to: ↑ 6 ) 11/02/07 16:52:48 changed by gohanlon

+1 (solves the problem without side-effects for me too)

Replying to will.bryant:

Ppl who are adding CCs: if you're following this ticket because you want to see it merged, please consider doing the +1 thing so it shows up on Report 12 (http://dev.rubyonrails.org/report/12).

11/02/07 23:06:37 changed by dgtized

+1 solves the problem without side-effects for me

it's fine to have the limited column types, but if you can specify limit then it's broken that it get's lost in a schema dump.

It might make sense for a more general patch to export the limit if it is set for any field.

Also for more adapters then mysql

02/21/08 04:42:52 changed by will.bryant

  • attachment test_mysql_schema_dump_blob_column_lengths.diff added.

currently-failing test case (updated to apply to r8918)

02/21/08 04:45:37 changed by will.bryant

  • keywords changed from mysql blob longblob text longtext limit mediumtext to mysql blob longblob text longtext limit mediumtext verified.

I've updated the patch to tests to match r8918. The existing patch to lib applies fine (offset 11 lines) and still works. Let's get this merged in...

Cheers, Will

03/02/08 04:42:13 changed by nzkoz

  • status changed from new to closed.
  • resolution set to fixed.

(In [8969]) Make the mysql schema dumper roundtrip the limits of text/blob columns. Closes #7424 [will.bryant]