Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Quote table names. Defaults to column quoting. Closes #4593.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7932 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 9b6207c3d0596599078b4171caa71b6e7c49ebc9 1 parent e7ed4c9
@jeremy jeremy authored
View
2  activerecord/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Quote table names. Defaults to column quoting. #4593 [Justin Lynn, gwcoffey, eadz, Dmitry V. Sabanin, Jeremy Kemper]
+
* Alias association #build to #new so it behaves predictably. #8787 [lifofifo]
* Add notes to documentation regarding attr_readonly behavior with counter caches and polymorphic associations. Closes #9835 [saimonmoore, rick]
View
26 activerecord/lib/active_record/base.rb
@@ -531,7 +531,7 @@ def destroy_all(conditions = nil)
# calling the destroy method). Example:
# Post.delete_all "person_id = 5 AND (category = 'Something' OR category = 'Else')"
def delete_all(conditions = nil)
- sql = "DELETE FROM #{table_name} "
+ sql = "DELETE FROM #{quoted_table_name} "
add_conditions!(sql, conditions, scope(:find))
connection.delete(sql, "#{name} Delete all")
end
@@ -1033,7 +1033,7 @@ def find_from_ids(ids, options)
def find_one(id, options)
conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions]
- options.update :conditions => "#{table_name}.#{connection.quote_column_name(primary_key)} = #{quote_value(id,columns_hash[primary_key])}#{conditions}"
+ options.update :conditions => "#{quoted_table_name}.#{connection.quote_column_name(primary_key)} = #{quote_value(id,columns_hash[primary_key])}#{conditions}"
# Use find_every(options).first since the primary key condition
# already ensures we have a single record. Using find_initial adds
@@ -1048,7 +1048,7 @@ def find_one(id, options)
def find_some(ids, options)
conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions]
ids_list = ids.map { |id| quote_value(id,columns_hash[primary_key]) }.join(',')
- options.update :conditions => "#{table_name}.#{connection.quote_column_name(primary_key)} IN (#{ids_list})#{conditions}"
+ options.update :conditions => "#{quoted_table_name}.#{connection.quote_column_name(primary_key)} IN (#{ids_list})#{conditions}"
result = find_every(options)
@@ -1126,8 +1126,8 @@ def type_name_with_module(type_name)
def construct_finder_sql(options)
scope = scope(:find)
- sql = "SELECT #{(scope && scope[:select]) || options[:select] || (options[:joins] && table_name + '.*') || '*'} "
- sql << "FROM #{(scope && scope[:from]) || options[:from] || table_name} "
+ sql = "SELECT #{(scope && scope[:select]) || options[:select] || (options[:joins] && quoted_table_name + '.*') || '*'} "
+ sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} "
add_joins!(sql, options, scope)
add_conditions!(sql, options[:conditions], scope)
@@ -1220,8 +1220,8 @@ def add_conditions!(sql, conditions, scope = :auto)
def type_condition
quoted_inheritance_column = connection.quote_column_name(inheritance_column)
- type_condition = subclasses.inject("#{table_name}.#{quoted_inheritance_column} = '#{name.demodulize}' ") do |condition, subclass|
- condition << "OR #{table_name}.#{quoted_inheritance_column} = '#{subclass.name.demodulize}' "
+ type_condition = subclasses.inject("#{quoted_table_name}.#{quoted_inheritance_column} = '#{name.demodulize}' ") do |condition, subclass|
+ condition << "OR #{quoted_table_name}.#{quoted_inheritance_column} = '#{subclass.name.demodulize}' "
end
" (#{type_condition}) "
@@ -1572,7 +1572,7 @@ def sanitize_sql_for_assignment(assignments)
# # => "age BETWEEN 13 AND 18"
def sanitize_sql_hash_for_conditions(attrs)
conditions = attrs.map do |attr, value|
- "#{table_name}.#{connection.quote_column_name(attr)} #{attribute_condition(value)}"
+ "#{quoted_table_name}.#{connection.quote_column_name(attr)} #{attribute_condition(value)}"
end.join(' AND ')
replace_bind_variables(conditions, expand_range_bind_variables(attrs.values))
@@ -1742,7 +1742,7 @@ def save!
def destroy
unless new_record?
connection.delete <<-end_sql, "#{self.class.name} Destroy"
- DELETE FROM #{self.class.table_name}
+ DELETE FROM #{self.class.quoted_table_name}
WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quoted_id}
end_sql
end
@@ -1986,7 +1986,7 @@ def update
quoted_attributes = attributes_with_quotes(false, false)
return 0 if quoted_attributes.empty?
connection.update(
- "UPDATE #{self.class.table_name} " +
+ "UPDATE #{self.class.quoted_table_name} " +
"SET #{quoted_comma_pair_list(connection, quoted_attributes)} " +
"WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quote_value(id)}",
"#{self.class.name} Update"
@@ -2005,7 +2005,7 @@ def create
statement = if quoted_attributes.empty?
connection.empty_insert_statement(self.class.table_name)
else
- "INSERT INTO #{self.class.table_name} " +
+ "INSERT INTO #{self.class.quoted_table_name} " +
"(#{quoted_column_names.join(', ')}) " +
"VALUES(#{quoted_attributes.values.join(', ')})"
end
@@ -2180,6 +2180,10 @@ def quoted_column_names(attributes = attributes_with_quotes)
end
end
+ def self.quoted_table_name
+ self.connection.quote_table_name(self.table_name)
+ end
+
def quote_columns(quoter, hash)
hash.inject({}) do |quoted, (name, value)|
quoted[quoter.quote_column_name(name)] = value
View
4 activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -136,11 +136,11 @@ def reset_sequence!(table, column, sequence = nil)
# Inserts the given fixture into the table. Overridden in adapters that require
# something beyond a simple insert (eg. Oracle).
def insert_fixture(fixture, table_name)
- execute "INSERT INTO #{table_name} (#{fixture.key_list}) VALUES (#{fixture.value_list})", 'Fixture Insert'
+ execute "INSERT INTO #{quote_table_name(table_name)} (#{fixture.key_list}) VALUES (#{fixture.value_list})", 'Fixture Insert'
end
def empty_insert_statement(table_name)
- "INSERT INTO #{table_name} VALUES(DEFAULT)"
+ "INSERT INTO #{quote_table_name(table_name)} VALUES(DEFAULT)"
end
protected
View
12 activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
@@ -39,10 +39,14 @@ def quote_string(s)
s.gsub(/\\/, '\&\&').gsub(/'/, "''") # ' (for ruby-mode)
end
- # Returns a quoted form of the column name. This is highly adapter
- # specific.
- def quote_column_name(name)
- name
+ # Quotes the column name. Defaults to no quoting.
+ def quote_column_name(column_name)
+ column_name
+ end
+
+ # Quotes the table name. Defaults to column name quoting.
+ def quote_table_name(table_name)
+ quote_column_name(table_name)
end
def quoted_true
View
26 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -87,39 +87,39 @@ def columns(table_name, name = nil) end
# )
#
# See also TableDefinition#column for details on how to create columns.
- def create_table(name, options = {})
+ def create_table(table_name, options = {})
table_definition = TableDefinition.new(self)
table_definition.primary_key(options[:primary_key] || "id") unless options[:id] == false
yield table_definition
if options[:force]
- drop_table(name, options) rescue nil
+ drop_table(table_name, options) rescue nil
end
create_sql = "CREATE#{' TEMPORARY' if options[:temporary]} TABLE "
- create_sql << "#{name} ("
+ create_sql << "#{quote_table_name(table_name)} ("
create_sql << table_definition.to_sql
create_sql << ") #{options[:options]}"
execute create_sql
end
-
+
# Renames a table.
# ===== Example
# rename_table('octopuses', 'octopi')
- def rename_table(name, new_name)
+ def rename_table(table_name, new_name)
raise NotImplementedError, "rename_table is not implemented"
end
# Drops a table from the database.
- def drop_table(name, options = {})
- execute "DROP TABLE #{name}"
+ def drop_table(table_name, options = {})
+ execute "DROP TABLE #{quote_table_name(table_name)}"
end
# Adds a new column to the named table.
# See TableDefinition#column for details of the options you can use.
def add_column(table_name, column_name, type, options = {})
- add_column_sql = "ALTER TABLE #{table_name} ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
+ add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
add_column_options!(add_column_sql, options)
execute(add_column_sql)
end
@@ -128,7 +128,7 @@ def add_column(table_name, column_name, type, options = {})
# ===== Examples
# remove_column(:suppliers, :qualification)
def remove_column(table_name, column_name)
- execute "ALTER TABLE #{table_name} DROP #{quote_column_name(column_name)}"
+ execute "ALTER TABLE #{quote_table_name(table_name)} DROP #{quote_column_name(column_name)}"
end
# Changes the column's definition according to the new options.
@@ -194,7 +194,7 @@ def add_index(table_name, column_name, options = {})
index_type = options
end
quoted_column_names = column_names.map { |e| quote_column_name(e) }.join(", ")
- execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{table_name} (#{quoted_column_names})"
+ execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} (#{quoted_column_names})"
end
# Remove the given index from the table.
@@ -234,8 +234,8 @@ def structure_dump
# The migrations module handles this automatically.
def initialize_schema_information
begin
- execute "CREATE TABLE #{ActiveRecord::Migrator.schema_info_table_name} (version #{type_to_sql(:integer)})"
- execute "INSERT INTO #{ActiveRecord::Migrator.schema_info_table_name} (version) VALUES(0)"
+ execute "CREATE TABLE #{quote_table_name(ActiveRecord::Migrator.schema_info_table_name)} (version #{type_to_sql(:integer)})"
+ execute "INSERT INTO #{quote_table_name(ActiveRecord::Migrator.schema_info_table_name)} (version) VALUES(0)"
rescue ActiveRecord::StatementInvalid
# Schema has been initialized
end
@@ -244,7 +244,7 @@ def initialize_schema_information
def dump_schema_information #:nodoc:
begin
if (current_schema = ActiveRecord::Migrator.current_version) > 0
- return "INSERT INTO #{ActiveRecord::Migrator.schema_info_table_name} (version) VALUES (#{current_schema})"
+ return "INSERT INTO #{quote_table_name(ActiveRecord::Migrator.schema_info_table_name)} (version) VALUES (#{current_schema})"
end
rescue ActiveRecord::StatementInvalid
# No Schema Info
View
6 activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -63,6 +63,12 @@ def reset_runtime #:nodoc:
rt
end
+ # QUOTING ==================================================
+
+ # Override to return the quoted table name if the database needs it
+ def quote_table_name(name)
+ name
+ end
# CONNECTION MANAGEMENT ====================================
View
32 activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -208,6 +208,10 @@ def quote_column_name(name) #:nodoc:
"`#{name}`"
end
+ def quote_table_name(name) #:nodoc:
+ quote_column_name(name)
+ end
+
def quote_string(string) #:nodoc:
@connection.quote(string)
end
@@ -322,7 +326,7 @@ def structure_dump #:nodoc:
select_all(sql).inject("") do |structure, table|
table.delete('Table_type')
- structure += select_one("SHOW CREATE TABLE #{table.to_a.first.last}")["Create Table"] + ";\n\n"
+ structure += select_one("SHOW CREATE TABLE #{quote_table_name(table.to_a.first.last)}")["Create Table"] + ";\n\n"
end
end
@@ -370,10 +374,14 @@ def tables(name = nil) #:nodoc:
tables
end
+ def drop_table(table_name, options = {})
+ super(table_name, options)
+ end
+
def indexes(table_name, name = nil)#:nodoc:
indexes = []
current_index = nil
- execute("SHOW KEYS FROM #{table_name}", name).each do |row|
+ execute("SHOW KEYS FROM #{quote_table_name(table_name)}", name).each do |row|
if current_index != row[2]
next if row[2] == "PRIMARY" # skip the primary key
current_index = row[2]
@@ -386,24 +394,24 @@ def indexes(table_name, name = nil)#:nodoc:
end
def columns(table_name, name = nil)#:nodoc:
- sql = "SHOW FIELDS FROM #{table_name}"
+ sql = "SHOW FIELDS FROM #{quote_table_name(table_name)}"
columns = []
execute(sql, name).each { |field| columns << MysqlColumn.new(field[0], field[4], field[1], field[2] == "YES") }
columns
end
- def create_table(name, options = {}) #:nodoc:
- super(name, {:options => "ENGINE=InnoDB"}.merge(options))
+ def create_table(table_name, options = {}) #:nodoc:
+ super(table_name, options.reverse_merge(:options => "ENGINE=InnoDB"))
end
- def rename_table(name, new_name)
- execute "RENAME TABLE #{name} TO #{new_name}"
+ def rename_table(table_name, new_name)
+ execute "RENAME TABLE #{quote_table_name(table_name)} TO #{quote_table_name(new_name)}"
end
def change_column_default(table_name, column_name, default) #:nodoc:
- current_type = select_one("SHOW COLUMNS FROM #{table_name} LIKE '#{column_name}'")["Type"]
+ current_type = select_one("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE '#{column_name}'")["Type"]
- execute("ALTER TABLE #{table_name} CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{current_type} DEFAULT #{quote(default)}")
+ execute("ALTER TABLE #{quote_table_name(table_name)} CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{current_type} DEFAULT #{quote(default)}")
end
def change_column(table_name, column_name, type, options = {}) #:nodoc:
@@ -415,14 +423,14 @@ def change_column(table_name, column_name, type, options = {}) #:nodoc:
end
end
- change_column_sql = "ALTER TABLE #{table_name} CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
+ change_column_sql = "ALTER TABLE #{quote_table_name(table_name)} CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
add_column_options!(change_column_sql, options)
execute(change_column_sql)
end
def rename_column(table_name, column_name, new_column_name) #:nodoc:
- current_type = select_one("SHOW COLUMNS FROM #{table_name} LIKE '#{column_name}'")["Type"]
- execute "ALTER TABLE #{table_name} CHANGE #{quote_column_name(column_name)} #{quote_column_name(new_column_name)} #{current_type}"
+ current_type = select_one("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE '#{column_name}'")["Type"]
+ execute "ALTER TABLE #{quote_table_name(table_name)} CHANGE #{quote_column_name(column_name)} #{quote_column_name(new_column_name)} #{current_type}"
end
View
2  activerecord/lib/active_record/fixtures.rb
@@ -318,7 +318,7 @@ def initialize(connection, table_name, class_name, fixture_path, file_filter = D
end
def delete_existing_fixtures
- @connection.delete "DELETE FROM #{@table_name}", 'Fixture Delete'
+ @connection.delete "DELETE FROM #{@connection.quote_table_name(table_name)}", 'Fixture Delete'
end
def insert_fixtures
View
6 activerecord/test/active_schema_test_mysql.rb
@@ -13,7 +13,7 @@ def teardown
end
def test_drop_table
- assert_equal "DROP TABLE people", drop_table(:people)
+ assert_equal "DROP TABLE `people`", drop_table(:people)
end
if current_adapter?(:MysqlAdapter)
@@ -25,11 +25,11 @@ def test_create_mysql_database_with_encoding
end
def test_add_column
- assert_equal "ALTER TABLE people ADD `last_name` varchar(255)", add_column(:people, :last_name, :string)
+ assert_equal "ALTER TABLE `people` ADD `last_name` varchar(255)", add_column(:people, :last_name, :string)
end
def test_add_column_with_limit
- assert_equal "ALTER TABLE people ADD `key` varchar(32)", add_column(:people, :key, :string, :limit => 32)
+ assert_equal "ALTER TABLE `people` ADD `key` varchar(32)", add_column(:people, :key, :string, :limit => 32)
end
private
View
5 activerecord/test/fixtures/reserved_words/distinct.yml
@@ -0,0 +1,5 @@
+distinct1:
+ id: 1
+
+distinct2:
+ id: 2
View
11 activerecord/test/fixtures/reserved_words/distincts_selects.yml
@@ -0,0 +1,11 @@
+distincts_selects1:
+ distinct_id: 1
+ select_id: 1
+
+distincts_selects2:
+ distinct_id: 1
+ select_id: 2
+
+distincts_selects3:
+ distinct_id: 2
+ select_id: 3
View
14 activerecord/test/fixtures/reserved_words/group.yml
@@ -0,0 +1,14 @@
+group1:
+ id: 1
+ select_id: 1
+ order: x
+
+group2:
+ id: 2
+ select_id: 2
+ order: y
+
+group3:
+ id: 3
+ select_id: 2
+ order: z
View
8 activerecord/test/fixtures/reserved_words/select.yml
@@ -0,0 +1,8 @@
+select1:
+ id: 1
+
+select2:
+ id: 2
+
+select3:
+ id: 3
View
7 activerecord/test/fixtures/reserved_words/values.yml
@@ -0,0 +1,7 @@
+values1:
+ id: 1
+ group_id: 2
+
+values2:
+ id: 2
+ group_id: 1
View
168 activerecord/test/reserved_word_test_mysql.rb
@@ -0,0 +1,168 @@
+require "#{File.dirname(__FILE__)}/abstract_unit"
+
+class Group < ActiveRecord::Base
+ Group.table_name = 'group'
+ belongs_to :select, :class_name => 'Select'
+ has_one :values
+end
+
+class Select < ActiveRecord::Base
+ Select.table_name = 'select'
+ has_many :groups
+end
+
+class Values < ActiveRecord::Base
+ Values.table_name = 'values'
+end
+
+class Distinct < ActiveRecord::Base
+ Distinct.table_name = 'distinct'
+ has_and_belongs_to_many :selects
+ has_many :values, :through => :groups
+end
+
+# a suite of tests to ensure the ConnectionAdapters#MysqlAdapter can handle tables with
+# reserved word names (ie: group, order, values, etc...)
+class MysqlReservedWordTest < Test::Unit::TestCase
+ def setup
+ @connection = ActiveRecord::Base.connection
+
+ # we call execute directly here (and do similar below) because ActiveRecord::Base#create_table()
+ # will fail with these table names if these test cases fail
+
+ create_tables_directly 'group'=>'id int auto_increment primary key, `order` varchar(255), select_id int',
+ 'select'=>'id int auto_increment primary key',
+ 'values'=>'id int auto_increment primary key, group_id int',
+ 'distinct'=>'id int auto_increment primary key',
+ 'distincts_selects'=>'distinct_id int, select_id int'
+ end
+
+ def teardown
+ drop_tables_directly ['group', 'select', 'values', 'distinct', 'distincts_selects', 'order']
+ end
+
+ # create tables with reserved-word names and columns
+ def test_create_tables
+ assert_nothing_raised {
+ @connection.create_table :order do |t|
+ t.column :group, :string
+ end
+ }
+ end
+
+ # rename tables with reserved-word names
+ def test_rename_tables
+ assert_nothing_raised { @connection.rename_table(:group, :order) }
+ end
+
+ # alter column with a reserved-word name in a table with a reserved-word name
+ def test_change_columns
+ assert_nothing_raised { @connection.change_column_default(:group, :order, 'whatever') }
+ #the quoting here will reveal any double quoting issues in change_column's interaction with the column method in the adapter
+ assert_nothing_raised { @connection.change_column('group', 'order', :Int, :default => 0) }
+ assert_nothing_raised { @connection.rename_column(:group, :order, :values) }
+ end
+
+ # dump structure of table with reserved word name
+ def test_structure_dump
+ assert_nothing_raised { @connection.structure_dump }
+ end
+
+ # introspect table with reserved word name
+ def test_introspect
+ assert_nothing_raised { @connection.columns(:group) }
+ assert_nothing_raised { @connection.indexes(:group) }
+ end
+
+ #fixtures
+ self.use_instantiated_fixtures = true
+ self.use_transactional_fixtures = false
+
+ #fixtures :group
+
+ def test_fixtures
+ f = create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
+
+ assert_nothing_raised {
+ f.each do |x|
+ x.delete_existing_fixtures
+ end
+ }
+
+ assert_nothing_raised {
+ f.each do |x|
+ x.insert_fixtures
+ end
+ }
+ end
+
+ #activerecord model class with reserved-word table name
+ def test_activerecord_model
+ create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
+ x = nil
+ assert_nothing_raised { x = Group.new }
+ x.order = 'x'
+ assert_nothing_raised { x.save }
+ x.order = 'y'
+ assert_nothing_raised { x.save }
+ assert_nothing_raised { y = Group.find_by_order('y') }
+ assert_nothing_raised { y = Group.find(1) }
+ x = Group.find(1)
+ end
+
+ # has_one association with reserved-word table name
+ def test_has_one_associations
+ create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
+ v = nil
+ assert_nothing_raised { v = Group.find(1).values }
+ assert_equal v.id, 2
+ end
+
+ # belongs_to association with reserved-word table name
+ def test_belongs_to_associations
+ create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
+ gs = nil
+ assert_nothing_raised { gs = Select.find(2).groups }
+ assert_equal gs.length, 2
+ assert(gs.collect{|x| x.id}.sort == [2, 3])
+ end
+
+ # has_and_belongs_to_many with reserved-word table name
+ def test_has_and_belongs_to_many
+ create_test_fixtures :select, :distinct, :group, :values, :distincts_selects
+ s = nil
+ assert_nothing_raised { s = Distinct.find(1).selects }
+ assert_equal s.length, 2
+ assert(s.collect{|x|x.id}.sort == [1, 2])
+ end
+
+ # activerecord model introspection with reserved-word table and column names
+ def test_activerecord_introspection
+ assert_nothing_raised { Group.table_exists? }
+ assert_nothing_raised { Group.columns }
+ end
+
+ #the following functions were added to DRY test cases
+
+ private
+ # custom fixture loader, uses Fixtures#create_fixtures and appends base_path to the current file's path
+ def create_test_fixtures(*fixture_names)
+ fixture_path = "./test/fixtures/reserved_words"
+ Fixtures.create_fixtures(fixture_path, fixture_names)
+ end
+
+ # custom drop table, uses execute on connection to drop a table if it exists. note: escapes table_name
+ def drop_tables_directly(table_names, connection = @connection)
+ table_names.each do |name|
+ connection.execute("DROP TABLE IF EXISTS `#{name}`")
+ end
+ end
+
+ # custom create table, uses execute on connection to create a table, note: escapes table_name, does NOT escape columns
+ def create_tables_directly (tables, connection = @connection)
+ tables.each do |table_name, column_properties|
+ connection.execute("CREATE TABLE `#{table_name}` ( #{column_properties} )")
+ end
+ end
+
+end
Please sign in to comment.
Something went wrong with that request. Please try again.