Fixed value quoting in all generated SQL statements, so that integers are not surrounded in quotes and that all sanitation are happening through the database's own quoting routine. This should hopefully make it lots easier for new adapters that doesn't accept '1' for integer columns.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@70 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

10 files changed, 71 insertions, 51 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 80218b9070..eb8a906338 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,9 @@
 *CVS*
 
+* Fixed value quoting in all generated SQL statements, so that integers are not surrounded in quotes and that all sanitation are happening
+  through the database's own quoting routine. This should hopefully make it lots easier for new adapters that doesn't accept '1' for integer
+  columns.
+
 * Fixed has_and_belongs_to_many guessing of foreign key so that keys are generated correctly for models like SomeVerySpecialClient 
   [Florian Weber]
 
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b83ce831c7..6f6da10a01 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -190,7 +190,7 @@ module ActiveRecord
         elsif options[:dependent]
           module_eval "before_destroy '#{association_name}.each { |o| o.destroy }'"
         elsif options[:exclusively_dependent]
-          module_eval "before_destroy { |record| #{association_class_name}.delete_all(%(#{association_class_primary_key_name} = '\#{record.id}')) }"
+          module_eval "before_destroy { |record| #{association_class_name}.delete_all(%(#{association_class_primary_key_name} = \#{record.quoted_id})) }"
         end
 
         define_method(association_name) do |*params|
@@ -323,7 +323,7 @@ module ActiveRecord
           if options[:remote]
             association_finder = <<-"end_eval"
               #{association_class_name}.find_first(
-                "#{class_primary_key_name} = '\#{id}'#{options[:conditions] ? " AND " + options[:conditions] : ""}",
+                "#{class_primary_key_name} = \#{quoted_id}#{options[:conditions] ? " AND " + options[:conditions] : ""}",
                 #{options[:order] ? "\"" + options[:order] + "\"" : "nil" }
               )
             end_eval
@@ -437,7 +437,7 @@ module ActiveRecord
           association
         end
 
-        before_destroy_sql = "DELETE FROM #{join_table} WHERE #{association_class_primary_key_name} = '\\\#{self.id}'"
+        before_destroy_sql = "DELETE FROM #{join_table} WHERE #{association_class_primary_key_name} = \\\#{self.quoted_id}"
         module_eval(%{before_destroy "self.connection.delete(%{#{before_destroy_sql}})"}) # "
         
         # deprecated api
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index a60b9ddab5..00758aa66c 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -81,7 +81,7 @@ module ActiveRecord
         end
 
         def quoted_record_ids(records)
-          records.map { |record| "'#{@association_class.send(:sanitize, record.id)}'" }.join(',')
+          records.map { |record| record.quoted_id }.join(',')
         end
 
         def interpolate_sql_options!(options, *keys)
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 8dec71403c..d53650fbd8 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -13,7 +13,7 @@ module ActiveRecord
         @finder_sql = options[:finder_sql] ||
               "SELECT t.*, j.* FROM #{association_table_name} t, #{@join_table} j " +
               "WHERE t.#{@owner.class.primary_key} = j.#{@association_foreign_key} AND " +
-              "j.#{association_class_primary_key_name} = '#{@owner.id}' " +
+              "j.#{association_class_primary_key_name} = #{@owner.quoted_id} " +
               (options[:conditions] ? " AND " + options[:conditions] : "") + " " +
               "ORDER BY #{@order}"
       end
@@ -26,11 +26,11 @@ module ActiveRecord
           each { |record| @owner.connection.execute(sql) }
         elsif @options[:conditions] 
           sql = 
-            "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = '#{@owner.id}' " +
+            "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = #{@owner.quoted_id} " +
             "AND #{@association_foreign_key} IN (#{collect { |record| record.id }.join(", ")})"
           @owner.connection.execute(sql)
         else
-          sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = '#{@owner.id}'"
+          sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = #{@owner.quoted_id}"
           @owner.connection.execute(sql)
         end
 
@@ -46,7 +46,7 @@ module ActiveRecord
           if loaded?
             find_all { |record| record.id == association_id.to_i }.first
           else
-            find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} = '#{association_id}' ORDER BY")).first
+            find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} = #{@owner.send(:quote, association_id)} ORDER BY")).first
           end
         end
       end
@@ -80,7 +80,8 @@ module ActiveRecord
           if @options[:insert_sql]
             @owner.connection.execute(interpolate_sql(@options[:insert_sql], record))
           else
-            sql = "INSERT INTO #{@join_table} (#{@association_class_primary_key_name}, #{@association_foreign_key}) VALUES ('#{@owner.id}','#{record.id}')"
+            sql = "INSERT INTO #{@join_table} (#{@association_class_primary_key_name}, #{@association_foreign_key}) " + 
+                  "VALUES (#{@owner.quoted_id},#{record.quoted_id})"
             @owner.connection.execute(sql)
           end
         end
@@ -98,7 +99,7 @@ module ActiveRecord
             records.each { |record| @owner.connection.execute(sql) }
           else
             ids = quoted_record_ids(records)
-            sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = '#{@owner.id}' AND #{@association_foreign_key} IN (#{ids})"
+            sql = "DELETE FROM #{@join_table} WHERE #{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_foreign_key} IN (#{ids})"
             @owner.connection.execute(sql)
           end
         end
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 0f2d20d240..1d8441e6f8 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -8,7 +8,7 @@ module ActiveRecord
         if options[:finder_sql]
           @finder_sql = interpolate_sql(options[:finder_sql])
         else
-          @finder_sql = "#{@association_class_primary_key_name} = '#{@owner.id}' #{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
+          @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id} #{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
         end
 
         if options[:counter_sql]
@@ -16,7 +16,7 @@ module ActiveRecord
         elsif options[:finder_sql]
           @counter_sql = options[:counter_sql] = @finder_sql.gsub(/SELECT (.*) FROM/i, "SELECT COUNT(*) FROM")
         else
-          @counter_sql = "#{@association_class_primary_key_name} = '#{@owner.id}'#{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
+          @counter_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
         end
       end
 
@@ -40,8 +40,8 @@ module ActiveRecord
           @collection.find_all(&block)
         else
           @association_class.find_all(
-            "#{@association_class_primary_key_name} = '#{@owner.id}' " +
-            "#{@conditions ? " AND " + @conditions : ""} #{runtime_conditions ? " AND " + @association_class.send(:sanitize_conditions, runtime_conditions) : ""}",
+            "#{@association_class_primary_key_name} = #{@owner.quoted_id}" +
+            "#{@conditions ? " AND " + @conditions : ""}#{runtime_conditions ? " AND " + @association_class.send(:sanitize_conditions, runtime_conditions) : ""}",
             orderings, 
             limit, 
             joins
@@ -55,7 +55,7 @@ module ActiveRecord
           @collection.find(&block)
         else
           @association_class.find_on_conditions(association_id,
-            "#{@association_class_primary_key_name} = '#{@owner.id}' #{@conditions ? " AND " + @conditions : ""}"
+            "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@conditions ? " AND " + @conditions : ""}"
           )
         end
       end
@@ -63,7 +63,7 @@ module ActiveRecord
       # Removes all records from this association.  Returns +self+ so
       # method calls may be chained.
       def clear
-        @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = '#{@owner.id}'")
+        @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = #{@owner.quoted_id}")
         @collection = []
         self
       end
@@ -101,7 +101,10 @@ module ActiveRecord
 
         def delete_records(records)
           ids = quoted_record_ids(records)
-          @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = '#{@owner.id}' AND #{@association_class.primary_key} IN (#{ids})")
+          @association_class.update_all(
+            "#{@association_class_primary_key_name} = NULL", 
+            "#{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_class.primary_key} IN (#{ids})"
+          )
         end
     end
   end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index a45480945e..f52a1524d2 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -239,7 +239,7 @@ module ActiveRecord #:nodoc:
         ids = ids.flatten.compact.uniq
 
         if ids.length > 1
-          ids_list = ids.map{ |id| "'#{sanitize(id)}'" }.join(", ")
+          ids_list = ids.map{ |id| "#{sanitize(id)}" }.join(", ")
           objects  = find_all("#{primary_key} IN (#{ids_list})", primary_key)
 
           if objects.length == ids.length
@@ -249,7 +249,7 @@ module ActiveRecord #:nodoc:
           end
         elsif ids.length == 1
           id = ids.first
-          sql = "SELECT * FROM #{table_name} WHERE #{primary_key} = '#{sanitize(id)}'"
+          sql = "SELECT * FROM #{table_name} WHERE #{primary_key} = #{sanitize(id)}"
           sql << " AND #{type_condition}" unless descends_from_active_record?
 
           if record = connection.select_one(sql, "#{name} Find")
@@ -267,7 +267,7 @@ module ActiveRecord #:nodoc:
       # Example:
       #   Person.find_on_conditions 5, "first_name LIKE '%dav%' AND last_name = 'heinemeier'"
       def find_on_conditions(id, conditions)
-        find_first("#{primary_key} = '#{sanitize(id)}' AND #{sanitize_conditions(conditions)}") || 
+        find_first("#{primary_key} = #{sanitize(id)} AND #{sanitize_conditions(conditions)}") || 
             raise(RecordNotFound, "Couldn't find #{name} with #{primary_key} = #{id} on the condition of #{conditions}")
       end
     
@@ -370,12 +370,12 @@ module ActiveRecord #:nodoc:
       # for looping over a collection where each element require a number of aggregate values. Like the DiscussionBoard
       # that needs to list both the number of posts and comments.
       def increment_counter(counter_name, id)
-        update_all "#{counter_name} = #{counter_name} + 1", "#{primary_key} = #{id}"
+        update_all "#{counter_name} = #{counter_name} + 1", "#{primary_key} = #{quote(id)}"
       end
 
       # Works like increment_counter, but decrements instead.
       def decrement_counter(counter_name, id)
-        update_all "#{counter_name} = #{counter_name} - 1", "#{primary_key} = #{id}"
+        update_all "#{counter_name} = #{counter_name} - 1", "#{primary_key} = #{quote(id)}"
       end
 
       # Attributes named in this macro are protected from mass-assignment, such as <tt>new(attributes)</tt> and 
@@ -526,10 +526,13 @@ module ActiveRecord #:nodoc:
         superclass == Base
       end
 
-      # Used to sanitize objects before they're used in an SELECT SQL-statement.
+      def quote(object)
+        connection.quote(object)
+      end
+
+      # Used to sanitize objects before they're used in an SELECT SQL-statement. Delegates to <tt>connection.quote</tt>.
       def sanitize(object) # :nodoc:
-        return object if Fixnum === object
-        object.to_s.gsub(/([;:])/, "").gsub('##', '\#\#').gsub(/'/, "''") # ' (for ruby-mode)
+        connection.quote(object)
       end
 
       # Used to aggregate logging and benchmark, so you can measure and represent multiple statements in a single block.
@@ -592,7 +595,7 @@ module ActiveRecord #:nodoc:
         
         def type_condition
           " (" + subclasses.inject("#{inheritance_column} = '#{Inflector.demodulize(name)}' ") do |condition, subclass| 
-            condition << "OR #{inheritance_column} = '#{Inflector.demodulize(subclass.name)}'"
+            condition << "OR #{inheritance_column} = '#{Inflector.demodulize(subclass.name)}' "
           end + ") "
         end
 
@@ -638,7 +641,7 @@ module ActiveRecord #:nodoc:
 
           statement =~ /\?/ ?
             replace_bind_variables(statement, values) :
-            statement % values.collect { |value| sanitize(value) }
+            statement % values.collect { |value| connection.quote_string(value.to_s) }
         end
 
         def replace_bind_variables(statement, values)
@@ -669,6 +672,10 @@ module ActiveRecord #:nodoc:
         read_attribute(self.class.primary_key)
       end
       
+      def quoted_id
+        quote(id, self.class.columns_hash[self.class.primary_key])
+      end
+      
       # Sets the primary ID.
       def id=(value)
         write_attribute(self.class.primary_key, value)
@@ -692,7 +699,7 @@ module ActiveRecord #:nodoc:
         unless new_record?
           connection.delete(
             "DELETE FROM #{self.class.table_name} " + 
-            "WHERE #{self.class.primary_key} = '#{id}'", 
+            "WHERE #{self.class.primary_key} = #{quote(id)}", 
             "#{self.class.name} Destroy"
           )
         end
@@ -814,7 +821,7 @@ module ActiveRecord #:nodoc:
         connection.update(
           "UPDATE #{self.class.table_name} " +
           "SET #{quoted_comma_pair_list(connection, attributes_with_quotes(false))} " +
-          "WHERE #{self.class.primary_key} = '#{id}'",
+          "WHERE #{self.class.primary_key} = #{quote(id)}",
           "#{self.class.name} Update"
         )
       end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 301ee208a3..6f4dca1b7b 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -320,13 +320,14 @@ module ActiveRecord
 
       def quote(value, column = nil)
         case value
-          when String                      then "'#{quote_string(value)}'" # ' (for ruby-mode)
-          when NilClass                    then "NULL"
-          when TrueClass                   then (column && column.type == :boolean ? "'t'" : "1")
-          when FalseClass                  then (column && column.type == :boolean ? "'f'" : "0")
-          when Float, Fixnum, Bignum, Date then "'#{value.to_s}'" 
-          when Time, DateTime              then "'#{value.strftime("%Y-%m-%d %H:%M:%S")}'"
-          else                                  "'#{quote_string(value.to_yaml)}'"
+          when String                then "'#{quote_string(value)}'" # ' (for ruby-mode)
+          when NilClass              then "NULL"
+          when TrueClass             then (column && column.type == :boolean ? "'t'" : "1")
+          when FalseClass            then (column && column.type == :boolean ? "'f'" : "0")
+          when Float, Fixnum, Bignum then value.to_s
+          when Date                  then "'#{value.to_s}'" 
+          when Time, DateTime        then "'#{value.strftime("%Y-%m-%d %H:%M:%S")}'"
+          else                            "'#{quote_string(value.to_yaml)}'"
         end
       end
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 5dcdded5bc..55c15c6823 100755
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -117,6 +117,10 @@ module ActiveRecord
         execute "CREATE DATABASE #{name}"
       end
       
+      def quote_string(s)
+        Mysql::quote(s)
+      end
+
       private
         def select(sql, name = nil)
           result = nil
diff --git a/activerecord/lib/active_record/support/inflector.rb b/activerecord/lib/active_record/support/inflector.rb
index b41f85188b..bc5724e742 100644
--- a/activerecord/lib/active_record/support/inflector.rb
+++ b/activerecord/lib/active_record/support/inflector.rb
@@ -62,18 +62,18 @@ module Inflector
 
     def singular_rules #:doc:
       [
-          [/(x|ch|ss)es$/, '\1'],
-          [/movies$/, 'movie'],
-          [/([^aeiouy]|qu)ies$/, '\1y'],
-          [/([lr])ves$/, '\1f'],
-          [/([^f])ves$/, '\1fe'],
-          [/(analy|ba|diagno|parenthe|progno|synop|the)ses$/, '\1sis'],
-          [/([ti])a$/, '\1um'],
-          [/people$/, 'person'],
-          [/men$/, 'man'],
-          [/status$/, 'status'],
-          [/children$/, 'child'],
-          [/s$/, '']
-        ]
+        [/(x|ch|ss)es$/, '\1'],
+        [/movies$/, 'movie'],
+        [/([^aeiouy]|qu)ies$/, '\1y'],
+        [/([lr])ves$/, '\1f'],
+        [/([^f])ves$/, '\1fe'],
+        [/(analy|ba|diagno|parenthe|progno|synop|the)ses$/, '\1sis'],
+        [/([ti])a$/, '\1um'],
+        [/people$/, 'person'],
+        [/men$/, 'man'],
+        [/status$/, 'status'],
+        [/children$/, 'child'],
+        [/s$/, '']
+      ]
     end
 end
\ No newline at end of file
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index b7b4ab589a..cc240c8acc 100755
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -68,7 +68,7 @@ class FinderTest < Test::Unit::TestCase
   end
 	
   def test_string_sanitation
-    assert_equal "something '' 1=1", ActiveRecord::Base.sanitize("something ' 1=1")
-    assert_equal "something select table", ActiveRecord::Base.sanitize("something; select table")
+    assert_not_equal "'something ' 1=1'", ActiveRecord::Base.sanitize("something ' 1=1")
+    assert_equal "'something; select table'", ActiveRecord::Base.sanitize("something; select table")
   end
 end
\ No newline at end of file