From 0007501669879a8560090897f78cf1d1b69bf30d Mon Sep 17 00:00:00 2001
From: Eugene Kenny <elkenny@gmail.com>
Date: Wed, 26 Sep 2018 03:10:51 +0100
Subject: Only define attribute methods from schema cache

To define the attribute methods for a model, Active Record needs to know
the schema of the underlying table, which is usually achieved by making
a request to the database. This is undesirable behaviour while the app
is booting, for two reasons: it makes the boot process dependent on the
availability of the database, and it means every new process will make
one query for each table, which can cause issues for large applications.

However, if the application is using the schema cache dump feature, then
the schema cache already contains the necessary information, and we can
define the attribute methods without causing any extra database queries.
---
 .../active_record/connection_adapters/schema_cache.rb    |  5 +++++
 activerecord/lib/active_record/core.rb                   |  4 ++++
 activerecord/lib/active_record/internal_metadata.rb      |  4 ++++
 activerecord/lib/active_record/railtie.rb                | 14 +++++++++++++-
 activerecord/lib/active_record/schema_migration.rb       |  4 ++++
 .../test/cases/connection_adapters/schema_cache_test.rb  | 16 ++++++++++++++++
 6 files changed, 46 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb
index c29cf1f9a1..c10765f42d 100644
--- a/activerecord/lib/active_record/connection_adapters/schema_cache.rb
+++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb
@@ -77,6 +77,11 @@ module ActiveRecord
         }]
       end
 
+      # Checks whether the columns hash is already cached for a table.
+      def columns_hash?(table_name)
+        @columns_hash.key?(table_name)
+      end
+
       # Clears out internal caches
       def clear!
         @columns.clear
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 392602bc0f..85c87227ff 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -283,6 +283,10 @@ module ActiveRecord
         TypeCaster::Map.new(self)
       end
 
+      def _internal? # :nodoc:
+        false
+      end
+
       private
 
         def cached_find_by_statement(key, &block)
diff --git a/activerecord/lib/active_record/internal_metadata.rb b/activerecord/lib/active_record/internal_metadata.rb
index 3626a13d7c..88b0c828ae 100644
--- a/activerecord/lib/active_record/internal_metadata.rb
+++ b/activerecord/lib/active_record/internal_metadata.rb
@@ -8,6 +8,10 @@ module ActiveRecord
   # as which environment migrations were run in.
   class InternalMetadata < ActiveRecord::Base # :nodoc:
     class << self
+      def _internal?
+        true
+      end
+
       def primary_key
         "key"
       end
diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index 81ad9ef3a2..74ccb98159 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -140,7 +140,19 @@ end_error
     initializer "active_record.define_attribute_methods" do |app|
       config.after_initialize do
         ActiveSupport.on_load(:active_record) do
-          descendants.each(&:define_attribute_methods) if app.config.eager_load
+          if app.config.eager_load
+            descendants.each do |model|
+              # SchemaMigration and InternalMetadata both override `table_exists?`
+              # to bypass the schema cache, so skip them to avoid the extra queries.
+              next if model._internal?
+
+              # If there's no connection yet, or the schema cache doesn't have the columns
+              # hash for the model cached, `define_attribute_methods` would trigger a query.
+              next unless model.connected? && model.connection.schema_cache.columns_hash?(model.table_name)
+
+              model.define_attribute_methods
+            end
+          end
         end
       end
     end
diff --git a/activerecord/lib/active_record/schema_migration.rb b/activerecord/lib/active_record/schema_migration.rb
index f2d8b038fa..1fca1a18f6 100644
--- a/activerecord/lib/active_record/schema_migration.rb
+++ b/activerecord/lib/active_record/schema_migration.rb
@@ -10,6 +10,10 @@ module ActiveRecord
   # to be executed the next time.
   class SchemaMigration < ActiveRecord::Base # :nodoc:
     class << self
+      def _internal?
+        true
+      end
+
       def primary_key
         "version"
       end
diff --git a/activerecord/test/cases/connection_adapters/schema_cache_test.rb b/activerecord/test/cases/connection_adapters/schema_cache_test.rb
index 67496381d1..727cab77f5 100644
--- a/activerecord/test/cases/connection_adapters/schema_cache_test.rb
+++ b/activerecord/test/cases/connection_adapters/schema_cache_test.rb
@@ -91,6 +91,22 @@ module ActiveRecord
         @cache.clear_data_source_cache!("posts")
       end
 
+      test "#columns_hash? is populated by #columns_hash" do
+        assert_not @cache.columns_hash?("posts")
+
+        @cache.columns_hash("posts")
+
+        assert @cache.columns_hash?("posts")
+      end
+
+      test "#columns_hash? is not populated by #data_source_exists?" do
+        assert_not @cache.columns_hash?("posts")
+
+        @cache.data_source_exists?("posts")
+
+        assert_not @cache.columns_hash?("posts")
+      end
+
       private
 
         def schema_dump_path
-- 
cgit v1.2.3