Fix bug related to migrating custom types derived from builtin types #31
Conversation
When dm-migrations' dm-do-adapter runs, Adapter#property_schema_hash is invoked
on each property to generate the SQL for it.
For Property::Text, type_map[Property::Text] yields a schema of TEXT with no
:length property. When DM encounters a String primitive whose length exceeds
the schema's capacity, it auto-adjusts the schema primitive to compensate
(i.e. in MySQL, {SHORT,MEDIUM,LONG}TEXT). Result: MEDIUMTEXT == AWESOME.
The case is different for (1) a custom Property derived from (2) a builtin
Property whose schema primitive changes based on the Property's size options.
For Property::Json, the first type_map[property.class] lookup is nil because
custom types can't/don't update Adapter#type_map -- custom properties can't know
what model/repository/adapter they're going to be on at definition time, which
they would need because the type_map is stored on the adapter *class*.
So, the second lookup type_map[property.primitive] kicks in, which for
Property::Json is type_map[String]. That in turn yields a schema of VARCHAR
with a :length property. As with Property::Text, when DM encounters a String
primitive whose length exceeds the schema's capacity, it auto-adjusts the schema
primitive to compensate (i.e. in MySQL, {SHORT,MEDIUM,LONG}TEXT). However, when
dm-migrations encounters any property_schema_hash with a :length option, it
automatically appends "(%i)" % length to the SQL statement. Result:
MEDIUMTEXT(123412341234) == entire migration FKD.
|
@jpr5 what if you just change this condition: https://github.com/datamapper/dm-migrations/blob/master/lib/dm-migrations/adapters/dm-do-adapter.rb#L206 so that it always use property.length if a property responds_to :length? that should allow you to define a property like this |
|
No, that won't work -- that is precisely the problem I'm talking about. Because of how dm-core+dm-migrations works, |
Conflicts:
lib/dm-migrations/adapters/dm-do-adapter.rb
Resolved by taking master and re-adding property.class.superclass
lookup.
|
@jpr5 I merged in your patch into release-1.2 branch (which is now the maintenance branch for the 1.2.x series) and it's going to be included in the first 1.2.0 final release. Also, thanks for rebasing it on top of latest master, it's merged in there too. |
When dm-migrations' dm-do-adapter runs, Adapter#property_schema_hash is invoked
on each property to generate the SQL for it.
For Property::Text, type_map[Property::Text] yields a schema of TEXT with no
:length property. When DM encounters a String primitive whose length exceeds
the schema's capacity, it auto-adjusts the schema primitive to compensate
(i.e. in MySQL, {SHORT,MEDIUM,LONG}TEXT). Result: MEDIUMTEXT == AWESOME.
The case is different for (1) a custom Property derived from (2) a builtin
Property whose schema primitive changes based on the Property's size options.
For Property::Json, the first type_map[property.class] lookup is nil because
custom types can't/don't update Adapter#type_map -- custom properties can't know
what model/repository/adapter they're going to be on at definition time, which
they would need because the type_map is stored on the adapter class.
So, the second lookup type_map[property.primitive] kicks in, which for
Property::Json is type_map[String]. That in turn yields a schema of VARCHAR
with a :length property. As with Property::Text, when DM encounters a String
primitive whose length exceeds the schema's capacity, it auto-adjusts the schema
primitive to compensate (i.e. in MySQL, {SHORT,MEDIUM,LONG}TEXT). However, when
dm-migrations encounters any property_schema_hash with a :length option, it
automatically appends "(%i)" % length to the SQL statement. Result:
MEDIUMTEXT(123412341234) == entire migration FKD.
Note there's a pending pull request on datamapper/dm-types for this change (datamapper/dm-types#45).