[PATCH] Rails should quote table names properly (revised patch)

Is there ever an instance where table names *shouldn't* have backticks?

It seems like this should be an easy fix...

Added backticks to all table references in the mysql adapter

I patched the adapter by adding backticks around the table name in each place.

Existing tests pass. I feel like we should have a test for "problematic" table names, but didn't implement it.

I'm using the latest stable version of rails, and though I have not tried it out with table names, I run into the same problem with column names. It does not hurt to put backticks around table names or column names regardless. I would suggest always putting backticks around the table names and column names.

This problem is actually three-fold. When you have a table whose name is a reserved word in MySQL (ie: 'group', 'order', 'values', etc...):

1: Migrations don't work. You also can't dump schema from an existing database that has any such tables. As far as I know, there is no workaround for this problem. This is mildly annoying in some cases, particularly when working with legacy databases.

2: Fixtures don't work. This is the biggest nuisance because my legacy database is essentially useless with fixtures, and by extension a lot of the test case niceness in rails is not available to me.

3: ActiveRecord model classes fail in numerous places. I have heard that simply doing "set_table_name 'whatever'" works around this problem, but I haven't tested it thoroughly. Since there is a workaround, this isn't a super-critical problem, but it is very possible that this workaround will break some day down the road, so it would be better for activerecord to simply work.

I have produced test cases for all three problems. I have also patched activerecord to fix the problem.

I have a few concerns I'd like feedback on:

First, the fix to migrations was relatively simple, and only required changes to the MySQL adapter. I see no reason this fix shouldn't make it into the trunk once it's been certified.

But the fix for fixtures and model classes is much deeper. I had to modify base.rb, fixtures.rb, abstract_adapter.rb, and database_statements.rb. Conceptually, I introduced a new method, AbstractAdapter#quote_table_name that returns the table name properly quoted. Every time activerecord generates SQL generically (in base.rb and fixtures.rb), it now runs the table name through this quoting process. This is similar in concept to how column names are quoted. The abstract implementation of quote_table_name simply returns the table name unchanged, so it *should* be transparent to third party adapters. Only the mysql adapter overrides this method to add table name escaping.

Since this change is deep, I think it needs to be looked at closely by someone with more knowledge of activerecord. In the worst case, I can produce a patch that only fixes Migrations and hopefully get that on the trunk sooner than later. We could then revisit the other fixes separately.

Also, this patch could affect all databases, but I have only run the mysql test suite. I don't have access to most of the additional databases to test. So the full suite of tests needs to be run and certified by someone.

PS: If this patch goes in, I think we may be able to add the same set of fixes for Microsoft SQL Server. I suspect it has the same problems, and to get it into compliance, the adapter probably just needs to override quote_table_name to return "#{table_name}". Other databases may similarly need fixes, which would all benefit from the infrastructure change.

A more comprehensive patch to fix this bug, including test cases (the above patch is not valid)

on a side note, does any AR expert know why AbstractAdapter doesn't declare quote_column_name? This method is used on connections throughout base.rb, but the method is implemented on the individual adapters and not on the abstract base class. This seems wrong to me. If it is, I'd be glad to fix.

duplicate of #4593, I enhanced that patch instead.