Refactor adapter interface to match abstract adapter (#1216)

Author: aidanharan

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 - [#1153](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1153) Only support Ruby v3.1+
 - [#1196](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1196) Use default inspect for database adapter
+- [#1216](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1216) Refactor adapter interface to match abstract adapter
 
 #### Fixed
 

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -13,47 +13,39 @@ def write_query?(sql) # :nodoc:
           !READ_QUERY.match?(sql.b)
         end
 
-        def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: true)
-          log(sql, name, async: async) do |notification_payload|
-            with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|
-              result = if id_insert_table_name = query_requires_identity_insert?(sql)
-                         with_identity_insert_enabled(id_insert_table_name, conn) { internal_raw_execute(sql, conn, perform_do: true) }
-                       else
-                         internal_raw_execute(sql, conn, perform_do: true)
-                       end
-              verified!
-              notification_payload[:row_count] = result
-              result
-            end
+        def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch:)
+          result = if id_insert_table_name = query_requires_identity_insert?(sql)
+                     with_identity_insert_enabled(id_insert_table_name, raw_connection) do
+                       internal_exec_sql_query(sql, raw_connection)
+                     end
+                   else
+                     internal_exec_sql_query(sql, raw_connection)
+                   end
+
+          verified!
+          notification_payload[:row_count] = result.count
+          result
+        end
+
+        def cast_result(raw_result)
+          if raw_result.columns.empty?
+            ActiveRecord::Result.empty
+          else
+            ActiveRecord::Result.new(raw_result.columns, raw_result.rows)
           end
         end
 
-        def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false)
-          sql = transform_query(sql)
-
-          check_if_write_query(sql)
-          mark_transaction_written_if_write(sql)
+        def affected_rows(raw_result)
+          raw_result.first['AffectedRows']
+        end
 
-          unless without_prepared_statement?(binds)
+        def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false)
+          unless binds.nil? || binds.empty?
             types, params = sp_executesql_types_and_parameters(binds)
             sql = sp_executesql_sql(sql, types, params, name)
           end
 
-          log(sql, name, binds, async: async) do |notification_payload|
-            with_raw_connection do |conn|
-              result = if id_insert_table_name = query_requires_identity_insert?(sql)
-                         with_identity_insert_enabled(id_insert_table_name, conn) do
-                           internal_exec_sql_query(sql, conn)
-                         end
-                       else
-                         internal_exec_sql_query(sql, conn)
-                       end
-
-              verified!
-              notification_payload[:row_count] = result.count
-              result
-            end
-          end
+          super
         end
 
         def internal_exec_sql_query(sql, conn)
@@ -63,14 +55,14 @@ def internal_exec_sql_query(sql, conn)
           finish_statement_handle(handle)
         end
 
-        def exec_delete(sql, name, binds)
+        def exec_delete(sql, name = nil, binds = [])
           sql = sql.dup << "; SELECT @@ROWCOUNT AS AffectedRows"
-          super(sql, name, binds).rows.first.first
+          super(sql, name, binds)
         end
 
-        def exec_update(sql, name, binds)
+        def exec_update(sql, name = nil, binds = [])
           sql = sql.dup << "; SELECT @@ROWCOUNT AS AffectedRows"
-          super(sql, name, binds).rows.first.first
+          super(sql, name, binds)
         end
 
         def begin_db_transaction
@@ -378,6 +370,7 @@ def sp_executesql_sql(sql, types, params, name)
             sql = "EXEC sp_executesql #{quote(sql)}"
             sql += ", #{types}, #{params}" unless params.empty?
           end
+
           sql.freeze
         end
 
@@ -455,10 +448,9 @@ def finish_statement_handle(handle)
         # TinyTDS returns false instead of raising an exception if connection fails.
         # Getting around this by raising an exception ourselves while PR
         # https://github.com/rails-sqlserver/tiny_tds/pull/469 is not released.
-        def internal_raw_execute(sql, conn, perform_do: false)
-          result = conn.execute(sql).tap do |_result|
-            raise TinyTds::Error, "failed to execute statement" if _result.is_a?(FalseClass)
-          end
+        def internal_raw_execute(sql, raw_connection, perform_do: false)
+          result = raw_connection.execute(sql)
+          raise TinyTds::Error, "failed to execute statement" if result.is_a?(FalseClass)
 
           perform_do ? result.do : result
         end

lib/active_record/connection_adapters/sqlserver/showplan.rb

@@ -16,6 +16,7 @@ def explain(arel, binds = [], options = [])
           sql = to_sql(arel)
           result = with_showplan_on { internal_exec_query(sql, "EXPLAIN", binds) }
           printer = showplan_printer.new(result)
+
           printer.pp
         end
 

test/cases/coerced_tests.rb

@@ -2250,14 +2250,6 @@ class LogSubscriberTest < ActiveRecord::TestCase
   def test_verbose_query_logs_coerced
     original_test_verbose_query_logs
   end
-
-  # Bindings logged slightly differently.
-  coerce_tests! :test_where_in_binds_logging_include_attribute_names
-  def test_where_in_binds_logging_include_attribute_names_coerced
-    Developer.where(id: [1, 2, 3, 4, 5]).load
-    wait
-    assert_match(%{@0 = 1, @1 = 2, @2 = 3, @3 = 4, @4 = 5  [["id", nil], ["id", nil], ["id", nil], ["id", nil], ["id", nil]]}, @logger.logged(:debug).last)
-  end
 end
 
 class ReloadModelsTest < ActiveRecord::TestCase