Skip to content

Commit

Permalink
fix when changes dont intersect with columns (#3)
Browse files Browse the repository at this point in the history
  • Loading branch information
@quentindemetz
quentindemetz authored Oct 17, 2024
1 parent e137368 commit 9b4a4b0
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 58 deletions.
6 changes: 3 additions & 3 deletions lib/batch_update.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def batch_update(entries, columns:, batch_size: 100, validate: true)
columns = (Array.wrap(columns).map(&:to_s) + %w[updated_at]).uniq
# + ::Auditable::ActiveRecord::AUDIT_LOG_UPDATED_COLUMNS).uniq

entries = entries.select(&:changed?)
entries = entries.select { columns.intersect?(_1.changed) }
entries.each { _1.updated_at = Time.current } if has_attribute?('updated_at')

entries.each(&:validate!) if validate
Expand All @@ -42,6 +42,8 @@ def batch_update(entries, columns:, batch_size: 100, validate: true)
updated_count
end

private

def batch_update_statements(entries, update_on: :id, batch_size: 100)
update_on = Array.wrap(update_on).map(&:to_s)

Expand All @@ -63,8 +65,6 @@ def batch_update_statements(entries, update_on: :id, batch_size: 100)
end
end

private

def cte_table
@cte_table ||= Arel::Table.new('batch_updates')
end
Expand Down
130 changes: 75 additions & 55 deletions spec/batch_update_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

describe BatchUpdate do
describe '#batch_update_statements' do
subject(:sql_queries) { Cat.batch_update_statements(cats, **kwargs) }
subject(:sql_queries) { Cat.__send__(:batch_update_statements, cats, **kwargs) }

let(:cats) { [] }
let(:kwargs) { {} }
Expand Down Expand Up @@ -100,40 +100,45 @@
end
end

it 'batches queries accordingly' do
query1 = <<~SQL.squish
WITH "batch_updates" (id, name) AS (
VALUES (CAST(1 AS INTEGER), CAST('foo' AS varchar))
)
UPDATE "cats"
SET
"name" = "batch_updates"."name"
FROM "batch_updates"
WHERE
"cats"."id" = "batch_updates"."id"
SQL

query2 = <<~SQL.squish
WITH "batch_updates" (id, name) AS (
VALUES (CAST(2 AS INTEGER), CAST('bar' AS varchar))
)
UPDATE "cats"
SET
"name" = "batch_updates"."name"
FROM "batch_updates"
WHERE
"cats"."id" = "batch_updates"."id"
SQL

expect(
Cat.batch_update_statements(
[
{ id: 1, name: 'foo' },
{ id: 2, name: 'bar' }
],
batch_size: 1
)
).to contain_exactly(query1, query2)
context 'with a custom batch_size' do
let(:cats) do
[
{ id: 1, name: 'foo' },
{ id: 2, name: 'bar' }
]
end

let(:kwargs) do
{ batch_size: 1 }
end

it 'batches queries accordingly' do
query1 = <<~SQL.squish
WITH "batch_updates" (id, name) AS (
VALUES (CAST(1 AS INTEGER), CAST('foo' AS varchar))
)
UPDATE "cats"
SET
"name" = "batch_updates"."name"
FROM "batch_updates"
WHERE
"cats"."id" = "batch_updates"."id"
SQL

query2 = <<~SQL.squish
WITH "batch_updates" (id, name) AS (
VALUES (CAST(2 AS INTEGER), CAST('bar' AS varchar))
)
UPDATE "cats"
SET
"name" = "batch_updates"."name"
FROM "batch_updates"
WHERE
"cats"."id" = "batch_updates"."id"
SQL

expect(sql_queries).to contain_exactly(query1, query2)
end
end
end

Expand All @@ -150,7 +155,7 @@

it 'does not insert a new record' do
expect do
Cat.batch_update([non_existing_record], columns: :all, validate: false)
Cat.batch_update([non_existing_record], columns: :all)
end.to execute_queries(/UPDATE/)
.and not_change(Cat, :count)
.and(not_change { Cat.exists?(id: non_existing_record.id) })
Expand All @@ -166,7 +171,7 @@
cat1.name = 'can I haz cheeseburger pls'
cat2.name = 'O\'Sullivans cuba libre'
cat2.birthday = '2024-01-01'
Cat.batch_update([cat1, cat2], columns: :all, validate: false)
Cat.batch_update([cat1, cat2], columns: :all)
end.to execute_queries(
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "birthday" = "batch_updates"."birthday", "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/,
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/
Expand All @@ -182,7 +187,7 @@
expect do
cat1.bitcoin_address = 'abc'

Cat.batch_update([cat1], columns: %i[bitcoin_address], validate: false)
Cat.batch_update([cat1], columns: %i[bitcoin_address])
end.to execute_queries(
/WITH "batch_updates" \(bitcoin_address, id, updated_at\) AS \( VALUES \(CAST\(.* AS varchar\), CAST\(\d* AS INTEGER\), CAST\(.* AS datetime\(6\)\)\) \) UPDATE "cats" SET "bitcoin_address" = "batch_updates"."bitcoin_address", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates" WHERE "cats"."id" = "batch_updates"."id"/
).and change { cat1.reload.bitcoin_address }.to('abc')
Expand All @@ -195,7 +200,7 @@
it 'does not remove it' do
expect do
cat1.name = 'can I haz cheeseburger pls'
Cat.batch_update([cat1], columns: :all, validate: false)
Cat.batch_update([cat1], columns: :all)
end.to change { cat1.reload.name }.to('can I haz cheeseburger pls')
end
end
Expand All @@ -206,27 +211,42 @@
it 'does not break the query' do
expect do
cat1.name = 'La Rose Blanche \\'
Cat.batch_update([cat1], columns: :all, validate: false)
Cat.batch_update([cat1], columns: :all)
end.to change { cat1.reload.name }.to('La Rose Blanche \\')
end
end
end

describe 'when some fields changed, but are not included in only kwarg' do
let!(:cat1) { Cat.create!(name: 'Felix', birthday: Date.new(2010, 1, 1)) }
let!(:cat2) { Cat.create!(name: 'Garfield', birthday: Date.new(2011, 2, 2)) }
context 'when the columns kwargs is specified' do
describe 'when not all changes are included the columns kwarg' do
let!(:cat1) { Cat.create!(name: 'Felix', birthday: Date.new(2010, 1, 1)) }
let!(:cat2) { Cat.create!(name: 'Garfield', birthday: Date.new(2011, 2, 2)) }

it 'does not change them' do
expect do
cat1.name = 'can I haz cheeseburger pls'
cat2.name = 'O\'Sullivans cuba libre'
cat2.birthday = Date.new(2024, 1, 1)
Cat.batch_update([cat1, cat2], columns: %w[name], validate: false)
end.to execute_queries(
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/
).and change { cat1.reload.name }.to('can I haz cheeseburger pls')
.and change { cat2.reload.name }.to('O\'Sullivans cuba libre')
.and(not_change { cat2.reload.birthday })
it 'does not change them' do
expect do
cat1.name = 'can I haz cheeseburger pls'
cat2.name = 'O\'Sullivans cuba libre'
cat2.birthday = Date.new(2024, 1, 1)
Cat.batch_update([cat1, cat2], columns: %w[name])
end.to execute_queries(
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/
).and(change { cat1.reload.name }.to('can I haz cheeseburger pls'))
.and(change { cat2.reload.name }.to('O\'Sullivans cuba libre'))
.and(not_change { cat2.reload.birthday })
end
end

describe 'when no changes overlap with the columns kwarg' do
let!(:cat) { Cat.create!(name: 'Felix', birthday: Date.new(2010, 1, 1)) }

it 'does not run any query and make no changes' do
expect do
cat.name = 'can I haz cheeseburger pls'
Cat.batch_update([cat], columns: %w[birthday])
end.to execute_no_queries
.and(not_change { cat.reload.name })
.and(not_change { cat.reload.birthday })
end
end
end

Expand Down Expand Up @@ -259,7 +279,7 @@
before_import_cat = cat1
before_import_cat.name = 'Yoda'

Cat.batch_update([before_import_cat], columns: %i[name], validate: false)
Cat.batch_update([before_import_cat], columns: %i[name])

after_import_cat = Cat.find(before_import_cat.id)
expect(before_import_cat.name).to eq('Yoda')
Expand Down

0 comments on commit 9b4a4b0

Please sign in to comment.