This is what's wrong

I must say, I got some really good responses to my post about what was wrong with the code I posted. Most of those responses concerned the design of the code, and I agree, this part of Rails could have been done much better. But what I was thinking about was actually a bug. And Lars Westegren (my former colleague) nailed it at the first try. Let me show two important excerpts from this code:

column_type_sql = native.is_a?(Hash) ? native[:name] : native

and here:

column_type_sql << "(#{limit})" if limit

Obviously, double left arrow is append, and for all cases where there is a limit, this append will change the String. This is one of the cases where it's kind of annoying that strings are mutable. If I cache away the values that native_database_types should return, then the next time anyone wants a string SQL type, that will generate VARCHAR(255)(255). The next time again, VARCHAR(255)(255)(255). And so one. So either I need to recreate the hash every time, or I need to do a deep clone of it every time. Neither of these options are very good, and it seems the deep clone option isn't fast enough, even when done in Java, so I decided to go with a hash literal instead. Was that the right choice? I don't know. It improves performance, but on the other hand it churns objects and creates new objects all the time. All of this because of some sloppy coding in Rails.

What's the lesson learned? Never modify your arguments, unless that is an explicit part of the contract for that method and part of the documentation.