Skip to content
Browse files

SQLite: count(distinct) queries supported in >= 3.2.6, fix calculatio…

…ns 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
  • Loading branch information...
1 parent ceb3859 commit b6171e71050dda9e3b4ebfccd440a180283667ce @jeremy jeremy committed
View
4 activerecord/CHANGELOG
@@ -1,5 +1,9 @@
*SVN*
+* SQLite: fix calculations workaround, remove count(distinct) query rewrite, cleanup test connection scripts. [Jeremy Kemper]
+
+* SQLite: count(distinct) queries supported in >= 3.2.6. #6544 [Bob Silva]
+
* Dynamically generate reader methods for serialized attributes. #6362 [Stefan Kaes]
* Deprecation: object transactions warning. [Jeremy Kemper]
View
2 activerecord/Rakefile
@@ -23,7 +23,7 @@ PKG_FILES = FileList[
desc "Default Task"
-task :default => [ :test_mysql, :test_sqlite, :test_postgresql ]
+task :default => [ :test_mysql, :test_sqlite, :test_sqlite3, :test_postgresql ]
# Run the unit tests
View
35 activerecord/lib/active_record/calculations.rb
@@ -6,7 +6,7 @@ def self.included(base)
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 ClassMethods
# 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 @@ def sum(column_name, options = {})
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 @@ def construct_count_options_from_legacy_args(*args)
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 @@ def construct_calculation_sql(operation, column_name, options) #:nodoc:
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
View
79 activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -73,16 +73,16 @@ def string_to_binary(value)
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 @@ def adapter_name #:nodoc:
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 @@ def select_one(sql, name = nil) #:nodoc:
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 @@ def primary_key(table_name) #:nodoc:
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 @@ def add_column(table_name, column_name, type, options = {}) #:nodoc:
# 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 @@ def change_column(table_name, column_name, type, options = {}) #:nodoc:
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 @@ def table_structure(table_name)
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 @@ def copy_table(from, to, options = {}) #:nodoc:
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 @@ def copy_table_indexes(from, to) #:nodoc:
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 @@ def catch_schema_changes
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:
View
18 activerecord/test/base_test.rb
@@ -1219,15 +1219,17 @@ def test_count_with_join
assert_equal res4, res5
- res6 = Post.count_by_sql "SELECT COUNT(DISTINCT p.id) FROM posts p, comments co WHERE p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id"
- res7 = nil
- assert_nothing_raised do
- res7 = Post.count(:conditions => "p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id",
- :joins => "p, comments co",
- :select => "p.id",
- :distinct => true)
+ unless current_adapter?(:SQLite2Adapter, :DeprecatedSQLiteAdapter)
+ res6 = Post.count_by_sql "SELECT COUNT(DISTINCT p.id) FROM posts p, comments co WHERE p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id"
+ res7 = nil
+ assert_nothing_raised do
+ res7 = Post.count(:conditions => "p.#{QUOTED_TYPE} = 'Post' AND p.id=co.post_id",
+ :joins => "p, comments co",
+ :select => "p.id",
+ :distinct => true)
+ end
+ assert_equal res6, res7
end
- assert_equal res6, res7
end
def test_clear_association_cache_stored
View
17 activerecord/test/connections/native_sqlite/connection.rb
@@ -10,25 +10,16 @@ class SqliteError < StandardError
sqlite_test_db = "#{BASE_DIR}/fixture_database.sqlite"
sqlite_test_db2 = "#{BASE_DIR}/fixture_database_2.sqlite"
-def make_connection(clazz, db_file, db_definitions_file)
+def make_connection(clazz, db_file)
ActiveRecord::Base.configurations = { clazz.name => { :adapter => 'sqlite', :database => db_file } }
unless File.exist?(db_file)
puts "SQLite database not found at #{db_file}. Rebuilding it."
sqlite_command = %Q{sqlite #{db_file} "create table a (a integer); drop table a;"}
puts "Executing '#{sqlite_command}'"
raise SqliteError.new("Seems that there is no sqlite executable available") unless system(sqlite_command)
- clazz.establish_connection(clazz.name)
- script = File.read("#{BASE_DIR}/db_definitions/#{db_definitions_file}")
- # SQLite-Ruby has problems with semi-colon separated commands, so split and execute one at a time
- script.split(';').each do
- |command|
- clazz.connection.execute(command) unless command.strip.empty?
- end
- else
- clazz.establish_connection(clazz.name)
end
+ clazz.establish_connection(clazz.name)
end
-make_connection(ActiveRecord::Base, sqlite_test_db, 'sqlite.sql')
-make_connection(Course, sqlite_test_db2, 'sqlite2.sql')
-load(File.join(BASE_DIR, 'db_definitions', 'schema.rb'))
+make_connection(ActiveRecord::Base, sqlite_test_db)
+make_connection(Course, sqlite_test_db2)
View
17 activerecord/test/connections/native_sqlite3/connection.rb
@@ -10,25 +10,16 @@ class SqliteError < StandardError
sqlite_test_db = "#{BASE_DIR}/fixture_database.sqlite3"
sqlite_test_db2 = "#{BASE_DIR}/fixture_database_2.sqlite3"
-def make_connection(clazz, db_file, db_definitions_file)
+def make_connection(clazz, db_file)
ActiveRecord::Base.configurations = { clazz.name => { :adapter => 'sqlite3', :database => db_file, :timeout => 5000 } }
unless File.exist?(db_file)
puts "SQLite3 database not found at #{db_file}. Rebuilding it."
sqlite_command = %Q{sqlite3 #{db_file} "create table a (a integer); drop table a;"}
puts "Executing '#{sqlite_command}'"
raise SqliteError.new("Seems that there is no sqlite3 executable available") unless system(sqlite_command)
- clazz.establish_connection(clazz.name)
- script = File.read("#{BASE_DIR}/db_definitions/#{db_definitions_file}")
- # SQLite-Ruby has problems with semi-colon separated commands, so split and execute one at a time
- script.split(';').each do
- |command|
- clazz.connection.execute(command) unless command.strip.empty?
- end
- else
- clazz.establish_connection(clazz.name)
end
+ clazz.establish_connection(clazz.name)
end
-make_connection(ActiveRecord::Base, sqlite_test_db, 'sqlite.sql')
-make_connection(Course, sqlite_test_db2, 'sqlite2.sql')
-load(File.join(BASE_DIR, 'db_definitions', 'schema.rb'))
+make_connection(ActiveRecord::Base, sqlite_test_db)
+make_connection(Course, sqlite_test_db2)
View
7 activerecord/test/migration_test.rb
@@ -165,10 +165,9 @@ def test_create_table_with_limits
Person.connection.drop_table :testings rescue nil
end
- # SQL Server and Sybase will not allow you to add a NOT NULL column
- # to a table without specifying a default value, so the
- # following test must be skipped
- unless current_adapter?(:SQLServerAdapter, :SybaseAdapter)
+ # SQL Server, Sybase, and SQLite3 will not allow you to add a NOT NULL
+ # column to a table without a default value.
+ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :SQLiteAdapter)
def test_add_column_not_null_without_default
Person.connection.create_table :testings do |t|
t.column :foo, :string

0 comments on commit b6171e7

Please sign in to comment.
Something went wrong with that request. Please try again.