Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #10698 (closed defect: fixed)

Opened 1 year ago

Last modified 9 months ago

[PATCH] Insufficient table name quoting

Reported by: dimdenis Assigned to: core
Priority: high Milestone: 2.1
Component: ActiveRecord Version: edge
Severity: major Keywords: table_name qouted_table_name quoting
Cc:

Description

Table names are not properly quoted in many parts of ActiveRecord. This causes several problems when working with legacy databases (using non-rails standard naming conventions).

Example:

class Something < ActiveRecord::Base
  set_table_name 'Legacy Database'
  validates_presence_of :name
end

i = Something.new(:name => 'Something')
i.valid? # => Exception (table name not quoted in where clause): SELECT * FROM `Legacy Database` WHERE (Legacy Database.value = 1000)  LIMIT 1

Something.update_all(:name => 'Something') # => Exception: UPDATE Legacy Database SET name = 'Something'

One aspect of this problem is covered in #10599, but there are seem to be many more (especially in the Associations component).

I put together a patch which should fix the known problems (but more might come up).

Dimitrij

Attachments

proper_table_name_quoting_patch.diff (14.7 kB) - added by dimdenis on 01/04/08 14:53:49.
10698.diff (15.4 kB) - added by lotswholetime on 01/04/08 19:17:03.
Updated patch (quoted a missed table name, adds missing files)
10698.2.diff (20.3 kB) - added by lotswholetime on 01/05/08 01:16:35.
Updated patch to fix errors against sqlite3
10698.2.2.diff (20.1 kB) - added by lotswholetime on 01/05/08 01:17:46.
8651.diff (0.8 kB) - added by garru on 01/17/08 21:00:06.
removed double declaration
8651.2.diff (0.8 kB) - added by garru on 01/17/08 21:02:04.
removed double quote_table_name definition

Change History

01/04/08 14:53:49 changed by dimdenis

  • attachment proper_table_name_quoting_patch.diff added.

01/04/08 19:17:03 changed by lotswholetime

  • attachment 10698.diff added.

Updated patch (quoted a missed table name, adds missing files)

01/04/08 19:17:41 changed by lotswholetime

+1 with my changes...

  • has_and_belongs_to_many_association.rb:132 -- missed quoting a options[:join_table]
  • forgot to include the new warehouse_thing model file
  • forgot to include the new warehouse-thing fixture

01/05/08 00:54:58 changed by bitsweat

  • status changed from new to closed.
  • resolution set to untested.
  • milestone changed from 2.x to 2.1.

Many test failures with sqlite and postgresql.

01/05/08 01:16:35 changed by lotswholetime

  • attachment 10698.2.diff added.

Updated patch to fix errors against sqlite3

01/05/08 01:17:46 changed by lotswholetime

  • attachment 10698.2.2.diff added.

01/05/08 14:58:32 changed by bitsweat

  • resolution changed from untested to fixed.

(In [8571]) More thoroughly quote table names. Exposes some issues with sqlite2 adapter. Closes #10698.

01/17/08 20:57:38 changed by garru

  • status changed from closed to reopened.
  • resolution deleted.

01/17/08 21:00:06 changed by garru

  • attachment 8651.diff added.

removed double declaration

01/17/08 21:02:04 changed by garru

  • attachment 8651.2.diff added.

removed double quote_table_name definition

01/17/08 21:06:18 changed by garru

I probably didn't follow the correct process, in reopening the ticket. So I apologize in advance.

I initially had some problems with the new version of quote_table_name, and I noticed that the old version wasn't removed from the source.

01/19/08 03:23:49 changed by bitsweat

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [8670]) Remove dead code. Closes #10698 [garru]

06/19/08 15:52:46 changed by masahji

It appears as though some of these changes got mixed up. The AbstractAdapter is now by default doing table name quoting. However, someone patched the SQLite3Adapter table_structure method to always to table name quoting when calling @connection.table_info. They must have been under the assumption that AbstractAdapter was not doing table name quoting. Because of this, the SQLite3Adapter does double quoting. We are seeing this as still broken in activerecord-2.1.0.

07/18/08 22:21:45 changed by booch

This is still broken for me in Rails 2.1.0 when using the sqlite3 adapter (which is of course, the default). Observe, and note that the "abcs" table does exist, but not the "'abcs'" table, which is what it's looking for:

$ rails --version
Rails 2.1.0
$ rails -q test10698
$ cd test10698
$ rake db:create
(in /home/booch/projects/test10698)
db/development.sqlite3 already exists
$ mkdir -p db/migrate
$ echo 'class ABC<ActiveRecord::Migration;def self.up;create_table(:abcs){};end;end' > db/migrate/0_abc.rb
$ rake db:migrate
(in /home/booch/projects/test10698)
== 0 Abc: migrating ===========================================================
-- create_table(:abcs)
   -> 0.0125s
== 0 Abc: migrated (0.0130s) ==================================================

rake aborted!
Could not find table 'abcs'

(See full trace by running task with --trace)
$ sqlite3 db/development.sqlite3 
SQLite version 3.3.8
Enter ".help" for instructions
sqlite> .schema
CREATE TABLE "abcs" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL);
CREATE TABLE "schema_migrations" ("version" varchar(255) NOT NULL);
CREATE UNIQUE INDEX "unique_schema_migrations" ON "schema_migrations" ("version");
$ rake db:schema:dump --trace
(in /home/booch/projects/test10698)
** Invoke db:schema:dump (first_time)
** Invoke environment (first_time)
** Execute environment
** Execute db:schema:dump
rake aborted!
Could not find table 'abcs'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/connection_adapters/sqlite3_adapter.rb:29:in `table_structure'
/usr/local/lib/site_ruby/1.8/gems/gems/activesupport-2.1.0/lib/active_support/core_ext/object/misc.rb:28:in `returning'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/connection_adapters/sqlite3_adapter.rb:28:in `table_structure'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/connection_adapters/sqlite_adapter.rb:189:in `columns'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/schema_dumper.rb:75:in `table'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/schema_dumper.rb:70:in `tables'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/schema_dumper.rb:61:in `each'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/schema_dumper.rb:61:in `tables'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/schema_dumper.rb:23:in `dump'
/usr/local/lib/site_ruby/1.8/gems/gems/activerecord-2.1.0/lib/active_record/schema_dumper.rb:17:in `dump'
/usr/local/lib/site_ruby/1.8/gems/gems/rails-2.1.0/lib/tasks/databases.rake:219
/usr/local/lib/site_ruby/1.8/gems/gems/rails-2.1.0/lib/tasks/databases.rake:218:in `open'
/usr/local/lib/site_ruby/1.8/gems/gems/rails-2.1.0/lib/tasks/databases.rake:218
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:546:in `call'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:546:in `execute'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:541:in `each'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:541:in `execute'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:508:in `invoke_with_call_chain'
/usr/lib/ruby/1.8/thread.rb:135:in `synchronize'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:501:in `invoke_with_call_chain'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:494:in `invoke'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1931:in `invoke_task'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1909:in `top_level'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1909:in `each'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1909:in `top_level'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1948:in `standard_exception_handling'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1903:in `top_level'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1881:in `run'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1948:in `standard_exception_handling'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/lib/rake.rb:1878:in `run'
/usr/local/lib/site_ruby/1.8/gems/gems/rake-0.8.1/bin/rake:31
/usr/local/bin/rake:19:in `load'
/usr/local/bin/rake:19

Removing the call to quote_table_name() in line 28 of activerecord-2.1.0/lib/active_record/connection_adapters/sqlite3_adapter.rb (as masahji suggested) fixes the problem.

10/14/08 05:02:26 changed by sunyc

Hi, still seeing this broken in 2.1.2