Skip to content

Commit

Permalink
feat: INSERT OR [IGNORE|UPDATE] (#332)
Browse files Browse the repository at this point in the history
* feat: INSERT OR [IGNORE|UPDATE]

Add support for `INSERT OR [IGNORE|UPDATE]` DML statements.

* chore: fix acceptance tests

* test: wait a bit for sessions to be deleted
  • Loading branch information
olavloite authored Dec 12, 2024
1 parent a34d521 commit 7378e6d
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 26 deletions.
29 changes: 22 additions & 7 deletions acceptance/cases/models/insert_all_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,24 @@ def test_insert_all
{ id: Author.next_sequence_value, name: "Carol" },
]

assert_raise(NotImplementedError) { Author.insert_all(values) }
Author.insert_all(values)

authors = Author.all.order(:name)

assert_equal "Alice", authors[0].name
assert_equal "Bob", authors[1].name
assert_equal "Carol", authors[2].name
end

def test_insert
value = { id: Author.next_sequence_value, name: "Alice" }

assert_raise(NotImplementedError) { Author.insert(value) }
Author.insert(value)

authors = Author.all.order(:name)

assert_equal 1, authors.length
assert_equal "Alice", authors[0].name
end

def test_insert_all!
Expand Down Expand Up @@ -136,12 +147,16 @@ def test_upsert_all_with_transaction
{ id: Author.next_sequence_value, name: "Carol" },
]

err = assert_raise(NotImplementedError) do
ActiveRecord::Base.transaction do
Author.upsert_all(values)
end
ActiveRecord::Base.transaction do
Author.upsert_all(values)
end
assert_match "Use upsert outside a transaction block", err.message

authors = Author.all.order(:name)

assert_equal 3, authors.length
assert_equal "Alice", authors[0].name
assert_equal "Bob", authors[1].name
assert_equal "Carol", authors[2].name
end

def test_upsert_all_with_buffered_mutation_transaction
Expand Down
2 changes: 2 additions & 0 deletions acceptance/cases/sessions/session_not_found_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ def delete_all_sessions
sessions.each do |session|
client.delete_session Google::Cloud::Spanner::V1::DeleteSessionRequest.new name: session.name
end
# Wait a bit to ensure that the sessions are really deleted.
sleep 5 unless ENV["SPANNER_EMULATOR_HOST"]
end

def test_single_read
Expand Down
12 changes: 7 additions & 5 deletions lib/active_record/connection_adapters/spanner_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,14 @@ def build_insert_sql insert
raise "ActiveRecordSpannerAdapter does not support insert_sql with buffered_mutations transaction."
end

if insert.skip_duplicates? || insert.update_duplicates?
raise NotImplementedError, "CloudSpanner does not support skip_duplicates and update_duplicates."
end

values_list, = insert.values_list
"INSERT #{insert.into} #{values_list}"
prefix = "INSERT"
if insert.update_duplicates?
prefix += " OR UPDATE"
elsif insert.skip_duplicates?
prefix += " OR IGNORE"
end
"#{prefix} #{insert.into} #{values_list}"
end

module TypeMapBuilder
Expand Down
15 changes: 8 additions & 7 deletions lib/activerecord_spanner_adapter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,13 @@ def self._upsert_record values, returning
_buffer_record values, :insert_or_update, returning
end

def self.insert_all _attributes, **_kwargs
raise NotImplementedError, "Cloud Spanner does not support skip_duplicates. Use insert! or upsert instead."
def self.insert_all attributes, returning: nil, **_kwargs
if active_transaction? && buffered_mutations?
raise NotImplementedError,
"Spanner does not support skip_duplicates for mutations. " \
"Use a transaction that uses DML, or use insert! or upsert instead."
end
super
end

def self.insert! attributes, returning: nil, **kwargs
Expand Down Expand Up @@ -169,11 +174,7 @@ def self.upsert attributes, returning: nil, **kwargs

def self.upsert_all attributes, returning: nil, unique_by: nil, **kwargs
return super unless spanner_adapter?
if active_transaction? && !buffered_mutations?
raise NotImplementedError, "Cloud Spanner does not support upsert using DML. " \
"Use upsert outside a transaction block or in a transaction " \
"block with isolation: :buffered_mutations"
end
return super if active_transaction? && !buffered_mutations?

# This might seem inefficient, but is actually not, as it is only buffering a mutation locally.
# The mutations will be sent as one batch when the transaction is committed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,18 @@ class SpannerActiveRecordMockServerTest < BaseSpannerMockServerTest
VERSION_7_1_0 = Gem::Version.create('7.1.0')

def test_insert
sql = "INSERT OR IGNORE INTO `singers` (`first_name`,`last_name`) VALUES ('Alice', 'Ecila')"
@mock.put_statement_result sql, StatementResult.new(1)

singer = { first_name: "Alice", last_name: "Ecila" }
Singer.insert(singer)

assert_raises(NotImplementedError) { Singer.insert(singer) }
execute_requests = @mock.requests.select { |req| req.is_a?(ExecuteSqlRequest) && req.sql == sql }
assert_equal 1, execute_requests.length
commit_requests = @mock.requests.select { |req| req.is_a?(CommitRequest) }
assert_equal 1, commit_requests.length
mutations = commit_requests[0].mutations
assert_equal 0, mutations.length
end

def test_insert_other_adapter
Expand Down Expand Up @@ -1149,12 +1158,16 @@ def _disable_query_logs current_query_transformers
end

def test_insert_all
sql = "INSERT OR IGNORE INTO `singers` (`id`,`first_name`,`last_name`) " +
"VALUES (1, 'Dave', 'Allison'), (2, 'Alice', 'Davidson'), (3, 'Rene', 'Henderson')"
@mock.put_statement_result sql, StatementResult.new(3)

values = [
{id: 1, first_name: "Dave", last_name: "Allison"},
{id: 2, first_name: "Alice", last_name: "Davidson"},
{id: 3, first_name: "Rene", last_name: "Henderson"},
]
assert_raises(NotImplementedError) { Singer.insert_all values }
Singer.insert_all values
end

def test_insert_all_bang_mutations
Expand Down Expand Up @@ -1224,17 +1237,24 @@ def test_upsert_all_mutations
end

def test_upsert_all_dml
sql = "INSERT OR UPDATE INTO `singers` (`id`,`first_name`,`last_name`) " +
"VALUES (1, 'Dave', 'Allison'), (2, 'Alice', 'Davidson'), (3, 'Rene', 'Henderson')"
@mock.put_statement_result sql, StatementResult.new(3)

values = [
{id: 1, first_name: "Dave", last_name: "Allison"},
{id: 2, first_name: "Alice", last_name: "Davidson"},
{id: 3, first_name: "Rene", last_name: "Henderson"},
]
err = assert_raises(NotImplementedError) do
Singer.transaction do
Singer.upsert_all values
end
Singer.transaction do
Singer.upsert_all values
end
assert_match "Use upsert outside a transaction block", err.message

commit_requests = @mock.requests.select { |req| req.is_a?(CommitRequest) }
assert_equal 1, commit_requests.length
assert_equal 0, commit_requests[0].mutations.length
execute_requests = @mock.requests.select { |req| req.is_a?(ExecuteSqlRequest) && req.sql == sql }
assert_equal 1, execute_requests.length
end

private
Expand Down

0 comments on commit 7378e6d

Please sign in to comment.