Skip to content

Commit 9b4a4b0

Browse files
quentindemetzquentindemetz
authored
fix when changes dont intersect with columns (#3)
1 parent e137368 commit 9b4a4b0

File tree

2 files changed

+78
-58
lines changed

2 files changed

+78
-58
lines changed

‎lib/batch_update.rb

+3-3
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def batch_update(entries, columns:, batch_size: 100, validate: true)
1919
columns = (Array.wrap(columns).map(&:to_s) + %w[updated_at]).uniq
2020
# + ::Auditable::ActiveRecord::AUDIT_LOG_UPDATED_COLUMNS).uniq
2121

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

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

45+
private
46+
4547
def batch_update_statements(entries, update_on: :id, batch_size: 100)
4648
update_on = Array.wrap(update_on).map(&:to_s)
4749

@@ -63,8 +65,6 @@ def batch_update_statements(entries, update_on: :id, batch_size: 100)
6365
end
6466
end
6567

66-
private
67-
6868
def cte_table
6969
@cte_table ||= Arel::Table.new('batch_updates')
7070
end

‎spec/batch_update_spec.rb

+75-55
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
describe BatchUpdate do
66
describe '#batch_update_statements' do
7-
subject(:sql_queries) { Cat.batch_update_statements(cats, **kwargs) }
7+
subject(:sql_queries) { Cat.__send__(:batch_update_statements, cats, **kwargs) }
88

99
let(:cats) { [] }
1010
let(:kwargs) { {} }
@@ -100,40 +100,45 @@
100100
end
101101
end
102102

103-
it 'batches queries accordingly' do
104-
query1 = <<~SQL.squish
105-
WITH "batch_updates" (id, name) AS (
106-
VALUES (CAST(1 AS INTEGER), CAST('foo' AS varchar))
107-
)
108-
UPDATE "cats"
109-
SET
110-
"name" = "batch_updates"."name"
111-
FROM "batch_updates"
112-
WHERE
113-
"cats"."id" = "batch_updates"."id"
114-
SQL
115-
116-
query2 = <<~SQL.squish
117-
WITH "batch_updates" (id, name) AS (
118-
VALUES (CAST(2 AS INTEGER), CAST('bar' AS varchar))
119-
)
120-
UPDATE "cats"
121-
SET
122-
"name" = "batch_updates"."name"
123-
FROM "batch_updates"
124-
WHERE
125-
"cats"."id" = "batch_updates"."id"
126-
SQL
127-
128-
expect(
129-
Cat.batch_update_statements(
130-
[
131-
{ id: 1, name: 'foo' },
132-
{ id: 2, name: 'bar' }
133-
],
134-
batch_size: 1
135-
)
136-
).to contain_exactly(query1, query2)
103+
context 'with a custom batch_size' do
104+
let(:cats) do
105+
[
106+
{ id: 1, name: 'foo' },
107+
{ id: 2, name: 'bar' }
108+
]
109+
end
110+
111+
let(:kwargs) do
112+
{ batch_size: 1 }
113+
end
114+
115+
it 'batches queries accordingly' do
116+
query1 = <<~SQL.squish
117+
WITH "batch_updates" (id, name) AS (
118+
VALUES (CAST(1 AS INTEGER), CAST('foo' AS varchar))
119+
)
120+
UPDATE "cats"
121+
SET
122+
"name" = "batch_updates"."name"
123+
FROM "batch_updates"
124+
WHERE
125+
"cats"."id" = "batch_updates"."id"
126+
SQL
127+
128+
query2 = <<~SQL.squish
129+
WITH "batch_updates" (id, name) AS (
130+
VALUES (CAST(2 AS INTEGER), CAST('bar' AS varchar))
131+
)
132+
UPDATE "cats"
133+
SET
134+
"name" = "batch_updates"."name"
135+
FROM "batch_updates"
136+
WHERE
137+
"cats"."id" = "batch_updates"."id"
138+
SQL
139+
140+
expect(sql_queries).to contain_exactly(query1, query2)
141+
end
137142
end
138143
end
139144

@@ -150,7 +155,7 @@
150155

151156
it 'does not insert a new record' do
152157
expect do
153-
Cat.batch_update([non_existing_record], columns: :all, validate: false)
158+
Cat.batch_update([non_existing_record], columns: :all)
154159
end.to execute_queries(/UPDATE/)
155160
.and not_change(Cat, :count)
156161
.and(not_change { Cat.exists?(id: non_existing_record.id) })
@@ -166,7 +171,7 @@
166171
cat1.name = 'can I haz cheeseburger pls'
167172
cat2.name = 'O\'Sullivans cuba libre'
168173
cat2.birthday = '2024-01-01'
169-
Cat.batch_update([cat1, cat2], columns: :all, validate: false)
174+
Cat.batch_update([cat1, cat2], columns: :all)
170175
end.to execute_queries(
171176
/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"/,
172177
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/
@@ -182,7 +187,7 @@
182187
expect do
183188
cat1.bitcoin_address = 'abc'
184189

185-
Cat.batch_update([cat1], columns: %i[bitcoin_address], validate: false)
190+
Cat.batch_update([cat1], columns: %i[bitcoin_address])
186191
end.to execute_queries(
187192
/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"/
188193
).and change { cat1.reload.bitcoin_address }.to('abc')
@@ -195,7 +200,7 @@
195200
it 'does not remove it' do
196201
expect do
197202
cat1.name = 'can I haz cheeseburger pls'
198-
Cat.batch_update([cat1], columns: :all, validate: false)
203+
Cat.batch_update([cat1], columns: :all)
199204
end.to change { cat1.reload.name }.to('can I haz cheeseburger pls')
200205
end
201206
end
@@ -206,27 +211,42 @@
206211
it 'does not break the query' do
207212
expect do
208213
cat1.name = 'La Rose Blanche \\'
209-
Cat.batch_update([cat1], columns: :all, validate: false)
214+
Cat.batch_update([cat1], columns: :all)
210215
end.to change { cat1.reload.name }.to('La Rose Blanche \\')
211216
end
212217
end
213218
end
214219

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

219-
it 'does not change them' do
220-
expect do
221-
cat1.name = 'can I haz cheeseburger pls'
222-
cat2.name = 'O\'Sullivans cuba libre'
223-
cat2.birthday = Date.new(2024, 1, 1)
224-
Cat.batch_update([cat1, cat2], columns: %w[name], validate: false)
225-
end.to execute_queries(
226-
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/
227-
).and change { cat1.reload.name }.to('can I haz cheeseburger pls')
228-
.and change { cat2.reload.name }.to('O\'Sullivans cuba libre')
229-
.and(not_change { cat2.reload.birthday })
225+
it 'does not change them' do
226+
expect do
227+
cat1.name = 'can I haz cheeseburger pls'
228+
cat2.name = 'O\'Sullivans cuba libre'
229+
cat2.birthday = Date.new(2024, 1, 1)
230+
Cat.batch_update([cat1, cat2], columns: %w[name])
231+
end.to execute_queries(
232+
/WITH "batch_updates" .* AS \( VALUES.* UPDATE "cats" SET "name" = "batch_updates"."name", "updated_at" = "batch_updates"."updated_at" FROM "batch_updates"/
233+
).and(change { cat1.reload.name }.to('can I haz cheeseburger pls'))
234+
.and(change { cat2.reload.name }.to('O\'Sullivans cuba libre'))
235+
.and(not_change { cat2.reload.birthday })
236+
end
237+
end
238+
239+
describe 'when no changes overlap with the columns kwarg' do
240+
let!(:cat) { Cat.create!(name: 'Felix', birthday: Date.new(2010, 1, 1)) }
241+
242+
it 'does not run any query and make no changes' do
243+
expect do
244+
cat.name = 'can I haz cheeseburger pls'
245+
Cat.batch_update([cat], columns: %w[birthday])
246+
end.to execute_no_queries
247+
.and(not_change { cat.reload.name })
248+
.and(not_change { cat.reload.birthday })
249+
end
230250
end
231251
end
232252

@@ -259,7 +279,7 @@
259279
before_import_cat = cat1
260280
before_import_cat.name = 'Yoda'
261281

262-
Cat.batch_update([before_import_cat], columns: %i[name], validate: false)
282+
Cat.batch_update([before_import_cat], columns: %i[name])
263283

264284
after_import_cat = Cat.find(before_import_cat.id)
265285
expect(before_import_cat.name).to eq('Yoda')

0 commit comments

Comments
 (0)