【Rails Code Reading 3回目】Read all records at once when using update with an array #40682
Rails Code Reading 3回目です。 今回は題目のPRを読んでみました。
このPRは今から2ヶ月以上前に作成されたのにも関わらず、誰からもコメントが付いてなく放置気味だったので、好奇心から読んでみました。
内容としては、ActiveRecord::Modelを継承したPersonモデルがあった時、
people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } }
Person.update(people.keys, people.values)
で引数に渡されたpeopleで一括更新できるメソッドがあります。
これに関して、
現状、このupdateの内部実装はこのようになっています。
id.map { |one_id| find(one_id) }.each_with_index { |object, idx|
object.update(attributes[idx])
}
idというハッシュをループして、一つ一つupdateしています。
これに対して、投稿されたパッチでは、idのうち重複したキーを排除した上でこの更新処理を行うようにしています。 これによって、sqlのupdateクエリが走る回数が減るというのが、実装者の目的だそうです。
例えば、[1,1,3]でidが与えられた場合、[1,3]と重複を削除してからupdateします。
とここまで読んできて、おや？？と思いました。
この場合、id.uniqで十分では？ 著者は、
merged_attributes = Hash.new { |hash, key| hash[key] = {} }
id.size.times { |i| merged_attributes[id[i]].merge!(attributes[i]) }
と、第二引数のattributesの重複を削除していますが、そもそもidの要素で検索してupdateしているので、重複削除するならidの方ではないかと思った。
これは実装があんまりよくないから、マージされないパターンなのかなあ。
