Commit
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
module ActiveRecord | ||
module Explain # :nodoc | ||
# logging_query_plan calls could appear nested in the call stack. In | ||
# particular this happens when a relation fetches its records, since | ||
# that results in find_by_sql calls downwards. | ||
# | ||
# This flag allows nested calls to detect this situation and bypass | ||
# it, thus preventing repeated EXPLAINs. | ||
LOGGING_QUERY_PLAN = :logging_query_plan | ||
|
||
# If auto explain is enabled, this method triggers EXPLAIN logging for the | ||
# queries triggered by the block if it takes more than the threshold as a | ||
# whole. That is, the threshold is not checked against each individual | ||
# query, but against the duration of the entire block. This approach is | ||
# convenient for relations. | ||
def logging_query_plan(&block) | ||
if (t = auto_explain_threshold_in_seconds) && !Thread.current[LOGGING_QUERY_PLAN] | ||
begin | ||
Thread.current[LOGGING_QUERY_PLAN] = true | ||
start = Time.now | ||
result, sqls, binds = collecting_sqls_for_explain(&block) | ||
logger.warn(exec_explain(sqls, binds)) if Time.now - start > t | ||
result | ||
ensure | ||
Thread.current[LOGGING_QUERY_PLAN] = false | ||
end | ||
else | ||
block.call | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong. |
||
end | ||
end | ||
|
||
# SCHEMA queries cannot be EXPLAINed, also we do not want to run EXPLAIN on | ||
# our own EXPLAINs now matter how loopingly beautiful that would be. | ||
SKIP_EXPLAIN_FOR = %(SCHEMA EXPLAIN) | ||
def ignore_explain_notification?(payload) | ||
payload[:exception] || SKIP_EXPLAIN_FOR.include?(payload[:name]) | ||
end | ||
|
||
# Collects all queries executed while the passed block runs. Returns an | ||
# array with three elements, the result of the block, the strings with the | ||
# queries, and their respective bindings. | ||
def collecting_sqls_for_explain(&block) | ||
sqls = [] | ||
binds = [] | ||
callback = lambda do |*args| | ||
payload = args.last | ||
unless ignore_explain_notification?(payload) | ||
sqls << payload[:sql] | ||
binds << payload[:binds] | ||
end | ||
end | ||
|
||
result = nil | ||
ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do | ||
result = block.call | ||
end | ||
|
||
[result, sqls, binds] | ||
end | ||
|
||
# Makes the adapter execute EXPLAIN for the given queries and bindings. | ||
# Returns a formatted string ready to be logged. | ||
def exec_explain(sqls, binds) | ||
sqls.zip(binds).map do |sql, bind| | ||
[].tap do |msg| | ||
msg << "EXPLAIN for: #{sql}" | ||
unless bind.empty? | ||
bind_msg = bind.map {|col, val| [col.name, val]}.inspect | ||
msg.last << " #{bind_msg}" | ||
end | ||
msg << connection.explain(sql, bind) | ||
end.join("\n") | ||
end.join("\n") | ||
end | ||
end | ||
end |
37 comments
on commit 0306f82
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AWESOME! I have been recommending tools to achieve the same result to almost every single team I coach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kudos! Awesome++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most awesome! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feature bloat alert! Why not put this in a gem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍺 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+++++++++++++++++++++++++++++1. Awesome commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I tend to agree this is pretty badass, I also tend to agree with @dkastner that this feature smells like bloat. Interested to see what Core team decides is good for everyone, +1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkastner, this is at the core of what "most people would need most of the time". All Active Record backed apps need to pay attention to their query times and this helps them find problems early as well as making it easy. A perfect fit for core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gracias Xavier!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 very nice...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant, can't wait to try it on a "real" project!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. Reasonable to put in core due to essentially ubiquitous requirement!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, glad to see this as a default. Will be nice to have early warnings -- especially helpful for new folks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
radical.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Also 👍 to the Timespan class if it doesn't exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be beneficial to show a (condensed) stack trace along with the explain output? It's not always apparent as to which of my app's ActiveRecord queries generated the associated raw query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What an entertaining read. Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome ! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just drooled on myself
out of interest - is there a specific reason you prefer block.call over yield here?