pushing caching and visitors down to the connection

7 files changed, 119 insertions, 93 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 3558ae3545..7ba67b8540 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -710,21 +710,21 @@ module ActiveRecord #:nodoc:
       # Returns an array of column objects for the table associated with this class.
       def columns
         if defined?(@primary_key)
-          connection_pool.primary_keys[table_name] ||= primary_key
+          connection.schema_cache.primary_keys[table_name] ||= primary_key
         end
 
-        connection_pool.columns[table_name]
+        connection.schema_cache.columns[table_name]
       end
 
       # Returns a hash of column objects for the table associated with this class.
       def columns_hash
-        connection_pool.columns_hash[table_name]
+        connection.schema_cache.columns_hash[table_name]
       end
 
       # Returns a hash where the keys are column names and the values are
       # default values when instantiating the AR object for this table.
       def column_defaults
-        connection_pool.column_defaults[table_name]
+        connection.schema_cache.column_defaults[table_name]
       end
 
       # Returns an array of column names as strings.
@@ -781,14 +781,14 @@ module ActiveRecord #:nodoc:
       def reset_column_information
         connection.clear_cache!
         undefine_attribute_methods
-        connection_pool.clear_table_cache!(table_name) if table_exists?
+        connection.schema_cache.clear_table_cache!(table_name) if table_exists?
 
         @column_names = @content_columns = @dynamic_methods_hash = @inheritance_column = nil
         @arel_engine = @relation = nil
       end
 
       def clear_cache! # :nodoc:
-        connection_pool.clear_cache!
+        connection.schema_cache.clear!
       end
 
       def attribute_method?(attribute)
@@ -1356,9 +1356,9 @@ MSG
           return nil if condition.blank?
 
           case condition
-            when Array; sanitize_sql_array(condition)
-            when Hash;  sanitize_sql_hash_for_conditions(condition, table_name)
-            else        condition
+          when Array; sanitize_sql_array(condition)
+          when Hash;  sanitize_sql_hash_for_conditions(condition, table_name)
+          else        condition
           end
         end
         alias_method :sanitize_sql, :sanitize_sql_for_conditions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 0ec0576795..f1781f057c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -2,6 +2,7 @@ require 'thread'
 require 'monitor'
 require 'set'
 require 'active_support/core_ext/module/synchronization'
+require 'active_support/core_ext/module/deprecation'
 
 module ActiveRecord
   # Raised when a connection could not be obtained within the connection
@@ -59,8 +60,6 @@ module ActiveRecord
     class ConnectionPool
       attr_accessor :automatic_reconnect
       attr_reader :spec, :connections
-      attr_reader :columns, :columns_hash, :primary_keys, :tables
-      attr_reader :column_defaults
 
       # Creates a new ConnectionPool object. +spec+ is a ConnectionSpecification
       # object which describes database connection information (e.g. adapter,
@@ -85,72 +84,7 @@ module ActiveRecord
         @connections         = []
         @checked_out         = []
         @automatic_reconnect = true
-        @tables              = {}
         @visitor             = nil
-
-        @columns     = Hash.new do |h, table_name|
-          h[table_name] = with_connection do |conn|
-
-            # Fetch a list of columns
-            conn.columns(table_name, "#{table_name} Columns").tap do |columns|
-
-              # set primary key information
-              columns.each do |column|
-                column.primary = column.name == primary_keys[table_name]
-              end
-            end
-          end
-        end
-
-        @columns_hash = Hash.new do |h, table_name|
-          h[table_name] = Hash[columns[table_name].map { |col|
-            [col.name, col]
-          }]
-        end
-
-        @column_defaults = Hash.new do |h, table_name|
-          h[table_name] = Hash[columns[table_name].map { |col|
-            [col.name, col.default]
-          }]
-        end
-
-        @primary_keys = Hash.new do |h, table_name|
-          h[table_name] = with_connection do |conn|
-            table_exists?(table_name) ? conn.primary_key(table_name) : 'id'
-          end
-        end
-      end
-
-      # A cached lookup for table existence.
-      def table_exists?(name)
-        return @tables[name] if @tables.key? name
-
-        with_connection do |conn|
-          conn.tables.each { |table| @tables[table] = true }
-          @tables[name] = conn.table_exists?(name) if !@tables.key?(name)
-        end
-
-        @tables[name]
-      end
-
-      # Clears out internal caches:
-      #
-      #   * columns
-      #   * columns_hash
-      #   * tables
-      def clear_cache!
-        @columns.clear
-        @columns_hash.clear
-        @column_defaults.clear
-        @tables.clear
-      end
-
-      # Clear out internal caches for table with +table_name+.
-      def clear_table_cache!(table_name)
-        @columns.delete table_name
-        @columns_hash.delete table_name
-        @column_defaults.delete table_name
-        @primary_keys.delete table_name
       end
 
       # Retrieve the connection associated with the current thread, or call
@@ -227,6 +161,35 @@ module ActiveRecord
         end
       end
 
+      def columns
+        with_connection do |c|
+          c.schema_cache.columns
+        end
+      end
+      deprecate :columns
+
+      def columns_hash
+        with_connection do |c|
+          c.schema_cache.columns_hash
+        end
+      end
+      deprecate :columns_hash
+
+      def primary_keys
+        raise
+        with_connection do |c|
+          c.schema_cache.primary_keys
+        end
+      end
+      deprecate :primary_keys
+
+      def clear_cache!
+        with_connection do |c|
+          c.schema_cache.clear!
+        end
+      end
+      deprecate :clear_cache!
+
       # Return any checked-out connections back to the pool by threads that
       # are no longer alive.
       def clear_stale_cached_connections!
@@ -301,16 +264,7 @@ module ActiveRecord
       private
 
       def new_connection
-        connection = ActiveRecord::Base.send(spec.adapter_method, spec.config)
-
-        # TODO: This is a bit icky, and in the long term we may want to change the method
-        #       signature for connections. Also, if we switch to have one visitor per
-        #       connection (and therefore per thread), we can get rid of the thread-local
-        #       variable in Arel::Visitors::ToSql.
-        @visitor ||= connection.class.visitor_for(self)
-        connection.visitor = @visitor
-
-        connection
+        ActiveRecord::Base.send(spec.adapter_method, spec.config)
       end
 
       def current_connection_id #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index c47bcfc406..cbb7423480 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -3,6 +3,7 @@ require 'bigdecimal'
 require 'bigdecimal/util'
 require 'active_support/core_ext/benchmark'
 require 'active_support/deprecation'
+require 'active_record/connection_adapters/schema_cache'
 
 module ActiveRecord
   module ConnectionAdapters # :nodoc:
@@ -51,6 +52,7 @@ module ActiveRecord
       define_callbacks :checkout, :checkin
 
       attr_accessor :visitor
+      attr_reader :schema_cache
 
       def initialize(connection, logger = nil) #:nodoc:
         @active = nil
@@ -60,6 +62,7 @@ module ActiveRecord
         @open_transactions = 0
         @instrumenter = ActiveSupport::Notifications.instrumenter
         @visitor = nil
+        @schema_cache = SchemaCache.new self
       end
 
       # Returns a visitor instance for this adaptor, which conforms to the Arel::ToSql interface
diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb
new file mode 100644
index 0000000000..b14b37ce89
--- /dev/null
+++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb
@@ -0,0 +1,72 @@
+module ActiveRecord
+  module ConnectionAdapters
+    class SchemaCache
+      attr_reader :columns, :columns_hash, :primary_keys, :tables
+      attr_reader :column_defaults
+      attr_reader :connection
+
+      def initialize(conn)
+        @connection = conn
+        @tables              = {}
+
+        @columns     = Hash.new do |h, table_name|
+          h[table_name] =
+            # Fetch a list of columns
+            conn.columns(table_name, "#{table_name} Columns").tap do |cs|
+              # set primary key information
+              cs.each do |column|
+                column.primary = column.name == primary_keys[table_name]
+              end
+            end
+        end
+
+        @columns_hash = Hash.new do |h, table_name|
+          h[table_name] = Hash[columns[table_name].map { |col|
+            [col.name, col]
+          }]
+        end
+
+        @column_defaults = Hash.new do |h, table_name|
+          h[table_name] = Hash[columns[table_name].map { |col|
+            [col.name, col.default]
+          }]
+        end
+
+        @primary_keys = Hash.new do |h, table_name|
+          h[table_name] = table_exists?(table_name) ?
+                          conn.primary_key(table_name) : 'id'
+        end
+      end
+
+      # A cached lookup for table existence.
+      def table_exists?(name)
+        return @tables[name] if @tables.key? name
+
+        connection.tables.each { |table| @tables[table] = true }
+        @tables[name] = connection.table_exists?(name) if !@tables.key?(name)
+
+        @tables[name]
+      end
+
+      # Clears out internal caches:
+      #
+      #   * columns
+      #   * columns_hash
+      #   * tables
+      def clear!
+        @columns.clear
+        @columns_hash.clear
+        @column_defaults.clear
+        @tables.clear
+      end
+
+      # Clear out internal caches for table with +table_name+.
+      def clear_table_cache!(table_name)
+        @columns.delete table_name
+        @columns_hash.delete table_name
+        @column_defaults.delete table_name
+        @primary_keys.delete table_name
+      end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index bc3804b3d9..c11f82a33f 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -89,10 +89,7 @@ module ActiveRecord
         @statements = StatementPool.new(@connection,
                                         config.fetch(:statement_limit) { 1000 })
         @config = config
-      end
-
-      def self.visitor_for(pool) # :nodoc:
-        Arel::Visitors::SQLite.new(pool)
+        @visitor = Arel::Visitors::SQLite.new self
       end
 
       def adapter_name #:nodoc:
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index af167dc59b..a789f48725 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -23,7 +23,7 @@ module ActiveRecord
             attribute.in(value.arel.ast)
           when Array, ActiveRecord::Associations::CollectionProxy
             values = value.to_a.map {|x| x.is_a?(ActiveRecord::Base) ? x.id : x}
-            ranges, values = values.partition {|value| value.is_a?(Range) || value.is_a?(Arel::Relation)}
+            ranges, values = values.partition {|v| v.is_a?(Range) || v.is_a?(Arel::Relation)}
 
             array_predicates = ranges.map {|range| attribute.in(range)}
 
diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb
index 76c37cc367..e3bbd06f7e 100644
--- a/activerecord/lib/active_record/session_store.rb
+++ b/activerecord/lib/active_record/session_store.rb
@@ -59,12 +59,12 @@ module ActiveRecord
       end
 
       def drop_table!
-        connection_pool.clear_table_cache!(table_name)
+        connection.schema_cache.clear_table_cache!(table_name)
         connection.drop_table table_name
       end
 
       def create_table!
-        connection_pool.clear_table_cache!(table_name)
+        connection.schema_cache.clear_table_cache!(table_name)
         connection.create_table(table_name) do |t|
           t.string session_id_column, :limit => 255
           t.text data_column_name