Rails - Mar 2009 - ActiveRecord::Migrator, method get_all_versions & hard-coded SQL

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


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

On Mar 15, 5:30 am, Mathias
<mathiasburb...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> 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?
>
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

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Hello Fred,

Thanks for your reply.

On Mar 15, 10:25 pm, Frederick Cheung
<frederick.che...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:
> 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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---