ActiveRecord::Migrator, method get_all_versions & hard-coded SQL

rubyonrails-talk
1

Hello Folks!

I only recently started to use Ruby on Rails coming from Delphi, which is why I am quite happy with using InterBase as my database. 3rdRail does come with an InterBase adapter but I could not run migrations with Rails 2.1 and 3rdRail 2.0 against InterBase 2009. The error occurred in ActiveRecord::Migrator when the get_all_versions method was executed. It has some hard-coded SQL: Base.connection.select_values("SELECT version FROM # {schema_migrations_table_name}").map(&:to_i).sort which caused a problem given that version is a reserved word in InterBase. I fixed the problem by freezing the rails 2.1 classes and introduced the following patch: version_column = Base.connection.quote_column_name("version") Base.connection.select_values("SELECT #{version_column} FROM # {schema_migrations_table_name}").map(&:to_i).sort With this change the base connection can check for reserved words and will wrap the name "version" into double quotes. The same problem resurfaces in the private method record_version_state_after_migrating of Migrator. It has some hard coded SQL for deleting or inserting a version string into the schema_migrations table. Here I had to ask the connection class for its version column name too. Whilst this is now working (e.g. I can migrate) I don't like this fix very much. As far as I understand the Migrator class shouldn't have any DB- specific knowledge. Wouldn't it be better to have a method on the connection class to ask for all versions applied so far? And shouldn't the DB connection class be responsible for inserting or deleting a version schema_migrations table? I know this will cause that a whole bunch of adapters will have to support those new methods. But from an pure OO perspective I think this would be a better way to do migrations. What do I need to do to submit such a change to the Rails core team for consideration?

Thanks for a short answer in advance.

Salut,   Mathias

2

You might want to have a look at http://afreshcup.com/2008/10/27/contributing-to-rails-step-by-step/

The only change I might make to that is where step 11 goes - depending on the change you are making it may be advisable to gather some opinions earlier on rather than after you've written a patch.

Fred

3

you can post your suggestion on the rails-core group:   http://groups.google.com/group/rubyonrails-core

or you could go and create an official bug-report and suggest your patch to solve it:    http://rails.lighthouseapp.com/projects/8994-ruby-on-rails

4

Hello Fred,

Thanks for your reply.

The only change I might make to that is where step 11 goes - depending on the change you are making it may be advisable to gather some opinions earlier on rather than after you've written a patch.

I am not too proud of this patch, Fred. I think all the SQL code should move into the DB connection class. Rather than submitting a patch, which I feel could be done better, I'd like to get some more comments from the core team (e.g. Is the connection class really the best place? or How to minimise the effect of my intended change on all the DB adapters out there?). Only then I would feel comfortable to submit anything.

I might follow MaD's advise - thanks for that MaD - and send another message to the rails-core group first and see what the experts have to say.

Salut,   Mathias