Merge pull request #44570 from matthewd/configure-connection

Define AbstractAdapter#configure_connection

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -112,6 +112,8 @@ module ActiveRecord
         )
 
         @default_timezone = self.class.validate_default_timezone(config[:default_timezone])
+
+        configure_connection
       end
 
       EXCEPTION_NEVER = { Exception => :never }.freeze # :nodoc:
@@ -550,11 +552,13 @@ module ActiveRecord
       end
 
       # Disconnects from the database if already connected, and establishes a
-      # new connection with the database. Implementors should call super if they
+      # new connection with the database. Implementors should call super
-      # override the default implementation.
+      # immediately after establishing the new connection (and while still
+      # holding @lock).
       def reconnect!
         clear_cache!(new_connection: true)
         reset_transaction
+        configure_connection
       end
 
       # Disconnects from the database if already connected. Otherwise, this
@@ -584,10 +588,14 @@ module ActiveRecord
       # transactions and other connection-related server-side state. Usually a
       # database-dependent operation.
       #
-      # The default implementation does nothing; the implementation should be
+      # If a database driver or protocol does not support such a feature,
-      # overridden by concrete adapters.
+      # implementors may alias this to #reconnect!. Otherwise, implementors
+      # should call super immediately after resetting the connection (and while
+      # still holding @lock).
       def reset!
-        # this should be overridden by concrete adapters
+        clear_cache!(new_connection: true)
+        reset_transaction
+        configure_connection
       end
 
       # Removes the connection from the pool and disconnect it.
@@ -883,6 +891,16 @@ module ActiveRecord
         def build_result(columns:, rows:, column_types: {})
           ActiveRecord::Result.new(columns, rows, column_types)
         end
+
+        # Perform any necessary initialization upon the newly-established
+        # @raw_connection -- this is the place to modify the adapter's
+        # connection settings, run queries to configure any application-global
+        # "session" variables, etc.
+        #
+        # Implementations may assume this method will only be called while
+        # holding @lock (or from #initialize).
+        def configure_connection
+        end
     end
   end
 end

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

@@ -59,7 +59,6 @@ module ActiveRecord
         check_prepared_statements_deprecation(config)
         superclass_config = config.reverse_merge(prepared_statements: false)
         super(connection, logger, connection_options, superclass_config)
-        configure_connection
       end
 
       def self.database_exists?(config)
@@ -125,9 +124,11 @@ module ActiveRecord
       end
 
       def reconnect!
-        super
+        @lock.synchronize do
           disconnect!
           connect
+          super
+        end
       end
       alias :reset! :reconnect!
 
@@ -155,7 +156,6 @@ module ActiveRecord
 
         def connect
           @raw_connection = self.class.new_client(@config)
-          configure_connection
         end
 
         def configure_connection

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -283,21 +283,13 @@ module ActiveRecord
 
       # Initializes and connects a PostgreSQL adapter.
       def initialize(connection, logger, connection_parameters, config)
-        super(connection, logger, config)
-
         @connection_parameters = connection_parameters || {}
 
-        # @local_tz is initialized as nil to avoid warnings when connect tries to use it
-        @local_tz = nil
         @max_identifier_length = nil
+        @type_map = nil
 
-        configure_connection
+        super(connection, logger, config)
-        add_pg_encoders
-        add_pg_decoders
 
-        @type_map = Type::HashLookupTypeMap.new
-        initialize_type_map
-        @local_tz = execute("SHOW TIME ZONE", "SCHEMA").first["TimeZone"]
         @use_insert_returning = @config.key?(:insert_returning) ? self.class.type_cast_config_to_boolean(@config[:insert_returning]) : true
       end
 
@@ -318,31 +310,38 @@ module ActiveRecord
       end
 
       def reload_type_map # :nodoc:
+        @lock.synchronize do
+          if @type_map
             type_map.clear
+          else
+            @type_map = Type::HashLookupTypeMap.new
+          end
+
           initialize_type_map
         end
+      end
 
       # Close then reopen the connection.
       def reconnect!
         @lock.synchronize do
+          begin
             @raw_connection.reset
-          super
-          configure_connection
-          reload_type_map
           rescue PG::ConnectionBad
             connect
           end
+
+          super
+        end
       end
 
       def reset!
         @lock.synchronize do
-          reset_transaction
           unless @raw_connection.transaction_status == ::PG::PQTRANS_IDLE
             @raw_connection.query "ROLLBACK"
           end
           @raw_connection.query "DISCARD ALL"
-          clear_cache!(new_connection: true)
+
-          configure_connection
+          super
         end
       end
 
@@ -853,9 +852,6 @@ module ActiveRecord
         # connected server's characteristics.
         def connect
           @raw_connection = self.class.new_client(@connection_parameters)
-          configure_connection
-          add_pg_encoders
-          add_pg_decoders
         end
 
         # Configures the encoding, verbosity, schema search path, and time zone of the connection.
@@ -872,16 +868,6 @@ module ActiveRecord
 
           variables = @config.fetch(:variables, {}).stringify_keys
 
-          # If using Active Record's time zone support configure the connection to return
-          # TIMESTAMP WITH ZONE types in UTC.
-          unless variables["timezone"]
-            if default_timezone == :utc
-              variables["timezone"] = "UTC"
-            elsif @local_tz
-              variables["timezone"] = @local_tz
-            end
-          end
-
           # Set interval output format to ISO 8601 for ease of parsing by ActiveSupport::Duration.parse
           execute("SET intervalstyle = iso_8601", "SCHEMA")
 
@@ -895,6 +881,28 @@ module ActiveRecord
               execute("SET SESSION #{k} TO #{quote(v)}", "SCHEMA")
             end
           end
+
+          add_pg_encoders
+          add_pg_decoders
+
+          reload_type_map
+        end
+
+        def reconfigure_connection_timezone
+          variables = @config.fetch(:variables, {}).stringify_keys
+
+          # If it's been directly configured as a connection variable, we don't
+          # need to do anything here; it will be set up by configure_connection
+          # and then never changed.
+          return if variables["timezone"]
+
+          # If using Active Record's time zone support configure the connection
+          # to return TIMESTAMP WITH ZONE types in UTC.
+          if default_timezone == :utc
+            execute("SET SESSION timezone TO 'UTC'", "SCHEMA")
+          else
+            execute("SET SESSION timezone TO DEFAULT", "SCHEMA")
+          end
         end
 
         # Returns the list of a table's column names, data types, and default values.
@@ -987,7 +995,7 @@ module ActiveRecord
 
             # if default timezone has changed, we need to reconfigure the connection
             # (specifically, the session time zone)
-            configure_connection
+            reconfigure_connection_timezone
           end
         end
 

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -88,7 +88,6 @@ module ActiveRecord
       def initialize(connection, logger, connection_options, config)
         @memory_database = config[:database] == ":memory:"
         super(connection, logger, config)
-        configure_connection
       end
 
       def self.database_exists?(config)
@@ -165,12 +164,17 @@ module ActiveRecord
       end
 
       def reconnect!
-        unless @raw_connection.closed?
+        @lock.synchronize do
+          if active?
             @raw_connection.rollback rescue nil
+          else
+            connect
           end
+
           super
-        connect if @raw_connection.closed?
         end
+      end
+      alias :reset! :reconnect!
 
       # Disconnects from the database if already connected. Otherwise, this
       # method does nothing.
@@ -608,7 +612,6 @@ module ActiveRecord
             @config[:database].to_s,
             @config.merge(results_as_hash: true)
           )
-          configure_connection
         end
 
         def configure_connection