Merge pull request #35795 from alimi/cache-database-version

Cache database version in schema cache

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

@@ -810,6 +810,7 @@ module ActiveRecord
         def new_connection
           Base.send(spec.adapter_method, spec.config).tap do |conn|
             conn.schema_cache = schema_cache.dup if schema_cache
+            conn.check_version
           end
         end
 

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -133,8 +133,6 @@ module ActiveRecord
         @advisory_locks_enabled = self.class.type_cast_config_to_boolean(
           config.fetch(:advisory_locks, true)
         )
-
-        check_version
       end
 
       def replica?
@@ -575,9 +573,17 @@ module ActiveRecord
         "INSERT #{insert.into} #{insert.values_list}"
       end
 
+      def get_database_version # :nodoc:
+      end
+
+      def database_version # :nodoc:
+        schema_cache.database_version
+      end
+
+      def check_version # :nodoc:
+      end
+
       private
-        def check_version
-        end
 
         def type_map
           @type_map ||= Type::TypeMap.new.tap do |mapping|

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -55,8 +55,8 @@ module ActiveRecord
         super(connection, logger, config)
       end
 
-      def version #:nodoc:
-        @version ||= Version.new(version_string)
+      def get_database_version #:nodoc:
+        Version.new(version_string)
       end
 
       def mariadb? # :nodoc:
@@ -68,11 +68,11 @@ module ActiveRecord
       end
 
       def supports_index_sort_order?
-        !mariadb? && version >= "8.0.1"
+        !mariadb? && database_version >= "8.0.1"
       end
 
       def supports_expression_index?
-        !mariadb? && version >= "8.0.13"
+        !mariadb? && database_version >= "8.0.13"
       end
 
       def supports_transaction_isolation?
@@ -96,16 +96,16 @@ module ActiveRecord
       end
 
       def supports_datetime_with_precision?
-        mariadb? || version >= "5.6.4"
+        mariadb? || database_version >= "5.6.4"
       end
 
       def supports_virtual_columns?
-        mariadb? || version >= "5.7.5"
+        mariadb? || database_version >= "5.7.5"
       end
 
       # See https://dev.mysql.com/doc/refman/8.0/en/optimizer-hints.html for more details.
       def supports_optimizer_hints?
-        !mariadb? && version >= "5.7.7"
+        !mariadb? && database_version >= "5.7.7"
       end
 
       def supports_advisory_locks?
@@ -526,12 +526,13 @@ module ActiveRecord
         sql
       end
 
-      private
-        def check_version
-          if version < "5.5.8"
-            raise "Your version of MySQL (#{version_string}) is too old. Active Record supports MySQL >= 5.5.8."
-          end
+      def check_version # :nodoc:
+        if database_version < "5.5.8"
+          raise "Your version of MySQL (#{database_version}) is too old. Active Record supports MySQL >= 5.5.8."
         end
+      end
+
+      private
 
         def initialize_type_map(m = type_map)
           super
@@ -702,7 +703,7 @@ module ActiveRecord
         end
 
         def supports_rename_index?
-          mariadb? ? false : version >= "5.7.6"
+          mariadb? ? false : database_version >= "5.7.6"
         end
 
         def configure_connection

activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb

@@ -64,7 +64,7 @@ module ActiveRecord
           end
 
           def extract_expression_for_virtual_column(column)
-            if @connection.mariadb? && @connection.version < "10.2.5"
+            if @connection.mariadb? && @connection.database_version < "10.2.5"
               create_table_info = @connection.send(:create_table_info, column.table_name)
               column_name = @connection.quote_column_name(column.name)
               if %r/#{column_name} #{Regexp.quote(column.sql_type)}(?: COLLATE \w+)? AS \((?<expression>.+?)\) #{column.extra}/ =~ create_table_info

activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb

@@ -126,9 +126,9 @@ module ActiveRecord
 
           def row_format_dynamic_by_default?
             if mariadb?
-              version >= "10.2.2"
+              database_version >= "10.2.2"
             else
-              version >= "5.7.9"
+              database_version >= "5.7.9"
             end
           end
 

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

@@ -43,7 +43,7 @@ module ActiveRecord
       end
 
       def supports_json?
-        !mariadb? && version >= "5.7.8"
+        !mariadb? && database_version >= "5.7.8"
       end
 
       def supports_comments?

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

@@ -287,7 +287,7 @@ module ActiveRecord
             quoted_sequence = quote_table_name(sequence)
             max_pk = query_value("SELECT MAX(#{quote_column_name pk}) FROM #{quote_table_name(table)}", "SCHEMA")
             if max_pk.nil?
-              if postgresql_version >= 100000
+              if database_version >= 100000
                 minvalue = query_value("SELECT seqmin FROM pg_sequence WHERE seqrelid = #{quote(quoted_sequence)}::regclass", "SCHEMA")
               else
                 minvalue = query_value("SELECT min_value FROM #{quoted_sequence}", "SCHEMA")

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -201,7 +201,7 @@ module ActiveRecord
       end
 
       def supports_insert_on_conflict?
-        postgresql_version >= 90500
+        database_version >= 90500
       end
       alias supports_insert_on_duplicate_skip? supports_insert_on_conflict?
       alias supports_insert_on_duplicate_update? supports_insert_on_conflict?
@@ -344,7 +344,7 @@ module ActiveRecord
       end
 
       def supports_pgcrypto_uuid?
-        postgresql_version >= 90400
+        database_version >= 90400
       end
 
       def supports_optimizer_hints?
@@ -424,7 +424,7 @@ module ActiveRecord
       }
 
       # Returns the version of the connected PostgreSQL server.
-      def postgresql_version
+      def get_database_version
         @connection.server_version
       end
 
@@ -446,12 +446,13 @@ module ActiveRecord
         sql
       end
 
-      private
-        def check_version
-          if postgresql_version < 90300
-            raise "Your version of PostgreSQL (#{postgresql_version}) is too old. Active Record supports PostgreSQL >= 9.3."
-          end
+      def check_version # :nodoc:
+        if database_version < 90300
+          raise "Your version of PostgreSQL (#{database_version}) is too old. Active Record supports PostgreSQL >= 9.3."
         end
+      end
+
+      private
 
         # See https://www.postgresql.org/docs/current/static/errcodes-appendix.html
         VALUE_LIMIT_VIOLATION = "22001"

activerecord/lib/active_record/connection_adapters/schema_cache.rb

@@ -26,21 +26,23 @@ module ActiveRecord
       end
 
       def encode_with(coder)
-        coder["columns"]      = @columns
-        coder["columns_hash"] = @columns_hash
-        coder["primary_keys"] = @primary_keys
-        coder["data_sources"] = @data_sources
-        coder["indexes"]      = @indexes
-        coder["version"]      = connection.migration_context.current_version
+        coder["columns"]          = @columns
+        coder["columns_hash"]     = @columns_hash
+        coder["primary_keys"]     = @primary_keys
+        coder["data_sources"]     = @data_sources
+        coder["indexes"]          = @indexes
+        coder["version"]          = connection.migration_context.current_version
+        coder["database_version"] = database_version
       end
 
       def init_with(coder)
-        @columns      = coder["columns"]
-        @columns_hash = coder["columns_hash"]
-        @primary_keys = coder["primary_keys"]
-        @data_sources = coder["data_sources"]
-        @indexes      = coder["indexes"] || {}
-        @version      = coder["version"]
+        @columns          = coder["columns"]
+        @columns_hash     = coder["columns_hash"]
+        @primary_keys     = coder["primary_keys"]
+        @data_sources     = coder["data_sources"]
+        @indexes          = coder["indexes"] || {}
+        @version          = coder["version"]
+        @database_version = coder["database_version"]
       end
 
       def primary_keys(table_name)
@@ -91,6 +93,10 @@ module ActiveRecord
         @indexes[table_name] ||= connection.indexes(table_name)
       end
 
+      def database_version # :nodoc:
+        @database_version ||= connection.get_database_version
+      end
+
       # Clears out internal caches
       def clear!
         @columns.clear
@@ -99,6 +105,7 @@ module ActiveRecord
         @data_sources.clear
         @indexes.clear
         @version = nil
+        @database_version = nil
       end
 
       def size
@@ -117,11 +124,11 @@ module ActiveRecord
       def marshal_dump
         # if we get current version during initialization, it happens stack over flow.
         @version = connection.migration_context.current_version
-        [@version, @columns, @columns_hash, @primary_keys, @data_sources, @indexes]
+        [@version, @columns, @columns_hash, @primary_keys, @data_sources, @indexes, database_version]
       end
 
       def marshal_load(array)
-        @version, @columns, @columns_hash, @primary_keys, @data_sources, @indexes = array
+        @version, @columns, @columns_hash, @primary_keys, @data_sources, @indexes, @database_version = array
         @indexes = @indexes || {}
       end
 

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -111,7 +111,7 @@ module ActiveRecord
       end
 
       def supports_expression_index?
-        sqlite_version >= "3.9.0"
+        database_version >= "3.9.0"
       end
 
       def requires_reloading?
@@ -135,7 +135,7 @@ module ActiveRecord
       end
 
       def supports_insert_on_conflict?
-        sqlite_version >= "3.24.0"
+        database_version >= "3.24.0"
       end
       alias supports_insert_on_duplicate_skip? supports_insert_on_conflict?
       alias supports_insert_on_duplicate_update? supports_insert_on_conflict?
@@ -397,6 +397,16 @@ module ActiveRecord
         sql
       end
 
+      def get_database_version # :nodoc:
+        SQLite3Adapter::Version.new(query_value("SELECT sqlite_version(*)"))
+      end
+
+      def check_version # :nodoc:
+        if database_version < "3.8.0"
+          raise "Your version of SQLite (#{database_version}) is too old. Active Record supports SQLite >= 3.8."
+        end
+      end
+
       private
         # See https://www.sqlite.org/limits.html,
         # the default value is 999 when not configured.
@@ -404,12 +414,6 @@ module ActiveRecord
           999
         end
 
-        def check_version
-          if sqlite_version < "3.8.0"
-            raise "Your version of SQLite (#{sqlite_version}) is too old. Active Record supports SQLite >= 3.8."
-          end
-        end
-
         def initialize_type_map(m = type_map)
           super
           register_class_with_limit m, %r(int)i, SQLite3Integer
@@ -527,10 +531,6 @@ module ActiveRecord
                      SELECT #{quoted_from_columns} FROM #{quote_table_name(from)}")
         end
 
-        def sqlite_version
-          @sqlite_version ||= SQLite3Adapter::Version.new(query_value("SELECT sqlite_version(*)"))
-        end
-
         def translate_exception(exception, message:, sql:, binds:)
           case exception.message
           # SQLite 3.8.2 returns a newly formatted error message:

activerecord/test/cases/adapters/mysql2/datetime_precision_quoting_test.rb

@@ -46,10 +46,7 @@ class Mysql2DatetimePrecisionQuotingTest < ActiveRecord::Mysql2TestCase
 
     def stub_version(full_version_string)
       @connection.stub(:full_version, full_version_string) do
-        @connection.remove_instance_variable(:@version) if @connection.instance_variable_defined?(:@version)
         yield
       end
-    ensure
-      @connection.remove_instance_variable(:@version) if @connection.instance_variable_defined?(:@version)
     end
 end

activerecord/test/cases/adapters/mysql2/sp_test.rb

@@ -9,7 +9,7 @@ class Mysql2StoredProcedureTest < ActiveRecord::Mysql2TestCase
 
   def setup
     @connection = ActiveRecord::Base.connection
-    unless ActiveRecord::Base.connection.version >= "5.6.0"
+    unless ActiveRecord::Base.connection.database_version >= "5.6.0"
       skip("no stored procedure support")
     end
   end

activerecord/test/cases/adapters/postgresql/geometric_test.rb

@@ -247,7 +247,7 @@ class PostgreSQLGeometricLineTest < ActiveRecord::PostgreSQLTestCase
   class PostgresqlLine < ActiveRecord::Base; end
 
   setup do
-    unless ActiveRecord::Base.connection.send(:postgresql_version) >= 90400
+    unless ActiveRecord::Base.connection.database_version >= 90400
       skip("line type is not fully implemented")
     end
     @connection = ActiveRecord::Base.connection

activerecord/test/cases/adapters/postgresql/partitions_test.rb

@@ -12,7 +12,7 @@ class PostgreSQLPartitionsTest < ActiveRecord::PostgreSQLTestCase
   end
 
   def test_partitions_table_exists
-    skip unless ActiveRecord::Base.connection.postgresql_version >= 100000
+    skip unless ActiveRecord::Base.connection.database_version >= 100000
     @connection.create_table :partitioned_events, force: true, id: false,
       options: "partition by range (issued_at)" do |t|
       t.timestamp :issued_at

activerecord/test/cases/connection_adapters/schema_cache_test.rb

@@ -6,8 +6,9 @@ module ActiveRecord
   module ConnectionAdapters
     class SchemaCacheTest < ActiveRecord::TestCase
       def setup
-        @connection = ActiveRecord::Base.connection
-        @cache      = SchemaCache.new @connection
+        @connection       = ActiveRecord::Base.connection
+        @cache            = SchemaCache.new @connection
+        @database_version = @connection.get_database_version
       end
 
       def test_primary_key
@@ -28,6 +29,7 @@ module ActiveRecord
           assert new_cache.data_sources("posts")
           assert_equal "id", new_cache.primary_keys("posts")
           assert_equal 1, new_cache.indexes("posts").size
+          assert_equal @database_version.to_s, new_cache.database_version.to_s
         end
       end
 
@@ -55,6 +57,20 @@ module ActiveRecord
         @connection.schema_cache = old_cache
       end
 
+      def test_yaml_loads_5_1_dump_without_database_version_still_queries_for_database_version
+        @cache = YAML.load(File.read(schema_dump_path))
+
+        # Simulate assignment in railtie after loading the cache.
+        old_cache, @connection.schema_cache = @connection.schema_cache, @cache
+
+        # We can't verify queries get executed because the database version gets
+        # cached in both MySQL and PostgreSQL outside of the schema cache.
+        assert_nil @cache.instance_variable_get(:@database_version)
+        assert_equal @database_version.to_s, @cache.database_version.to_s
+      ensure
+        @connection.schema_cache = old_cache
+      end
+
       def test_primary_key_for_non_existent_table
         assert_nil @cache.primary_keys("omgponies")
       end
@@ -74,6 +90,14 @@ module ActiveRecord
         assert_equal indexes, @cache.indexes("posts")
       end
 
+      def test_caches_database_version
+        @cache.database_version # cache database_version
+
+        assert_no_queries do
+          assert_equal @database_version.to_s, @cache.database_version.to_s
+        end
+      end
+
       def test_clearing
         @cache.columns("posts")
         @cache.columns_hash("posts")
@@ -84,6 +108,7 @@ module ActiveRecord
         @cache.clear!
 
         assert_equal 0, @cache.size
+        assert_nil @cache.instance_variable_get(:@database_version)
       end
 
       def test_dump_and_load
@@ -101,6 +126,7 @@ module ActiveRecord
           assert @cache.data_sources("posts")
           assert_equal "id", @cache.primary_keys("posts")
           assert_equal 1, @cache.indexes("posts").size
+          assert_equal @database_version.to_s, @cache.database_version.to_s
         end
       end
 

activerecord/test/cases/defaults_test.rb

@@ -89,7 +89,7 @@ if current_adapter?(:PostgreSQLAdapter)
 
     test "schema dump includes default expression" do
       output = dump_table_schema("defaults")
-      if ActiveRecord::Base.connection.postgresql_version >= 100000
+      if ActiveRecord::Base.connection.database_version >= 100000
         assert_match %r/t\.date\s+"modified_date",\s+default: -> { "CURRENT_DATE" }/, output
         assert_match %r/t\.datetime\s+"modified_time",\s+default: -> { "CURRENT_TIMESTAMP" }/, output
       else

activerecord/test/cases/helper.rb

@@ -53,7 +53,7 @@ def supports_default_expression?
     true
   elsif current_adapter?(:Mysql2Adapter)
     conn = ActiveRecord::Base.connection
-    !conn.mariadb? && conn.version >= "8.0.13"
+    !conn.mariadb? && conn.database_version >= "8.0.13"
   end
 end
 

activerecord/test/cases/migration/columns_test.rb

@@ -136,7 +136,7 @@ module ActiveRecord
       def test_remove_column_with_multi_column_index
         # MariaDB starting with 10.2.8
         # Dropping a column that is part of a multi-column UNIQUE constraint is not permitted.
-        skip if current_adapter?(:Mysql2Adapter) && connection.mariadb? && connection.version >= "10.2.8"
+        skip if current_adapter?(:Mysql2Adapter) && connection.mariadb? && connection.database_version >= "10.2.8"
 
         add_column "test_models", :hat_size, :integer
         add_column "test_models", :hat_style, :string, limit: 100