Retry SELECT queries on connection-related exceptions

SELECT queries are idempotent and thus safe to retry automatically.
This commit leverages `allow_retry` to enable automatic retries on
queries that go through `ActiveRecord::ConnectionAdaptersDatabaseStatements#select`.

This should help with connection-related exceptions stemming
from network issues, such as `Trilogy::EOFError` or `Mysql2::Error`.

Co-authored-by: Gannon McGibbon <gannon.mcgibbon@shopify.com>

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Retry SELECT queries on connection-related exceptions
+
+    SELECT queries are idempotent and thus safe to retry automatically. Previously,
+    adapters such as `TrilogyAdapter` would raise `ActiveRecord::ConnectionFailed: Trilogy::EOFError`
+    when encountering a connection error mid-request during a SELECT query.
+
+    *Adrianna Chang*, *Gannon McGibbon*
+
 *   Add support for encrypting binary columns
 
     Ensure encryption and decryption pass `Type::Binary::Data` around for binary data.

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

@@ -507,7 +507,7 @@ def high_precision_current_timestamp
         HIGH_PRECISION_CURRENT_TIMESTAMP
       end
 
-      def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
+      def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false) # :nodoc:
         raise NotImplementedError
       end
 
@@ -627,7 +627,7 @@ def select(sql, name = nil, binds = [], prepare: false, async: false)
             return future_result
           end
 
-          result = internal_exec_query(sql, name, binds, prepare: prepare)
+          result = internal_exec_query(sql, name, binds, prepare: prepare, allow_retry: true)
           if async
             FutureResult.wrap(result)
           else

activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb

@@ -18,7 +18,7 @@ def select_all(*, **) # :nodoc:
           result
         end
 
-        def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
+        def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false) # :nodoc:
           if without_prepared_statement?(binds)
             execute_and_free(sql, name, async: async) do |result|
               if result

activerecord/lib/active_record/connection_adapters/sqlite3/database_statements.rb

@@ -21,7 +21,7 @@ def explain(arel, binds = [], _options = [])
           SQLite3::ExplainPrettyPrinter.new.pp(result)
         end
 
-        def internal_exec_query(sql, name = nil, binds = [], prepare: false, async: false) # :nodoc:
+        def internal_exec_query(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false) # :nodoc:
           sql = transform_query(sql)
           check_if_write_query(sql)
 

activerecord/lib/active_record/connection_adapters/trilogy/database_statements.rb

@@ -12,12 +12,12 @@ def select_all(*, **) # :nodoc:
           result
         end
 
-        def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
+        def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false) # :nodoc:
           sql = transform_query(sql)
           check_if_write_query(sql)
           mark_transaction_written_if_write(sql)
 
-          result = raw_execute(sql, name, async: async)
+          result = raw_execute(sql, name, async: async, allow_retry: allow_retry)
           ActiveRecord::Result.new(result.fields, result.to_a)
         end
 

activerecord/test/cases/adapter_test.rb

@@ -632,17 +632,25 @@ def teardown
         assert_predicate @connection, :active?
       end
 
-      test "querying after a failed query restores and succeeds" do
+      test "querying after a failed non-retryable query restores and succeeds" do
         Post.first # Connection verified (and prepared statement pool populated if enabled)
 
         remote_disconnect @connection
 
         assert_raises(ActiveRecord::ConnectionFailed) do
-          Post.first # Connection no longer verified after failed query
+          @connection.execute("INSERT INTO posts(title, body) VALUES ('foo', 'bar')")
         end
 
         assert Post.first # Verifying the connection causes a reconnect and the query succeeds
+        assert_predicate @connection, :active?
+      end
+
+      test "select queries are retried and result in a reconnect" do
+        Post.first # Connection verified (and prepared statement pool populated if enabled)
+
+        remote_disconnect @connection
 
+        assert Post.first
         assert_predicate @connection, :active?
       end