Make it the responsibility of the connection to hold onto an ARel visitor for generating SQL. This improves the code architecture generally, and solves some problems with marshalling. Adapter authors please take note: you now need to define an Adapter.visitor_for method, but it degrades gracefully with a deprecation warning for now.

5 files changed, 19 insertions, 24 deletions

diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index f1023ed7ef..94497e37c7 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -157,4 +157,14 @@ class AdapterTest < ActiveRecord::TestCase
       end
     end
   end
+
+  def test_deprecated_visitor_for
+    visitor_klass = Class.new(Arel::Visitors::ToSql)
+    Arel::Visitors::VISITORS['fuuu'] = visitor_klass
+    pool = stub(:spec => stub(:config => { :adapter => 'fuuu' }))
+    visitor = assert_deprecated {
+      ActiveRecord::ConnectionAdapters::AbstractAdapter.visitor_for(pool)
+    }
+    assert visitor.is_a?(visitor_klass)
+  end
 end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 12101c1683..0a9340aea1 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -144,25 +144,6 @@ class BasicsTest < ActiveRecord::TestCase
     end
   end
 
-  def test_use_table_engine_for_quoting_where
-    relation = Topic.where(Topic.arel_table[:id].eq(1))
-    engine = relation.table.engine
-
-    fakepool = Class.new(Struct.new(:spec)) {
-      def with_connection; yield self; end
-      def connection_pool; self; end
-      def table_exists?(name); false; end
-      def quote_table_name(*args); raise "lol quote_table_name"; end
-    }
-
-    relation.table.engine = fakepool.new(engine.connection_pool.spec)
-
-    error = assert_raises(RuntimeError) { relation.to_a }
-    assert_match('lol', error.message)
-  ensure
-    relation.table.engine = engine
-  end
-
   def test_preserving_time_objects
     assert_kind_of(
       Time, Topic.find(1).bonus_time,
diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb
index f92f4e62c5..8a0f453127 100644
--- a/activerecord/test/cases/connection_pool_test.rb
+++ b/activerecord/test/cases/connection_pool_test.rb
@@ -135,6 +135,10 @@ module ActiveRecord
           pool.with_connection
         end
       end
+
+      def test_pool_sets_connection_visitor
+        assert @pool.connection.visitor.is_a?(Arel::Visitors::ToSql)
+      end
     end
   end
 end
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index a0cb5dbdc5..0ab4f30363 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -14,7 +14,7 @@ class MethodScopingTest < ActiveRecord::TestCase
 
   def test_set_conditions
     Developer.send(:with_scope, :find => { :conditions => 'just a test...' }) do
-      assert_match '(just a test...)', Developer.scoped.arel.to_sql
+      assert_match '(just a test...)', Developer.scoped.to_sql
     end
   end
 
@@ -274,7 +274,7 @@ class NestedScopingTest < ActiveRecord::TestCase
     Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do
       Developer.send(:with_scope, :find => { :limit => 10 }) do
         devs = Developer.scoped
-        assert_match '(salary = 80000)', devs.arel.to_sql
+        assert_match '(salary = 80000)', devs.to_sql
         assert_equal 10, devs.taken
       end
     end
@@ -308,7 +308,7 @@ class NestedScopingTest < ActiveRecord::TestCase
     Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do
       Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do
         devs = Developer.scoped
-        assert_match "(name = 'David') AND (salary = 80000)", devs.arel.to_sql
+        assert_match "(name = 'David') AND (salary = 80000)", devs.to_sql
         assert_equal(1, Developer.count)
       end
       Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do
@@ -321,7 +321,7 @@ class NestedScopingTest < ActiveRecord::TestCase
     Developer.send(:with_scope, :find => { :conditions => 'salary = 80000', :limit => 10 }) do
       Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do
         devs = Developer.scoped
-        assert_match "(salary = 80000) AND (name = 'David')", devs.arel.to_sql
+        assert_match "(salary = 80000) AND (name = 'David')", devs.to_sql
         assert_equal 10, devs.taken
       end
     end
diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb
index f2d177d834..673aff403f 100644
--- a/activerecord/test/cases/relation_scoping_test.rb
+++ b/activerecord/test/cases/relation_scoping_test.rb
@@ -170,7 +170,7 @@ class NestedRelationScopingTest < ActiveRecord::TestCase
     Developer.where('salary = 80000').scoping do
       Developer.limit(10).scoping do
         devs = Developer.scoped
-        assert_match '(salary = 80000)', devs.arel.to_sql
+        assert_match '(salary = 80000)', devs.to_sql
         assert_equal 10, devs.taken
       end
     end