Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

SQLite adapters now support DDL transactions [#2080 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
  • Loading branch information...
commit ac3848201dfd7400708d3ccae0acb9388318fb99 1 parent 5b025a1
@smathy smathy authored lifo committed
View
55 activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -72,6 +72,18 @@ def binary_to_string(value)
#
# * <tt>:database</tt> - 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:
View
26 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)
View
15 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
Please sign in to comment.
Something went wrong with that request. Please try again.