`pluck`でstrict_loading違反が検出されなかったバグを修正

CollectionProxy#pluckを呼び出した際にstrict_loading違反が検出されず、N+1クエリがサイレントに実行されていた問題が修正されました。合わせて、violates_strict_loading?がプリロード済みのアソシエーションでも誤って違反と判定していた問題も解消されています。

背景

strict_loading!を設定したリレーションでpluckを呼び出しても、StrictLoadingViolationErrorが発生しないというバグが#51524として報告されていました。to_aやload経由のアクセスでは正しくエラーが発生するにもかかわらず、pluckだけが例外を素通りさせていたのです。

問題を再現するコードは明快です。Post.all.strict_loading!で取得したレコードに対し、posts.first.comments.to_aはStrictLoadingViolationErrorを発生させます。一方、posts.first.comments.pluck(:id)は同じN+1クエリを発行するにもかかわらず、エラーも警告も発生させずにクエリを実行していました。

このバグが発生していた理由は、CollectionProxy#pluckがexec_queriesやload_targetを経由せず直接クエリを実行する独自のコードパスを持っており、そこにstrict_loadingのチェックが組み込まれていなかったためです。

技術的な変更

今回の修正は2つの独立した変更から構成されています。CollectionProxy#pluckへのチェック追加と、violates_strict_loading?のアクセス修飾子変更です。

activerecord/lib/active_record/associations/collection_proxy.rbのpluckメソッドに、違反チェックのガード節が追加されました。

変更前:

def pluck(*column_names)
  null_scope? ? scope.pluck(*column_names) : super
end

変更後:

def pluck(*column_names)
  if proxy_association.violates_strict_loading?
    Base.strict_loading_violation!(owner: proxy_association.owner.class, reflection: proxy_association.reflection)
  end

  null_scope? ? scope.pluck(*column_names) : super
end

これにより、クエリ実行前にviolates_strict_loading?を評価し、違反があればstrict_loading_violation!を呼び出してエラーまたはログ出力を行います。

もう一つの変更は、violates_strict_loading?メソッドのアクセス修飾子の変更です。このメソッドはactiverecord/lib/active_record/associations/association.rbのprivateセクションに定義されていましたが、CollectionProxy#pluckから直接呼び出す必要が生じたため、publicメソッドに昇格されました。合わせて、メソッドの先頭にreturn unless find_target?という条件が追加されています。

変更前:

# private セクション内
def violates_strict_loading?
  return if @skip_strict_loading

  return unless owner.validation_context.nil?

  return reflection.strict_loading? if reflection.options.key?(:strict_loading)

  owner.strict_loading? && !owner.strict_loading_n_plus_one_only?
end

変更後:

# public セクション内
def violates_strict_loading?
  return unless find_target?

  return if @skip_strict_loading

  return unless owner.validation_context.nil?

  return reflection.strict_loading? if reflection.options.key?(:strict_loading)

  owner.strict_loading? && !owner.strict_loading_n_plus_one_only?
end

追加されたreturn unless find_target?は、プリロード済みのアソシエーションに対してviolates_strict_loading?がtrueを返してしまう問題を防ぐためのものです。find_target?はアソシエーションのターゲットがまだロードされていない場合にtrueを返すメソッドであり、既にプリロードされている場合はfalseを返します。これによりDeveloper.preload(:audit_logs).first.audit_logs.pluck(:id)のような正当な使い方が誤ってエラーを発生させることがなくなります。

設計判断

violates_strict_loading?をpublicに昇格させてCollectionProxyから直接呼び出す設計が採用されています。

代替案として、exec_queriesや別の共通パスにチェックを集約する方法も考えられますが、pluckはexec_queriesを経由しないコードパスを持つため、そのアプローチは取られませんでした。関連する#50389がクエリメソッド全般における同種の問題をexec_queriesへのチェック追加で対処しているのと対照的に、本PRはpluck固有のコードパスに対してピンポイントの修正を施しています。

find_target?によるガードは、プリロードとstrict_loadingを組み合わせた際の正当なユースケースを壊さないための重要な配慮です。strict_loadingの目的はN+1クエリの検出であり、プリロード済みのデータへのアクセスはその定義からして問題ではありません。このロジックをviolates_strict_loading?自体に組み込むことで、今後同メソッドを呼び出す箇所が増えた場合でも一貫した判定が保証されます。

まとめ

この修正により、pluckを使用した際のstrict_loading違反が他のアクセスパターンと同様に検出されるようになり、N+1クエリ検出の一貫性が確保されました。violates_strict_loading?へのfind_target?ガードの追加は、プリロード済みアソシエーションへの正当なアクセスを保護しつつ、strict_loadingの本来の目的を正確に実装し直したものといえます。