Passing tests for 3.1.0.rc6.

Author: metaskills

RUNNING_UNIT_TESTS

@@ -93,6 +93,11 @@ variable.
 
 = Current Expected Failures
 
-* Misc Date/Time object instance matches when using ODBC mode.
+Note that there are a few tests that we just did not even try to pass and hence coerced 
+into passing. The biggest are a collection found in our relations_test_sqlserver.rb file.
+If you run into problems and think you can make the Arel visitor pass these test, patches 
+are very much welcome.
+
+  * Misc Date/Time object instance matches when using ODBC mode.
 
 

activerecord-sqlserver-adapter.gemspec

@@ -2,7 +2,7 @@
 Gem::Specification.new do |s|
   s.platform      = Gem::Platform::RUBY
   s.name          = "activerecord-sqlserver-adapter"
-  s.version       = "3.1.0.rc5"
+  s.version       = "3.1.0.rc6"
   s.summary       = "SQL Server 2005 and 2008 Adapter For ActiveRecord."
   s.description   = "SQL Server 2005 and 2008 Adapter For ActiveRecord"
 
@@ -16,6 +16,6 @@ Gem::Specification.new do |s|
   s.rdoc_options.concat ['--main',  'README.rdoc']
   s.rubyforge_project = 'activerecord-sqlserver-adapter'
 
-  s.add_dependency('activerecord', '~> 3.1.0.rc5')
+  s.add_dependency('activerecord', '~> 3.1.0.rc6')
 end
 

lib/active_record/connection_adapters/sqlserver/quoting.rb

@@ -38,7 +38,7 @@ def quote_string(string)
 
         def quote_column_name(name)
           @sqlserver_quoted_column_and_table_names[name] ||= 
-            name.to_s.split('.').map{ |n| n =~ /^\[.*\]$/ ? n : "[#{n}]" }.join('.')
+            name.to_s.split('.').map{ |n| n =~ /^\[.*\]$/ ? n : "[#{n.to_s.gsub(']', ']]')}]" }.join('.')
         end
 
         def quote_table_name(name)

lib/active_record/connection_adapters/sqlserver_adapter.rb

@@ -159,7 +159,7 @@ class SQLServerAdapter < AbstractAdapter
       include Sqlserver::Errors
 
       ADAPTER_NAME                = 'SQLServer'.freeze
-      VERSION                     = '3.1.0.rc5'.freeze
+      VERSION                     = '3.1.0.rc6'.freeze
       DATABASE_VERSION_REGEXP     = /Microsoft SQL Server\s+"?(\d{4}|\w+)"?/
       SUPPORTED_VERSIONS          = [2005,2008,2010,2011].freeze
 
@@ -171,6 +171,13 @@ class SQLServerAdapter < AbstractAdapter
 
       self.enable_default_unicode_types = true
 
+      class << self
+        
+        def visitor_for(pool)
+          Arel::Visitors::SQLServer.new(pool)
+        end
+        
+      end
 
       def initialize(logger,config)
         @connection_options = config

lib/arel/visitors/sqlserver.rb

@@ -21,14 +21,16 @@ def eql?(other)
   end
 
   class SelectManager < Arel::TreeManager
-
+    
+    AR_CA_SQLSA_NAME = 'ActiveRecord::ConnectionAdapters::SQLServerAdapter'.freeze
+    
     # Getting real Ordering objects is very important for us. We need to be able to call #uniq on
     # a colleciton of them reliably as well as using their true object attributes to mutate them
     # to grouping objects for the inner sql during a select statment with an offset/rownumber. So this
     # is here till ActiveRecord & ARel does this for us instead of using SqlLiteral objects.
     alias :order_without_sqlserver :order
     def order(*expr)
-      return order_without_sqlserver(*expr) unless Arel::Visitors::SQLServer === @visitor
+      return order_without_sqlserver(*expr) unless engine_activerecord_sqlserver_adapter?
       @ast.orders.concat(expr.map{ |x|
         case x
         when Arel::Attributes::Attribute
@@ -55,7 +57,7 @@ def order(*expr)
     # custom string hints down. See the visit_Arel_Nodes_LockWithSQLServer delegation method.
     alias :lock_without_sqlserver :lock
     def lock(locking=true)
-      if Arel::Visitors::SQLServer === @visitor
+      if engine_activerecord_sqlserver_adapter?
         case locking
         when true
           locking = Arel.sql('WITH(HOLDLOCK, ROWLOCK)')
@@ -69,7 +71,13 @@ def lock(locking=true)
         lock_without_sqlserver(locking)
       end
     end
-
+    
+    private
+    
+    def engine_activerecord_sqlserver_adapter?
+      @engine.connection && @engine.connection.class.name == AR_CA_SQLSA_NAME
+    end
+    
   end
 
   module Visitors
@@ -117,7 +125,7 @@ def visit_Arel_Nodes_Ordering(o)
       end
 
       def visit_Arel_Nodes_Bin(o)
-        "#{visit o.expr} #{@engine.connection.cs_equality_operator}"
+        "#{visit o.expr} #{@connection.cs_equality_operator}"
       end
 
       # SQLServer ToSql/Visitor (Additions)
@@ -189,7 +197,7 @@ def visit_Arel_Nodes_SelectStatementForComplexCount(o)
 
       def source_with_lock_for_select_statement(o)
         core = o.cores.first
-        source = visit(core.source).strip if core.source
+        source = "FROM #{visit(core.source).strip}" if core.source
         if source && o.lock
           lock = visit o.lock
           index = source.match(/FROM [\w\[\]\.]+/)[0].length
@@ -272,6 +280,14 @@ def complex_count_sql?(o)
           o.limit &&
           !join_in_select_statement?(o)
       end
+      
+      def select_primary_key_sql?(o)
+        core = o.cores.first
+        return false if core.projections.size != 1
+        p = core.projections.first
+        t = table_from_select_statement(o)
+        Arel::Attributes::Attribute === p && t.primary_key && t.primary_key.name == p.name
+      end
 
       def find_and_fix_uncorrelated_joins_in_select_statement(o)
         core = o.cores.first
@@ -315,6 +331,8 @@ def rowtable_projections(o)
           end
         elsif function_select_statement?(o)
           [Arel.star]
+        elsif select_primary_key_sql?(o)
+          [Arel.sql("[__rnt].#{quote_column_name(core.projections.first.name)}")]
         else
           core.projections.map do |x|
             if x.respond_to?(:relation)

test/cases/base_test_sqlserver.rb

@@ -0,0 +1,14 @@
+require 'cases/sqlserver_helper'
+
+class BasicsTest < ActiveRecord::TestCase
+  
+  COERCED_TESTS = [:test_column_names_are_escaped]
+  
+  include SqlserverCoercedTest
+  
+  def test_coerced_column_names_are_escaped
+    assert_equal "[foo]]bar]", ActiveRecord::Base.connection.quote_column_name("foo]bar")
+  end
+  
+end
+

test/cases/column_test_sqlserver.rb

@@ -1,8 +1,6 @@
 require 'cases/sqlserver_helper'
 require 'models/binary'
 
-class SqlServerEdgeSchema < ActiveRecord::Base; end;
-
 class ColumnTestSqlserver < ActiveRecord::TestCase
 
   def setup

test/cases/relations_test_sqlserver.rb

@@ -27,8 +27,7 @@ class RelationTest < ActiveRecord::TestCase
   COERCED_TESTS = [
     :test_finding_with_cross_table_order_and_limit,
     :test_finding_with_complex_order_and_limit,
-    :test_finding_with_complex_order,
-    :test_count_explicit_columns
+    :test_finding_with_complex_order
   ]
 
   include SqlserverCoercedTest

test/cases/specific_schema_test_sqlserver.rb

@@ -1,17 +1,5 @@
 require 'cases/sqlserver_helper'
 
-class NoPkData < ActiveRecord::Base ; self.table_name = 'no_pk_data' ; end
-class StringDefault < ActiveRecord::Base; end;
-class SqlServerEdgeSchema < ActiveRecord::Base; end;
-class SqlServerEdgeSchema < ActiveRecord::Base
-  attr_accessor :new_id_setting
-  before_create :set_new_id
-  protected
-  def set_new_id
-    self[:guid_newid] ||= connection.newid_function if new_id_setting
-  end
-end
-
 class SpecificSchemaTestSqlserver < ActiveRecord::TestCase
 
   should 'be able to complex count tables with no primary key' do
@@ -59,6 +47,17 @@ class SpecificSchemaTestSqlserver < ActiveRecord::TestCase
       @edge_class = SqlServerEdgeSchema
     end
 
+    context 'With special quoted column' do
+
+      should 'work as normal' do
+        @edge_class.delete_all
+        r = @edge_class.create! 'crazy]]quote' => 'crazyqoute'
+        assert @edge_class.columns_hash['crazy]]quote']
+        assert_equal r, @edge_class.first(:conditions => {'crazy]]quote' => 'crazyqoute'})
+      end
+
+    end
+    
     context 'with description column' do
 
       setup do

test/cases/sqlserver_helper.rb

@@ -42,6 +42,16 @@ class SqlServerQuotedView1 < ActiveRecord::Base ; self.table_name = 'quoted-view
 class SqlServerQuotedView2 < ActiveRecord::Base ; self.table_name = 'quoted-view2' ; end
 class SqlServerUnicode < ActiveRecord::Base ; end
 class SqlServerString < ActiveRecord::Base ; end
+class NoPkData < ActiveRecord::Base ; self.table_name = 'no_pk_data' ; end
+class StringDefault < ActiveRecord::Base; end
+class SqlServerEdgeSchema < ActiveRecord::Base
+  attr_accessor :new_id_setting
+  before_create :set_new_id
+  protected
+  def set_new_id
+    self[:guid_newid] ||= connection.newid_function if new_id_setting
+  end
+end
 class SqlServerChronic < ActiveRecord::Base
   coerce_sqlserver_date :date
   coerce_sqlserver_time :time