r3095@asus: jeremy | 2005-11-15 22:40:51 -0800

 Ticket #1874 - Firebird adapter
 r3107@asus:  jeremy | 2005-11-16 00:06:14 -0800
 quote column aliases
 r3108@asus:  jeremy | 2005-11-16 00:08:12 -0800
 quote columns in construct_conditions_from_arguments.  update sequence_name docs.  introduce prefetched primary keys.
 r3109@asus:  jeremy | 2005-11-16 00:09:08 -0800
 double-quote rather than single-quote sqlite columns
 r3110@asus:  jeremy | 2005-11-16 00:09:56 -0800
 quote column names and use attribute_condition in validates_uniqueness_of
 r3111@asus:  jeremy | 2005-11-16 00:12:24 -0800
 Use QUOTED_TYPE constant in tests
 r3112@asus:  jeremy | 2005-11-16 00:13:28 -0800
 restrict test_inserts_with_pre_and_suffix to those adapters which support migrations
 r3113@asus:  jeremy | 2005-11-16 00:14:09 -0800
 Use QUOTED_TYPE constant in tests
 r3114@asus:  jeremy | 2005-11-16 00:14:30 -0800
 Use QUOTED_TYPE constant in tests


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

15 files changed, 88 insertions, 60 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index a4c440f4b4..d4b7f88907 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -999,7 +999,7 @@ module ActiveRecord
         end
 
         def column_aliases(schema_abbreviations)
-          schema_abbreviations.collect { |cn, tc| "#{tc.join(".")} AS #{cn}" }.join(", ")
+          schema_abbreviations.collect { |cn, tc| "#{tc[0]}.#{connection.quote_column_name tc[1]} AS #{cn}" }.join(", ")
         end
 
         def association_join(reflection)
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 06902a6927..550b657102 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -706,9 +706,11 @@ module ActiveRecord #:nodoc:
       # given block. This is required for Oracle and is useful for any
       # database which relies on sequences for primary key generation.
       #
-      # Setting the sequence name when using other dbs will have no effect.
-      # If a sequence name is not explicitly set when using Oracle, it will
-      # default to the commonly used pattern of: #{table_name}_seq
+      # If a sequence name is not explicitly set when using Oracle or Firebird,
+      # it will default to the commonly used pattern of: #{table_name}_seq
+      #
+      # If a sequence name is not explicitly set when using PostgreSQL, it
+      # will discover the sequence corresponding to your primary key for you.
       #
       # Example:
       #
@@ -962,8 +964,9 @@ module ActiveRecord #:nodoc:
         end
 
         def type_condition
-          type_condition = subclasses.inject("#{table_name}.#{inheritance_column} = '#{name.demodulize}' ") do |condition, subclass|
-            condition << "OR #{table_name}.#{inheritance_column} = '#{subclass.name.demodulize}' "
+          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}' "
           end
 
           " (#{type_condition}) "
@@ -1017,7 +1020,7 @@ module ActiveRecord #:nodoc:
 
         def construct_conditions_from_arguments(attribute_names, arguments)
           conditions = []
-          attribute_names.each_with_index { |name, idx| conditions << "#{table_name}.#{name} #{attribute_condition(arguments[idx])} " }
+          attribute_names.each_with_index { |name, idx| conditions << "#{table_name}.#{connection.quote_column_name(name)} #{attribute_condition(arguments[idx])} " }
           [ conditions.join(" AND "), *arguments[0...attribute_names.length] ]
         end
         
@@ -1457,6 +1460,10 @@ module ActiveRecord #:nodoc:
 
       # Creates a new record with values matching those of the instance attributes.
       def create
+        if self.id.nil? and connection.prefetch_primary_key?(self.class.table_name)
+          self.id = connection.next_sequence_value(self.class.sequence_name)
+        end
+
         self.id = connection.insert(
           "INSERT INTO #{self.class.table_name} " +
           "(#{quoted_column_names.join(', ')}) " +
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 9dbfe93cdf..ef835ff022 100755
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -45,6 +45,14 @@ module ActiveRecord
         false
       end
 
+      # Should primary key values be selected from their corresponding
+      # sequence before the insert statement?  If true, next_sequence_value
+      # is called before each insert to set the record's primary key.
+      # This is false for all adapters but Firebird.
+      def prefetch_primary_key?
+        false
+      end
+
       def reset_runtime #:nodoc:
         rt, @runtime = @runtime, 0
         rt
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index 2c302d7414..2c87d78fc8 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -123,7 +123,7 @@ module ActiveRecord
       end
 
       def quote_column_name(name) #:nodoc:
-        "'#{name}'"
+        %Q("#{name}")
       end
 
 
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 904153bf24..b6c9647b78 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -487,13 +487,20 @@ module ActiveRecord
         configuration = { :message => ActiveRecord::Errors.default_error_messages[:taken] }
         configuration.update(attr_names.pop) if attr_names.last.is_a?(Hash)
 
-        if scope = configuration[:scope]
-          validates_each(attr_names,configuration) do |record, attr_name, value|
-            record.errors.add(attr_name, configuration[:message]) if record.class.find(:first, :conditions => (record.new_record? ? ["#{attr_name} = ? AND #{scope} = ?", record.send(attr_name), record.send(scope)] : ["#{attr_name} = ? AND #{record.class.primary_key} <> ? AND #{scope} = ?", record.send(attr_name), record.send(:id), record.send(scope)]))
+        validates_each(attr_names,configuration) do |record, attr_name, value|
+          condition_sql = "#{attr_name} #{attribute_condition(value)}"
+          condition_params = [value]
+          if scope = configuration[:scope]
+            scope_value = record.send(scope)
+            condition_sql << " AND #{scope} #{attribute_condition(scope_value)}"
+            condition_params << scope_value
           end
-        else
-          validates_each(attr_names,configuration) do |record, attr_name, value|
-            record.errors.add(attr_name, configuration[:message]) if record.class.find(:first, :conditions => (record.new_record? ? ["#{attr_name} = ?", record.send(attr_name)] : ["#{attr_name} = ? AND #{record.class.primary_key} <> ?", record.send(attr_name), record.send(:id) ] ))
+          unless record.new_record?
+            condition_sql << " AND #{record.class.primary_key} <> ?"
+            condition_params << record.send(:id)
+          end
+          if record.class.find(:first, :conditions => [condition_sql, *condition_params])
+            record.errors.add(attr_name, configuration[:message])
           end
         end
       end
diff --git a/activerecord/test/abstract_unit.rb b/activerecord/test/abstract_unit.rb
index 5c585283f4..0626243521 100755
--- a/activerecord/test/abstract_unit.rb
+++ b/activerecord/test/abstract_unit.rb
@@ -8,6 +8,8 @@ require 'active_support/binding_of_caller'
 require 'active_support/breakpoint'
 require 'connection'
 
+QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name('type') unless Object.const_defined?(:QUOTED_TYPE)
+
 class Test::Unit::TestCase #:nodoc:
   self.fixture_path = File.dirname(__FILE__) + "/fixtures/"
   self.use_instantiated_fixtures = false
diff --git a/activerecord/test/associations_go_eager_test.rb b/activerecord/test/associations_go_eager_test.rb
index f05e5c6fe2..26dc4456df 100644
--- a/activerecord/test/associations_go_eager_test.rb
+++ b/activerecord/test/associations_go_eager_test.rb
@@ -21,7 +21,7 @@ class EagerAssociationTest < Test::Unit::TestCase
   end
 
   def test_loading_conditions_with_or
-    posts = authors(:david).posts.find(:all, :include => :comments, :conditions => "comments.body like 'Normal%' OR comments.type = 'SpecialComment'")
+    posts = authors(:david).posts.find(:all, :include => :comments, :conditions => "comments.body like 'Normal%' OR comments.#{QUOTED_TYPE} = 'SpecialComment'")
     assert_nil posts.detect { |p| p.author_id != authors(:david).id },
       "expected to find only david's posts"
   end
@@ -120,7 +120,7 @@ class EagerAssociationTest < Test::Unit::TestCase
     assert_raises(ArgumentError) do
       posts = authors(:david).posts.find(:all, 
         :include    => :comments, 
-        :conditions => "comments.body like 'Normal%' OR comments.type = 'SpecialComment'",
+        :conditions => "comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'",
         :limit      => 2
       )
     end
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 351dc4bcb3..ef2872e13c 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -357,7 +357,7 @@ class HasManyAssociationsTest < Test::Unit::TestCase
   def test_find_all
     firm = Firm.find_first
     assert_equal firm.clients, firm.clients.find_all
-    assert_equal 2, firm.clients.find(:all, :conditions => "type = 'Client'").length
+    assert_equal 2, firm.clients.find(:all, :conditions => "#{QUOTED_TYPE} = 'Client'").length
     assert_equal 1, firm.clients.find(:all, :conditions => "name = 'Summit'").length
   end
 
@@ -373,16 +373,16 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     firm = Firm.find_first
     client2 = Client.find(2)
     assert_equal firm.clients.first, firm.clients.find_first
-    assert_equal client2, firm.clients.find_first("type = 'Client'")
-    assert_equal client2, firm.clients.find(:first, :conditions => "type = 'Client'")
+    assert_equal client2, firm.clients.find_first("#{QUOTED_TYPE} = 'Client'")
+    assert_equal client2, firm.clients.find(:first, :conditions => "#{QUOTED_TYPE} = 'Client'")
   end
 
   def test_find_first_sanitized
     firm = Firm.find_first
     client2 = Client.find(2)
-    assert_equal client2, firm.clients.find_first(["type = ?", "Client"])
-    assert_equal client2, firm.clients.find(:first, :conditions => ['type = ?', 'Client'])
-    assert_equal client2, firm.clients.find(:first, :conditions => ['type = :type', { :type => 'Client' }])
+    assert_equal client2, firm.clients.find_first(["#{QUOTED_TYPE} = ?", "Client"])
+    assert_equal client2, firm.clients.find(:first, :conditions => ["#{QUOTED_TYPE} = ?", 'Client'])
+    assert_equal client2, firm.clients.find(:first, :conditions => ["#{QUOTED_TYPE} = :type", { :type => 'Client' }])
   end
 
   def test_find_in_collection
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 2db2dbf837..e66c90c7fe 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -505,7 +505,7 @@ class BasicsTest < Test::Unit::TestCase
     assert_nil topic.last_read
     assert_nil topic.approved
   end
-  
+
   def test_equality
     assert_equal Topic.find(1), Topic.find(2).parent
   end
@@ -1003,10 +1003,10 @@ class BasicsTest < Test::Unit::TestCase
   end
 
   def test_count_with_join
-    res = Post.count_by_sql "SELECT COUNT(*) FROM posts LEFT JOIN comments ON posts.id=comments.post_id WHERE posts.type = 'Post'"
+    res = Post.count_by_sql "SELECT COUNT(*) FROM posts LEFT JOIN comments ON posts.id=comments.post_id WHERE posts.#{QUOTED_TYPE} = 'Post'"
     res2 = res + 1
     assert_nothing_raised do
-      res2 = Post.count("posts.type = 'Post'",
+      res2 = Post.count("posts.#{QUOTED_TYPE} = 'Post'",
                         "LEFT JOIN comments ON posts.id=comments.post_id")
     end
     assert_equal res, res2
diff --git a/activerecord/test/deprecated_associations_test.rb b/activerecord/test/deprecated_associations_test.rb
index d96a587dc0..5772ab1335 100755
--- a/activerecord/test/deprecated_associations_test.rb
+++ b/activerecord/test/deprecated_associations_test.rb
@@ -312,7 +312,7 @@ class DeprecatedAssociationsTest < Test::Unit::TestCase
   end
 
   def test_has_many_find_all
-    assert_equal 2, Firm.find_first.find_all_in_clients("type = 'Client'").length
+    assert_equal 2, Firm.find_first.find_all_in_clients("#{QUOTED_TYPE} = 'Client'").length
     assert_equal 1, Firm.find_first.find_all_in_clients("name = 'Summit'").length
   end
 
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index 897657faeb..7e73eec7ad 100644
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -350,7 +350,7 @@ class FinderTest < Test::Unit::TestCase
   def test_find_by_id_with_conditions_with_or
     assert_nothing_raised do
       Post.find([1,2,3],
-        :conditions => "posts.id <= 3 OR posts.type = 'Post'")
+        :conditions => "posts.id <= 3 OR posts.#{QUOTED_TYPE} = 'Post'")
     end
   end
 
diff --git a/activerecord/test/fixtures/comment.rb b/activerecord/test/fixtures/comment.rb
index fd3a43ff5a..3eab263fa4 100644
--- a/activerecord/test/fixtures/comment.rb
+++ b/activerecord/test/fixtures/comment.rb
@@ -6,7 +6,7 @@ class Comment < ActiveRecord::Base
   end
   
   def self.search_by_type(q)
-    self.find(:all, :conditions => ['type = ?', q])
+    self.find(:all, :conditions => ["#{QUOTED_TYPE} = ?", q])
   end
 end
 
diff --git a/activerecord/test/fixtures/company.rb b/activerecord/test/fixtures/company.rb
index 4a0c96bdd3..6d33d4e1a8 100755
--- a/activerecord/test/fixtures/company.rb
+++ b/activerecord/test/fixtures/company.rb
@@ -7,7 +7,9 @@ end
 
 
 class Firm < Company
-  has_many :clients, :order => "id", :dependent => true, :counter_sql => "SELECT COUNT(*) FROM companies WHERE firm_id = 1 AND (type = 'Client' OR type = 'SpecialClient' OR type = 'VerySpecialClient' )"
+  has_many :clients, :order => "id", :dependent => true, :counter_sql =>
+      "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " +
+      "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )"
   has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC"
   has_many :clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id"
   has_many :dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => true
diff --git a/activerecord/test/fixtures_test.rb b/activerecord/test/fixtures_test.rb
index 41fc666945..813291aa18 100755
--- a/activerecord/test/fixtures_test.rb
+++ b/activerecord/test/fixtures_test.rb
@@ -51,42 +51,44 @@ class FixturesTest < Test::Unit::TestCase
     assert_nil(secondRow["author_email_address"])
   end
 
-  def test_inserts_with_pre_and_suffix
-    ActiveRecord::Base.connection.create_table :prefix_topics_suffix do |t|
-      t.column :title, :string
-      t.column :author_name, :string
-      t.column :author_email_address, :string
-      t.column :written_on, :datetime
-      t.column :bonus_time, :time
-      t.column :last_read, :date
-      t.column :content, :text
-      t.column :approved, :boolean, :default => true
-      t.column :replies_count, :integer, :default => 0
-      t.column :parent_id, :integer
-      t.column :type, :string, :limit => 50
-    end
+  if ActiveRecord::Base.connection.supports_migrations?
+    def test_inserts_with_pre_and_suffix
+      ActiveRecord::Base.connection.create_table :prefix_topics_suffix do |t|
+        t.column :title, :string
+        t.column :author_name, :string
+        t.column :author_email_address, :string
+        t.column :written_on, :datetime
+        t.column :bonus_time, :time
+        t.column :last_read, :date
+        t.column :content, :text
+        t.column :approved, :boolean, :default => true
+        t.column :replies_count, :integer, :default => 0
+        t.column :parent_id, :integer
+        t.column :type, :string, :limit => 50
+      end
 
-    # Store existing prefix/suffix
-    old_prefix = ActiveRecord::Base.table_name_prefix
-    old_suffix = ActiveRecord::Base.table_name_suffix
+      # Store existing prefix/suffix
+      old_prefix = ActiveRecord::Base.table_name_prefix
+      old_suffix = ActiveRecord::Base.table_name_suffix
 
-    # Set a prefix/suffix we can test against
-    ActiveRecord::Base.table_name_prefix = 'prefix_'
-    ActiveRecord::Base.table_name_suffix = '_suffix'
+      # Set a prefix/suffix we can test against
+      ActiveRecord::Base.table_name_prefix = 'prefix_'
+      ActiveRecord::Base.table_name_suffix = '_suffix'
 
-    topics = create_fixtures("topics")
+      topics = create_fixtures("topics")
 
-    # Restore prefix/suffix to its previous values
-    ActiveRecord::Base.table_name_prefix = old_prefix 
-    ActiveRecord::Base.table_name_suffix = old_suffix 
+      # Restore prefix/suffix to its previous values
+      ActiveRecord::Base.table_name_prefix = old_prefix 
+      ActiveRecord::Base.table_name_suffix = old_suffix 
 
-    firstRow = ActiveRecord::Base.connection.select_one("SELECT * FROM prefix_topics_suffix WHERE author_name = 'David'")
-    assert_equal("The First Topic", firstRow["title"])
+      firstRow = ActiveRecord::Base.connection.select_one("SELECT * FROM prefix_topics_suffix WHERE author_name = 'David'")
+      assert_equal("The First Topic", firstRow["title"])
 
-    secondRow = ActiveRecord::Base.connection.select_one("SELECT * FROM prefix_topics_suffix WHERE author_name = 'Mary'")
-    assert_nil(secondRow["author_email_address"])        
-  ensure
-    ActiveRecord::Base.connection.drop_table :prefix_topics_suffix rescue nil
+      secondRow = ActiveRecord::Base.connection.select_one("SELECT * FROM prefix_topics_suffix WHERE author_name = 'Mary'")
+      assert_nil(secondRow["author_email_address"])        
+    ensure
+      ActiveRecord::Base.connection.drop_table :prefix_topics_suffix rescue nil
+    end
   end
 
   def test_insert_with_datetime
diff --git a/activerecord/test/inheritance_test.rb b/activerecord/test/inheritance_test.rb
index 432aee7e0e..d726583126 100755
--- a/activerecord/test/inheritance_test.rb
+++ b/activerecord/test/inheritance_test.rb
@@ -11,7 +11,7 @@ class InheritanceTest < Test::Unit::TestCase
     if current_adapter?(:SQLServerAdapter)
       Company.connection.execute "SET IDENTITY_INSERT companies ON"
     end
-    Company.connection.insert "INSERT INTO companies (id, type, name) VALUES(100, 'bad_class!', 'Not happening')"
+    Company.connection.insert "INSERT INTO companies (id, #{QUOTED_TYPE}, name) VALUES(100, 'bad_class!', 'Not happening')"
 
     #We then need to turn it back Off before continuing.
     if current_adapter?(:SQLServerAdapter)