Skip to content
This repository has been archived by the owner on Jun 20, 2022. It is now read-only.

Commit

Permalink
Revert "Merge pull request rails#6197 from blakesmith/connection_adap…
Browse files Browse the repository at this point in the history
…ters_without_explain_support"

This reverts commit 29d564a.

This Rails upstream commit, added after 3.2.12 and included in 3.2.13, mistakenly opens a database connection to check for auto explain support.
This causes issues if you use threads as each will make a database connection for any queries, even cached ones.

It turns out that the upstream Rails 4-0-stable branch has this issue fixed as they removed the auto explain feature entirely: rails#9400
That pull references this issue, rails#9385, where a database connection was required during assets precompiling following the commit we are reverting.
  • Loading branch information
jrafanie committed Jul 1, 2013
1 parent 7ef5740 commit f3621ad
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 24 deletions.
13 changes: 6 additions & 7 deletions activerecord/lib/active_record/explain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ def self.extended(base)
end
end

# If the database adapter supports explain and 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.

# 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.
#
# The available_queries_for_explain thread variable collects the queries
# to be explained. If the value is nil, it means queries are not being
Expand All @@ -27,7 +26,7 @@ def logging_query_plan # :nodoc:

threshold = auto_explain_threshold_in_seconds
current = Thread.current
if connection.supports_explain? && threshold && current[:available_queries_for_explain].nil?
if threshold && current[:available_queries_for_explain].nil?
begin
queries = current[:available_queries_for_explain] = []
start = Time.now
Expand Down
7 changes: 0 additions & 7 deletions activerecord/lib/active_record/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,6 @@ class Railtie < Rails::Railtie
end
end

initializer "active_record.validate_explain_support" do |app|
if app.config.active_record[:auto_explain_threshold_in_seconds] &&
!ActiveRecord::Base.connection.supports_explain?
warn "auto_explain_threshold_in_seconds is set but will be ignored because your adapter does not support this feature. Please unset the configuration to avoid this warning."
end
end

# Expose database runtime to controller for logging.
initializer "active_record.log_runtime" do |app|
require "active_record/railties/controller_runtime"
Expand Down
10 changes: 0 additions & 10 deletions activerecord/test/cases/explain_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,6 @@ def test_exec_explain_with_binds
assert_equal expected, base.exec_explain(queries)
end

def test_unsupported_connection_adapter
connection.stubs(:supports_explain?).returns(false)

base.logger.expects(:warn).never

with_threshold(0) do
Car.where(:name => 'honda').to_a
end
end

def test_silence_auto_explain
base.expects(:collecting_sqls_for_explain).never
base.logger.expects(:warn).never
Expand Down

0 comments on commit f3621ad

Please sign in to comment.