Retry on Net::HTTPBadResponse (stale keep-alive socket) + remaining Rails 8 fixes

.github/workflows/testing.yml

@@ -22,6 +22,8 @@ jobs:
       max-parallel: 1
       matrix:
         version:
+          - ruby: 3.4
+            rails: 7.1.5
           - ruby: 3.2
             rails: 7.2.3
           - ruby: 3.2

lib/active_record/connection_adapters/clickhouse/oid/date_time.rb

@@ -9,6 +9,10 @@ class DateTime < Type::DateTime # :nodoc:
           def serialize(value)
             value = super
             return unless value
+            # Rails 8 routes raw query values (e.g. integer epochs from defaults)
+            # through #serialize; only Time-like values can be strftime'd, pass
+            # anything else (Integer, etc.) through for ClickHouse to interpret.
+            return value unless value.respond_to?(:strftime)
 
             value.strftime('%Y-%m-%d %H:%M:%S' + (@precision.present? && @precision > 0 ? ".%#{@precision}N" : ''))
           end

lib/active_record/connection_adapters/clickhouse/schema_statements.rb

@@ -12,6 +12,20 @@ module SchemaStatements
         HTTP_AUTH_X_HEADERS = :x_clickhouse_headers
         HTTP_AUTH_TYPES = [HTTP_AUTH_QUERY_PARAMS, HTTP_AUTH_BASIC, HTTP_AUTH_X_HEADERS].freeze
 
+        # Rails calls #write_query? via #check_if_write_query for every statement
+        # when write prevention is enabled (e.g. connected_to(role: :reading));
+        # the AbstractAdapter default raises NotImplementedError. Classify reads
+        # the same way the built-in adapters do (SELECT/WITH/SHOW/DESCRIBE/etc.).
+        READ_QUERY = ActiveRecord::ConnectionAdapters::AbstractAdapter.build_read_query_regexp(
+          :describe, :exists, :show
+        ).freeze
+
+        def write_query?(sql)
+          !READ_QUERY.match?(sql)
+        rescue ArgumentError # Invalid encoding
+          !READ_QUERY.match?(sql.b)
+        end
+
         def with_settings(**settings)
           @block_settings ||= {}
           prev_settings = @block_settings
@@ -306,8 +320,13 @@ def raw_execute(sql, settings: {}, except_params: [])
           retries = 2
           begin
             response = request(statement, settings: settings, except_params: except_params)
-          rescue EOFError, Errno::ECONNRESET
+          rescue EOFError, Errno::ECONNRESET, Net::HTTPBadResponse
+            # Net::HTTPBadResponse means the keep-alive socket was left with
+            # unread bytes from an aborted previous response (e.g. a timeout
+            # raised mid-body-read); Net::HTTP has already closed the socket by
+            # the time it raises, so retrying transparently reconnects.
             retry if (retries -= 1) > 0 # rubocop:disable Style/NumericPredicate
+            raise
           end
           statement.processed_response(response)
         end

lib/active_record/connection_adapters/clickhouse_adapter.rb

@@ -168,6 +168,15 @@ def initialize(config_or_deprecated_connection, deprecated_logger = nil, depreca
         connect
       end
 
+      # ClickHouse is accessed over stateless HTTP, not a pooled connection that
+      # can run a query on a separate thread. Rails 8 routes async queries
+      # (load_async / FutureResult) through raw_execute/perform_query paths this
+      # adapter does not implement; disabling concurrent connections makes
+      # async_enabled? false so those queries run synchronously via the normal path.
+      def supports_concurrent_connections?
+        false
+      end
+
       def disconnect!
         @connection.finish if @connection&.started?
         super
@@ -574,6 +583,16 @@ def apply_cluster(sql)
         end
       end
 
+      # ClickHouse cannot return values from an INSERT: it has no RETURNING
+      # clause and no auto-increment columns. The AbstractAdapter default
+      # (`column.auto_populated?`) is true for any column with a DEFAULT
+      # expression (e.g. PeerDB's `_peerdb_synced_at DateTime64 DEFAULT now64()`),
+      # which on Rails 8 makes _create_record write the bogus exec_insert result
+      # back into that attribute. Nothing is ever fetched after an insert here.
+      def return_value_after_insert?(_column) # :nodoc:
+        false
+      end
+
       def supports_insert_on_duplicate_skip?
         true
       end

spec/single/raw_execute_retry_spec.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+# Regression coverage for stale keep-alive sockets.
+#
+# When a previous response is aborted mid-read (e.g. a timeout raised while
+# Net::HTTP is reading the body), unread bytes stay buffered on the persistent
+# connection and the next query fails with Net::HTTPBadResponse ("wrong status
+# line"). Net::HTTP closes the socket before raising, so a retry reconnects
+# and succeeds.
+RSpec.describe 'ActiveRecord::ConnectionAdapters::Clickhouse::SchemaStatements#raw_execute retries' do
+  let(:connection) { ActiveRecord::Base.connection }
+  let(:http) { connection.instance_variable_get(:@connection) }
+
+  it 'retries and succeeds when the first attempt raises Net::HTTPBadResponse' do
+    calls = 0
+    allow(http).to receive(:post).and_wrap_original do |original, *args|
+      calls += 1
+      raise Net::HTTPBadResponse, 'wrong status line: "c8e74\"]"' if calls == 1
+
+      original.call(*args)
+    end
+
+    expect(connection.select_value('SELECT 1').to_i).to eq(1)
+    expect(calls).to eq(2)
+  end
+
+  it 'raises the original error when retries are exhausted' do
+    allow(http).to receive(:post).and_raise(Net::HTTPBadResponse, 'wrong status line: "c8e74\"]"')
+
+    expect { connection.select_value('SELECT 1') }
+      .to raise_error(ActiveRecord::ActiveRecordError, /wrong status line/)
+  end
+end