diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 6077ddcdb6585..afd6472db8046 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -72,6 +72,18 @@ def binary_to_string(value) # # * :database - Path to the database file. class SQLiteAdapter < AbstractAdapter + class Version + include Comparable + + def initialize(version_string) + @version = version_string.split('.').map(&:to_i) + end + + def <=>(version_string) + @version <=> version_string.split('.').map(&:to_i) + end + end + def initialize(connection, logger, config) super(connection, logger) @config = config @@ -81,6 +93,10 @@ def adapter_name #:nodoc: 'SQLite' end + def supports_ddl_transactions? + sqlite_version >= '2.0.0' + end + def supports_migrations? #:nodoc: true end @@ -88,6 +104,10 @@ def supports_migrations? #:nodoc: def requires_reloading? true end + + def supports_add_column? + sqlite_version >= '3.1.6' + end def disconnect! super @@ -169,7 +189,6 @@ def rollback_db_transaction #:nodoc: catch_schema_changes { @connection.rollback } end - # SELECT ... FOR UPDATE is redundant since the table is locked. def add_lock!(sql, options) #:nodoc: sql @@ -218,14 +237,20 @@ def rename_table(name, new_name) execute "ALTER TABLE #{name} RENAME TO #{new_name}" end + # See: http://www.sqlite.org/lang_altertable.html + # SQLite has an additional restriction on the ALTER TABLE statement + def valid_alter_table_options( type, options) + type.to_sym != :primary_key + end + def add_column(table_name, column_name, type, options = {}) #:nodoc: - if @connection.respond_to?(:transaction_active?) && @connection.transaction_active? - raise StatementInvalid, 'Cannot add columns to a SQLite database while inside a transaction' + if supports_add_column? && valid_alter_table_options( type, options ) + super(table_name, column_name, type, options) + else + alter_table(table_name) do |definition| + definition.column(column_name, type, options) + end end - - super(table_name, column_name, type, options) - # See last paragraph on http://www.sqlite.org/lang_altertable.html - execute "VACUUM" end def remove_column(table_name, *column_names) #:nodoc: @@ -385,7 +410,7 @@ def catch_schema_changes end def sqlite_version - @sqlite_version ||= select_value('select sqlite_version(*)') + @sqlite_version ||= SQLiteAdapter::Version.new(select_value('select sqlite_version(*)')) end def default_primary_key_type @@ -398,23 +423,9 @@ def default_primary_key_type end class SQLite2Adapter < SQLiteAdapter # :nodoc: - 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: - if @connection.respond_to?(:transaction_active?) && @connection.transaction_active? - raise StatementInvalid, 'Cannot add columns to a SQLite database while inside a transaction' - end - - alter_table(table_name) do |definition| - definition.column(column_name, type, options) - end - end end class DeprecatedSQLiteAdapter < SQLite2Adapter # :nodoc: diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 50d039ec77f5d..16861f21b17e5 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -93,6 +93,30 @@ def test_add_index end end + def testing_table_with_only_foo_attribute + Person.connection.create_table :testings, :id => false do |t| + t.column :foo, :string + end + + yield Person.connection + ensure + Person.connection.drop_table :testings rescue nil + end + protected :testing_table_with_only_foo_attribute + + def test_create_table_without_id + testing_table_with_only_foo_attribute do |connection| + assert_equal connection.columns(:testings).size, 1 + end + end + + def test_add_column_with_primary_key_attribute + testing_table_with_only_foo_attribute do |connection| + assert_nothing_raised { connection.add_column :testings, :id, :primary_key } + assert_equal connection.columns(:testings).size, 2 + end + end + def test_create_table_adds_id Person.connection.create_table :testings do |t| t.column :foo, :string @@ -928,7 +952,7 @@ def test_migrator_double_down assert_equal(0, ActiveRecord::Migrator.current_version) end - if current_adapter?(:PostgreSQLAdapter) + if ActiveRecord::Base.connection.supports_ddl_transactions? def test_migrator_one_up_with_exception_and_rollback assert !Person.column_methods_hash.include?(:last_name) diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index f9ae5115fc8f0..f6533b539640a 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -349,7 +349,7 @@ def test_open_transactions_count_is_reset_to_zero_if_no_transaction_active end end - def test_sqlite_add_column_in_transaction_raises_statement_invalid + def test_sqlite_add_column_in_transaction return true unless current_adapter?(:SQLite3Adapter, :SQLiteAdapter) # Test first if column creation/deletion works correctly when no @@ -368,10 +368,15 @@ def test_sqlite_add_column_in_transaction_raises_statement_invalid assert !Topic.column_names.include?('stuff') end - # Test now inside a transaction: add_column should raise a StatementInvalid - Topic.transaction do - assert_raise(ActiveRecord::StatementInvalid) { Topic.connection.add_column('topics', 'stuff', :string) } - raise ActiveRecord::Rollback + if Topic.connection.supports_ddl_transactions? + assert_nothing_raised do + Topic.transaction { Topic.connection.add_column('topics', 'stuff', :string) } + end + else + Topic.transaction do + assert_raise(ActiveRecord::StatementInvalid) { Topic.connection.add_column('topics', 'stuff', :string) } + raise ActiveRecord::Rollback + end end end