1 files changed, 40 insertions, 47 deletions

diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb
index abe6fff5d5..6dff53d959 100644
--- a/activerecord/lib/active_record/explain.rb
+++ b/activerecord/lib/active_record/explain.rb
@@ -1,68 +1,65 @@
 module ActiveRecord
   module Explain
-    # 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) # :nodoc:
+    #
+    # The available_queries_for_explain thread variable collects the queries
+    # to be explained. If the value is nil, it means queries are not being
+    # currently collected. A false value indicates collecting is turned
+    # off. Otherwise it is an array of queries.
+    def logging_query_plan # :nodoc:
       threshold = auto_explain_threshold_in_seconds
-      if threshold && !Thread.current[LOGGING_QUERY_PLAN] && !Thread.current[SILENCED]
+      current   = Thread.current
+      if threshold && current[:available_queries_for_explain].nil?
         begin
-          Thread.current[LOGGING_QUERY_PLAN] = true
+          queries = current[:available_queries_for_explain] = []
           start = Time.now
-          result, sqls, binds = collecting_sqls_for_explain(&block)
-          logger.warn(exec_explain(sqls, binds)) if Time.now - start > threshold
+          result = yield
+          logger.warn(exec_explain(queries)) if Time.now - start > threshold
           result
         ensure
-          Thread.current[LOGGING_QUERY_PLAN] = false
+          current[:available_queries_for_explain] = nil
         end
       else
         yield
       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 = %w(SCHEMA EXPLAIN)
-    def ignore_explain_notification?(payload) # :nodoc:
-      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 # :nodoc:
-      sqls  = []
-      binds = []
-      callback = lambda do |*args|
-        payload = args.last
-        unless ignore_explain_notification?(payload)
-          sqls  << payload[:sql]
-          binds << payload[:binds]
+    # This method receives payloads from the explain subscriber and is
+    # responsible for collecting or ignoring them.
+    def collect_queries_for_explain(payload) # :nodoc:
+      if queries = Thread.current[:available_queries_for_explain]
+        unless ignore_payload_for_explain?(payload)
+          queries << payload.values_at(:sql, :binds)
         end
       end
+    end
 
-      result = nil
-      ActiveSupport::Notifications.subscribed(callback, "sql.active_record") do
-        result = yield
-      end
+    # Relation#explain needs to be able to collect the queries regardless of
+    # whether auto explain is enabled. This method serves that purpose.
+    def collecting_queries_for_explain # :nodoc:
+      current = Thread.current
+      original, current[:available_queries_for_explain] = current[:available_queries_for_explain], []
+      return yield, current[:available_queries_for_explain]
+    ensure
+      # Note that the return value above does not depend on this assigment.
+      current[:available_queries_for_explain] = original
+    end
 
-      [result, sqls, binds]
+    # 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 = %w(SCHEMA EXPLAIN)
+    def ignore_payload_for_explain?(payload) # :nodoc:
+      payload[:exception] || SKIP_EXPLAIN_FOR.include?(payload[:name])
     end
 
-    # Makes the adapter execute EXPLAIN for the given queries and bindings.
+    # Makes the adapter execute EXPLAIN for the tuples of queries and bindings.
     # Returns a formatted string ready to be logged.
-    def exec_explain(sqls, binds) # :nodoc:
-      sqls.zip(binds).map do |sql, bind|
+    def exec_explain(queries) # :nodoc:
+      queries && queries.map do |sql, bind|
         [].tap do |msg|
           msg << "EXPLAIN for: #{sql}"
           unless bind.empty?
@@ -74,8 +71,6 @@ module ActiveRecord
       end.join("\n")
     end
 
-    SILENCED = :silence_explain
-
     # Silences automatic EXPLAIN logging for the duration of the block.
     #
     # This has high priority, no EXPLAINs will be run even if downwards
@@ -84,13 +79,11 @@ module ActiveRecord
     # As the name of the method suggests this only applies to automatic
     # EXPLAINs, manual calls to +ActiveRecord::Relation#explain+ run.
     def silence_auto_explain
-      # Implemented as a flag rather that setting the threshold to nil
-      # because we should not depend on a value that may be changed
-      # downwards.
-      Thread.current[SILENCED] = true
+      current = Thread.current
+      original, current[:available_queries_for_explain] = current[:available_queries_for_explain], false
       yield
     ensure
-      Thread.current[SILENCED] = false
+      current[:available_queries_for_explain] = original
     end
   end
 end