SQLite: count(distinct) queries supported in >= 3.2.6, fix calculations workaround, remove count(distinct) query rewrite, cleanup test connection scripts. Closes #6544.

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

2 files changed, 53 insertions, 61 deletions

diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index 9069b9723e..54bedf1294 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -6,7 +6,7 @@ module ActiveRecord
     end
 
     module ClassMethods
-      # Count operates using three different approaches. 
+      # Count operates using three different approaches.
       #
       # * Count all: By not passing any parameters to count, it will return a count of all the rows for the model.
       # * Count by conditions or joins: This API has been deprecated and will be removed in Rails 2.0
@@ -36,7 +36,7 @@ module ActiveRecord
       # Examples for count with options:
       #   Person.count(:conditions => "age > 26")
       #   Person.count(:conditions => "age > 26 AND job.salary > 60000", :include => :job) # because of the named association, it finds the DISTINCT count using LEFT OUTER JOIN.
-      #   Person.count(:conditions => "age > 26 AND job.salary > 60000", :joins => "LEFT JOIN jobs on jobs.person_id = person.id") # finds the number of rows matching the conditions and joins. 
+      #   Person.count(:conditions => "age > 26 AND job.salary > 60000", :joins => "LEFT JOIN jobs on jobs.person_id = person.id") # finds the number of rows matching the conditions and joins.
       #   Person.count('id', :conditions => "age > 26") # Performs a COUNT(id)
       #   Person.count(:all, :conditions => "age > 26") # Performs a COUNT(*) (:all is an alias for '*')
       #
@@ -74,11 +74,11 @@ module ActiveRecord
       end
 
       # This calculates aggregate values in the given column:  Methods for count, sum, average, minimum, and maximum have been added as shortcuts.
-      # Options such as :conditions, :order, :group, :having, and :joins can be passed to customize the query.  
+      # Options such as :conditions, :order, :group, :having, and :joins can be passed to customize the query.
       #
       # There are two basic forms of output:
       #   * Single aggregate value: The single value is type cast to Fixnum for COUNT, Float for AVG, and the given column's type for everything else.
-      #   * Grouped values: This returns an ordered hash of the values and groups them by the :group option.  It takes either a column name, or the name 
+      #   * Grouped values: This returns an ordered hash of the values and groups them by the :group option.  It takes either a column name, or the name
       #     of a belongs_to association.
       #
       #       values = Person.maximum(:age, :group => 'last_name')
@@ -157,22 +157,29 @@ module ActiveRecord
         end
 
         def construct_calculation_sql(operation, column_name, options) #:nodoc:
+          operation = operation.to_s.downcase
+          options = options.symbolize_keys
+
           scope           = scope(:find)
           merged_includes = merge_includes(scope ? scope[:include] : [], options[:include])
           aggregate_alias = column_alias_for(operation, column_name)
-          use_workaround  = !Base.connection.supports_count_distinct? && options[:distinct] && operation.to_s.downcase == 'count'
-          join_dependency = nil
 
-          if merged_includes.any? && operation.to_s.downcase == 'count'
-            options[:distinct] = true
-            column_name = options[:select] || [table_name, primary_key] * '.'
+          if operation == 'count'
+            if merged_includes.any?
+              options[:distinct] = true
+              column_name = options[:select] || [table_name, primary_key] * '.'
+            end
+
+            if options[:distinct]
+              use_workaround = !connection.supports_count_distinct?
+            end
           end
 
-          sql  = "SELECT #{operation}(#{'DISTINCT ' if options[:distinct]}#{column_name}) AS #{aggregate_alias}"
+          sql = "SELECT #{operation}(#{'DISTINCT ' if options[:distinct]}#{column_name}) AS #{aggregate_alias}"
 
           # A (slower) workaround if we're using a backend, like sqlite, that doesn't support COUNT DISTINCT.
           sql = "SELECT COUNT(*) AS #{aggregate_alias}" if use_workaround
-          
+
           sql << ", #{options[:group_field]} AS #{options[:group_alias]}" if options[:group]
           sql << " FROM (SELECT DISTINCT #{column_name}" if use_workaround
           sql << " FROM #{table_name} "
@@ -185,17 +192,17 @@ module ActiveRecord
           add_limited_ids_condition!(sql, options, join_dependency) if join_dependency && !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
 
           if options[:group]
-            group_key = Base.connection.adapter_name == 'FrontBase' ?  :group_alias : :group_field
+            group_key = connection.adapter_name == 'FrontBase' ?  :group_alias : :group_field
             sql << " GROUP BY #{options[group_key]} "
           end
 
           if options[:group] && options[:having]
             # FrontBase requires identifiers in the HAVING clause and chokes on function calls
-            if Base.connection.adapter_name == 'FrontBase'
+            if connection.adapter_name == 'FrontBase'
               options[:having].downcase!
               options[:having].gsub!(/#{operation}\s*\(\s*#{column_name}\s*\)/, aggregate_alias)
             end
-              
+
             sql << " HAVING #{options[:having]} "
           end
 
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
index a9d00e8b3f..43d513e293 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -73,16 +73,16 @@ module ActiveRecord
               when "\0" then "%00"
               when "%"  then "%25"
             end
-          end                
+          end
         end
-        
+
         def binary_to_string(value)
           value.gsub(/%00|%25/) do |b|
             case b
               when "%00" then "\0"
               when "%25" then "%"
             end
-          end                
+          end
         end
       end
     end
@@ -101,9 +101,9 @@ module ActiveRecord
       def supports_migrations? #:nodoc:
         true
       end
-      
+
       def supports_count_distinct? #:nodoc:
-        false
+        sqlite_version >= '3.2.6'
       end
 
       def native_database_types #:nodoc:
@@ -178,7 +178,7 @@ module ActiveRecord
       def begin_db_transaction #:nodoc:
         catch_schema_changes { @connection.transaction }
       end
-      
+
       def commit_db_transaction #:nodoc:
         catch_schema_changes { @connection.commit }
       end
@@ -225,7 +225,7 @@ module ActiveRecord
       def remove_index(table_name, options={}) #:nodoc:
         execute "DROP INDEX #{quote_column_name(index_name(table_name, options))}"
       end
-      
+
       def rename_table(name, new_name)
         execute "ALTER TABLE #{name} RENAME TO #{new_name}"
       end
@@ -235,13 +235,13 @@ module ActiveRecord
         # See last paragraph on http://www.sqlite.org/lang_altertable.html
         execute "VACUUM"
       end
-      
+
       def remove_column(table_name, column_name) #:nodoc:
         alter_table(table_name) do |definition|
           definition.columns.delete(definition[column_name])
         end
       end
-      
+
       def change_column_default(table_name, column_name, default) #:nodoc:
         alter_table(table_name) do |definition|
           definition[column_name].default = default
@@ -261,7 +261,7 @@ module ActiveRecord
       def rename_column(table_name, column_name, new_column_name) #:nodoc:
         alter_table(table_name, :rename => {column_name => new_column_name})
       end
-          
+
 
       protected
         def table_structure(table_name)
@@ -269,23 +269,23 @@ module ActiveRecord
             raise ActiveRecord::StatementInvalid if structure.empty?
           end
         end
-        
+
         def alter_table(table_name, options = {}) #:nodoc:
           altered_table_name = "altered_#{table_name}"
           caller = lambda {|definition| yield definition if block_given?}
 
           transaction do
-            move_table(table_name, altered_table_name, 
+            move_table(table_name, altered_table_name,
               options.merge(:temporary => true))
             move_table(altered_table_name, table_name, &caller)
           end
         end
-        
+
         def move_table(from, to, options = {}, &block) #:nodoc:
           copy_table(from, to, options, &block)
           drop_table(from)
         end
-        
+
         def copy_table(from, to, options = {}) #:nodoc:
           create_table(to, options) do |@definition|
             columns(from).each do |column|
@@ -294,20 +294,20 @@ module ActiveRecord
                  options[:rename][column.name.to_sym] ||
                  column.name) : column.name
 
-              @definition.column(column_name, column.type, 
+              @definition.column(column_name, column.type,
                 :limit => column.limit, :default => column.default,
                 :null => column.null)
             end
             @definition.primary_key(primary_key(from))
             yield @definition if block_given?
           end
-          
+
           copy_table_indexes(from, to)
-          copy_table_contents(from, to, 
-            @definition.columns.map {|column| column.name}, 
+          copy_table_contents(from, to,
+            @definition.columns.map {|column| column.name},
             options[:rename] || {})
         end
-        
+
         def copy_table_indexes(from, to) #:nodoc:
           indexes(from).each do |index|
             name = index.name
@@ -316,27 +316,27 @@ module ActiveRecord
             elsif from == "altered_#{to}"
               name = name[5..-1]
             end
-            
+
             # index name can't be the same
             opts = { :name => name.gsub(/_(#{from})_/, "_#{to}_") }
             opts[:unique] = true if index.unique
             add_index(to, index.columns, opts)
           end
         end
-        
+
         def copy_table_contents(from, to, columns, rename = {}) #:nodoc:
           column_mappings = Hash[*columns.map {|name| [name, name]}.flatten]
           rename.inject(column_mappings) {|map, a| map[a.last] = a.first; map}
           from_columns = columns(from).collect {|col| col.name}
           columns = columns.find_all{|col| from_columns.include?(column_mappings[col])}
           @connection.execute "SELECT * FROM #{from}" do |row|
-            sql = "INSERT INTO #{to} ("+columns*','+") VALUES ("            
+            sql = "INSERT INTO #{to} ("+columns*','+") VALUES ("
             sql << columns.map {|col| quote row[column_mappings[col]]} * ', '
             sql << ')'
             @connection.execute sql
           end
         end
-        
+
         def catch_schema_changes
           return yield
         rescue ActiveRecord::StatementInvalid => exception
@@ -347,41 +347,26 @@ module ActiveRecord
             raise
           end
         end
+
+        def sqlite_version
+          @sqlite_version ||= select_value('select sqlite_version(*)')
+        end
     end
-    
+
     class SQLite2Adapter < SQLiteAdapter # :nodoc:
-      # SQLite 2 does not support COUNT(DISTINCT) queries:
-      #
-      #   select COUNT(DISTINCT ArtistID) from CDs;    
-      #
-      # In order to get  the number of artists we execute the following statement
-      # 
-      #   SELECT COUNT(ArtistID) FROM (SELECT DISTINCT ArtistID FROM CDs);
-      def execute(sql, name = nil) #:nodoc:
-        super(rewrite_count_distinct_queries(sql), name)
-      end
-      
-      def rewrite_count_distinct_queries(sql)
-        if sql =~ /count\(distinct ([^\)]+)\)( AS \w+)? (.*)/i
-          distinct_column = $1
-          distinct_query  = $3
-          column_name     = distinct_column.split('.').last
-          "SELECT COUNT(#{column_name}) FROM (SELECT DISTINCT #{distinct_column} #{distinct_query})"
-        else
-          sql
-        end
+      def supports_count_distinct? #:nodoc:
+        false
       end
-      
+
       def rename_table(name, new_name)
         move_table(name, new_name)
       end
-      
+
       def add_column(table_name, column_name, type, options = {}) #:nodoc:
         alter_table(table_name) do |definition|
           definition.column(column_name, type, options)
         end
       end
-      
     end
 
     class DeprecatedSQLiteAdapter < SQLite2Adapter # :nodoc: