Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Aditya Sharad <[email protected]>
  • Loading branch information
yoff and adityasharad authored Feb 6, 2025
1 parent 5feb401 commit 7af8fa7
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
9 changes: 5 additions & 4 deletions ruby/ql/src/queries/performance/CouldBeHoisted.qhelp
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@

<overview>
<p>
When a Rails ActiveRecord query is executed in a loop, it is potentially an n+1 problem.
This query identifies situations where an ActiveRecord query execution could be pulled out of a loop.
When a database query operation, for example a call to a query method in the Rails `ActiveRecord::Relation` class, is executed in a loop, this can lead to a performance issue known as an "n+1 query problem".
The database query will be executed in each iteration of the loop.
Performance can usually be improved by performing a single database query outside of a loop, which retrieves all the required objects in a single operation.
</p>
</overview>
<recommendation>
<p>If possible, pull the query out of the loop, thus replacing the many calls with a single one.
<p>If possible, pull the database query out of the loop and rewrite it to retrieve all the required objects. This replaces multiple database operations with a single one.
</p>
</recommendation>
<example>
<p>The following (suboptimal) example code queries the User object in each iteration of the loop:</p>
<sample src="examples/straight_loop.rb" />
<p>To improve the performance, we instead query the User object once outside the loop, gathereing all necessary information:</p>
<p>To improve the performance, we instead query the <code>User</code> object once outside the loop, gathering all necessary information in a single query:</p>
<sample src="examples/preload.rb" />
</example>
</qhelp>
2 changes: 1 addition & 1 deletion ruby/ql/src/queries/performance/CouldBeHoisted.ql
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,4 @@ where
// Only report calls that are likely to be expensive
call instanceof ActiveRecordModelFinderCall and
not call.getMethodName() in ["new", "create"]
select call, "This call happens inside $@, and could be hoisted.", loop, "this loop"
select call, "This call to a database query operation happens inside $@, and could be hoisted to a single call outside the loop.", loop, "this loop"

0 comments on commit 7af8fa7

Please sign in to comment.