Merge pull request rails#48054 from Shopify/cleanup-execute

Refactor Mysql2Adapter and TrilogyAdapter

Author: byroot

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -225,18 +225,31 @@ def disable_referential_integrity # :nodoc:
       # Setting +allow_retry+ to true causes the db to reconnect and retry
       # executing the SQL statement in case of a connection-related exception.
       # This option should only be enabled for known idempotent queries.
-      def execute(sql, name = nil, async: false, allow_retry: false)
+      def execute(sql, name = nil, allow_retry: false)
         sql = transform_query(sql)
         check_if_write_query(sql)
 
-        raw_execute(sql, name, async: async, allow_retry: allow_retry)
+        mark_transaction_written_if_write(sql)
+
+        log(sql, name) do
+          with_raw_connection(allow_retry: allow_retry) do |conn|
+            sync_timezone_changes(conn)
+            result = conn.query(sql)
+            handle_warnings(sql)
+            result
+          end
+        end
       end
 
       # Mysql2Adapter doesn't have to free a result after using it, but we use this method
       # to write stuff in an abstract way without concerning ourselves about whether it
       # needs to be explicitly freed or not.
       def execute_and_free(sql, name = nil, async: false) # :nodoc:
-        yield execute(sql, name, async: async)
+        sql = transform_query(sql)
+        check_if_write_query(sql)
+
+        mark_transaction_written_if_write(sql)
+        yield raw_execute(sql, name, async: async)
       end
 
       def begin_db_transaction # :nodoc:
@@ -769,6 +782,8 @@ def sync_timezone_changes(raw_connection)
         end
 
         def internal_execute(sql, name = "SCHEMA", allow_retry: true, uses_transaction: false)
+          sql = transform_query(sql)
+          check_if_write_query(sql)
           raw_execute(sql, name, allow_retry: allow_retry, uses_transaction: uses_transaction)
         end
 

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

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  module ConnectionAdapters
+    module Trilogy
+      module DatabaseStatements
+        READ_QUERY = AbstractAdapter.build_read_query_regexp(
+          :desc, :describe, :set, :show, :use
+        ) # :nodoc:
+        private_constant :READ_QUERY
+
+        HIGH_PRECISION_CURRENT_TIMESTAMP = Arel.sql("CURRENT_TIMESTAMP(6)").freeze # :nodoc:
+        private_constant :HIGH_PRECISION_CURRENT_TIMESTAMP
+
+        def select_all(*, **) # :nodoc:
+          result = nil
+          with_raw_connection do |conn|
+            result = super
+            conn.next_result while conn.more_results_exist?
+          end
+          result
+        end
+
+        def write_query?(sql) # :nodoc:
+          !READ_QUERY.match?(sql)
+        rescue ArgumentError # Invalid encoding
+          !READ_QUERY.match?(sql.b)
+        end
+
+        def explain(arel, binds = [], options = [])
+          sql     = build_explain_clause(options) + " " + to_sql(arel, binds)
+          start   = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+          result  = exec_query(sql, "EXPLAIN", binds)
+          elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
+
+          MySQL::ExplainPrettyPrinter.new.pp(result, elapsed)
+        end
+
+        def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
+          sql = transform_query(sql)
+          check_if_write_query(sql)
+
+          result = raw_execute(sql, name, async: async)
+          ActiveRecord::Result.new(result.fields, result.to_a)
+        end
+
+        def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) # :nodoc:
+          sql = transform_query(sql)
+          check_if_write_query(sql)
+
+          raw_execute(to_sql(sql, binds), name)
+        end
+
+        def exec_delete(sql, name = nil, binds = []) # :nodoc:
+          sql = transform_query(sql)
+          check_if_write_query(sql)
+
+          result = raw_execute(to_sql(sql, binds), name)
+          result.affected_rows
+        end
+
+        alias :exec_update :exec_delete # :nodoc:
+
+        def high_precision_current_timestamp
+          HIGH_PRECISION_CURRENT_TIMESTAMP
+        end
+
+        def build_explain_clause(options = [])
+          return "EXPLAIN" if options.empty?
+
+          explain_clause = "EXPLAIN #{options.join(" ").upcase}"
+
+          if analyze_without_explain? && explain_clause.include?("ANALYZE")
+            explain_clause.sub("EXPLAIN ", "")
+          else
+            explain_clause
+          end
+        end
+
+        private
+          def last_inserted_id(result)
+            result.last_insert_id
+          end
+      end
+    end
+  end
+end

activerecord/lib/active_record/connection_adapters/trilogy_adapter.rb

@@ -5,6 +5,7 @@
 gem "trilogy", "~> 2.4"
 require "trilogy"
 
+require "active_record/connection_adapters/trilogy/database_statements"
 require "active_record/connection_adapters/trilogy/lost_connection_exception_translator"
 require "active_record/connection_adapters/trilogy/errors"
 
@@ -37,86 +38,13 @@ def trilogy_connection(config)
   end
   module ConnectionAdapters
     class TrilogyAdapter < AbstractMysqlAdapter
-      module DatabaseStatements
-        READ_QUERY = AbstractAdapter.build_read_query_regexp(
-          :desc, :describe, :set, :show, :use
-        ) # :nodoc:
-        private_constant :READ_QUERY
-
-        HIGH_PRECISION_CURRENT_TIMESTAMP = Arel.sql("CURRENT_TIMESTAMP(6)").freeze # :nodoc:
-        private_constant :HIGH_PRECISION_CURRENT_TIMESTAMP
-
-        def select_all(*, **) # :nodoc:
-          result = nil
-          with_raw_connection do |conn|
-            result = super
-            conn.next_result while conn.more_results_exist?
-          end
-          result
-        end
-
-        def write_query?(sql) # :nodoc:
-          !READ_QUERY.match?(sql)
-        rescue ArgumentError # Invalid encoding
-          !READ_QUERY.match?(sql.b)
-        end
-
-        def explain(arel, binds = [], options = [])
-          sql     = build_explain_clause(options) + " " + to_sql(arel, binds)
-          start   = Process.clock_gettime(Process::CLOCK_MONOTONIC)
-          result  = exec_query(sql, "EXPLAIN", binds)
-          elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
-
-          MySQL::ExplainPrettyPrinter.new.pp(result, elapsed)
-        end
-
-        def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false)
-          result = execute(sql, name, async: async)
-          ActiveRecord::Result.new(result.fields, result.to_a)
-        end
-
-        alias exec_without_stmt exec_query
-
-        def exec_insert(sql, name, binds, pk = nil, sequence_name = nil)
-          execute(to_sql(sql, binds), name)
-        end
-
-        def exec_delete(sql, name = nil, binds = [])
-          result = execute(to_sql(sql, binds), name)
-          result.affected_rows
-        end
-
-        alias :exec_update :exec_delete
-
-        def high_precision_current_timestamp
-          HIGH_PRECISION_CURRENT_TIMESTAMP
-        end
-
-        def build_explain_clause(options = [])
-          return "EXPLAIN" if options.empty?
-
-          explain_clause = "EXPLAIN #{options.join(" ").upcase}"
-
-          if analyze_without_explain? && explain_clause.include?("ANALYZE")
-            explain_clause.sub("EXPLAIN ", "")
-          else
-            explain_clause
-          end
-        end
-
-        private
-          def last_inserted_id(result)
-            result.last_insert_id
-          end
-      end
-
       ER_BAD_DB_ERROR = 1049
       ER_DBACCESS_DENIED_ERROR = 1044
       ER_ACCESS_DENIED_ERROR = 1045
 
       ADAPTER_NAME = "Trilogy"
 
-      include DatabaseStatements
+      include Trilogy::DatabaseStatements
 
       SSL_MODES = {
         SSL_MODE_DISABLED: ::Trilogy::SSL_DISABLED,
@@ -214,28 +142,35 @@ def discard!
         end
       end
 
-      def each_hash(result)
-        return to_enum(:each_hash, result) unless block_given?
+      def execute(sql, name = nil, allow_retry: false)
+        sql = transform_query(sql)
+        check_if_write_query(sql)
 
-        keys = result.fields.map(&:to_sym)
-        result.rows.each do |row|
-          hash = {}
-          idx = 0
-          row.each do |value|
-            hash[keys[idx]] = value
-            idx += 1
+        raw_execute(sql, name, allow_retry: allow_retry)
+      end
+
+      private
+        def each_hash(result)
+          return to_enum(:each_hash, result) unless block_given?
+
+          keys = result.fields.map(&:to_sym)
+          result.rows.each do |row|
+            hash = {}
+            idx = 0
+            row.each do |value|
+              hash[keys[idx]] = value
+              idx += 1
+            end
+            yield hash
           end
-          yield hash
-        end
 
-        nil
-      end
+          nil
+        end
 
-      def error_number(exception)
-        exception.error_code if exception.respond_to?(:error_code)
-      end
+        def error_number(exception)
+          exception.error_code if exception.respond_to?(:error_code)
+        end
 
-      private
         def connection
           @raw_connection
         end

activerecord/test/cases/adapters/trilogy/trilogy_adapter_test.rb

@@ -319,7 +319,7 @@ class TrilogyAdapterTest < ActiveRecord::TrilogyTestCase
     exception = Minitest::Mock.new
     exception.expect :error_code, 123
 
-    assert_equal 123, @conn.error_number(exception)
+    assert_equal 123, @conn.send(:error_number, exception)
   end
 
   def assert_raises_with_message(exception, message, &block)