Removed legacy connection handling (#1121)

Author: aidanharan

lib/active_record/connection_adapters/sqlserver/database_statements.rb

@@ -19,9 +19,9 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac
           log(sql, name, async: async) do
             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) { _execute(sql, conn, perform_do: true) }
+                         with_identity_insert_enabled(id_insert_table_name, conn) { internal_raw_execute(sql, conn, perform_do: true) }
                        else
-                         _execute(sql, conn, perform_do: true)
+                         internal_raw_execute(sql, conn, perform_do: true)
                        end
             end
           end
@@ -49,11 +49,11 @@ def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: fa
                 # TODO: Look into refactoring this.
                 if id_insert_table_name = query_requires_identity_insert?(sql)
                   with_identity_insert_enabled(id_insert_table_name, conn) do
-                    handle = _execute(sql, conn)
+                    handle = internal_raw_execute(sql, conn)
                     result = handle_to_names_and_values(handle, options)
                   end
                 else
-                  handle = _execute(sql, conn)
+                  handle = internal_raw_execute(sql, conn)
                   result = handle_to_names_and_values(handle, options)
                 end
               ensure
@@ -176,7 +176,7 @@ def execute_procedure(proc_name, *variables)
 
           log(sql, "Execute Procedure") do
             with_raw_connection do |conn|
-              result = _execute(sql, conn)
+              result = internal_raw_execute(sql, conn)
               options = { as: :hash, cache_rows: true, timezone: ActiveRecord.default_timezone || :utc }
 
               result.each(options) do |row|
@@ -305,28 +305,13 @@ def sql_for_insert(sql, pk, binds, returning)
         # === SQLServer Specific ======================================== #
 
         def set_identity_insert(table_name, conn, enable)
-          _execute("SET IDENTITY_INSERT #{table_name} #{enable ? 'ON' : 'OFF'}", conn , perform_do: true)
+          internal_raw_execute("SET IDENTITY_INSERT #{table_name} #{enable ? 'ON' : 'OFF'}", conn , perform_do: true)
         rescue Exception
           raise ActiveRecordError, "IDENTITY_INSERT could not be turned #{enable ? 'ON' : 'OFF'} for table #{table_name}"
         end
 
         # === SQLServer Specific (Executing) ============================ #
 
-        # TODO: Adapter should be refactored to use `with_raw_connection` to translate exceptions.
-        def sp_executesql(sql, name, binds, options = {})
-          options[:ar_result] = true if options[:fetch] != :rows
-
-          unless without_prepared_statement?(binds)
-            types, params = sp_executesql_types_and_parameters(binds)
-            sql = sp_executesql_sql(sql, types, params, name)
-          end
-
-          raw_select sql, name, binds, options
-        rescue => original_exception
-          translated_exception = translate_exception_class(original_exception, sql, binds)
-          raise translated_exception
-        end
-
         def sp_executesql_types_and_parameters(binds)
           types, params = [], []
           binds.each_with_index do |attr, index|
@@ -377,13 +362,6 @@ def sp_executesql_sql(sql, types, params, name)
           sql.freeze
         end
 
-        def raw_connection_do(sql)
-          result = ensure_established_connection! { dblib_execute(sql) }
-          result.do
-        ensure
-          @update_sql = false
-        end
-
         # === SQLServer Specific (Identity Inserts) ===================== #
 
         def use_output_inserted?
@@ -422,21 +400,14 @@ def identity_columns(table_name)
 
         # === SQLServer Specific (Selecting) ============================ #
 
-        def raw_select(sql, name = "SQL", binds = [], options = {})
-          log(sql, name, binds, async: options[:async]) { _raw_select(sql, options) }
-        end
+        def _raw_select(sql, conn, options = {})
+          handle = internal_raw_execute(sql, conn)
 
-        def _raw_select(sql, options = {})
-          handle = raw_connection_run(sql)
           handle_to_names_and_values(handle, options)
         ensure
           finish_statement_handle(handle)
         end
 
-        def raw_connection_run(sql)
-          ensure_established_connection! { dblib_execute(sql) }
-        end
-
         def handle_to_names_and_values(handle, options = {})
           query_options = {}.tap do |qo|
             qo[:timezone] = ActiveRecord.default_timezone || :utc
@@ -453,33 +424,16 @@ def finish_statement_handle(handle)
           handle
         end
 
-        # TODO: Rename
-        def _execute(sql, conn, perform_do: false)
+        # 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|
-            # 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.
             raise TinyTds::Error, "failed to execute statement" if _result.is_a?(FalseClass)
           end
 
           perform_do ? result.do : result
         end
-
-        # TODO: Remove
-        def dblib_execute(sql)
-          @raw_connection.execute(sql).tap do |result|
-            # TinyTDS returns false instead of raising an exception if connection fails.
-            # Getting around this by raising an exception ourselves while this PR
-            # https://github.com/rails-sqlserver/tiny_tds/pull/469 is not released.
-            raise TinyTds::Error, "failed to execute statement" if result.is_a?(FalseClass)
-          end
-        end
-
-        def ensure_established_connection!
-          raise TinyTds::Error, 'SQL Server client is not connected' unless @raw_connection
-
-          yield
-        end
       end
     end
   end

lib/active_record/connection_adapters/sqlserver/schema_statements.rb

@@ -124,7 +124,7 @@ def primary_keys_select(table_name)
           binds << Relation::QueryAttribute.new("TABLE_NAME", identifier.object, nv128)
           binds << Relation::QueryAttribute.new("TABLE_SCHEMA", identifier.schema, nv128) unless identifier.schema.blank?
 
-          sp_executesql(sql, "SCHEMA", binds).map { |r| r["name"] }
+          internal_exec_query(sql, "SCHEMA", binds).map { |row| row["name"] }
         end
 
         def rename_table(table_name, new_name, **options)
@@ -424,12 +424,13 @@ def column_definitions(table_name)
           view_tblnm  = view_table_name(table_name) if view_exists
 
           if view_exists
-            results = sp_executesql %{
+            sql = <<~SQL
               SELECT LOWER(c.COLUMN_NAME) AS [name], c.COLUMN_DEFAULT AS [default]
               FROM #{database}.INFORMATION_SCHEMA.COLUMNS c
               WHERE c.TABLE_NAME = #{quote(view_tblnm)}
-            }.squish, "SCHEMA", []
-            default_functions = results.each.with_object({}) {|row, out| out[row["name"]] = row["default"] }.compact
+            SQL
+            results = internal_exec_query(sql, "SCHEMA")
+            default_functions = results.each.with_object({}) { |row, out| out[row["name"]] = row["default"] }.compact
           end
 
           sql = column_definitions_sql(database, identifier)
@@ -438,7 +439,8 @@ def column_definitions(table_name)
           nv128 = SQLServer::Type::UnicodeVarchar.new limit: 128
           binds << Relation::QueryAttribute.new("TABLE_NAME", identifier.object, nv128)
           binds << Relation::QueryAttribute.new("TABLE_SCHEMA", identifier.schema, nv128) unless identifier.schema.blank?
-          results = sp_executesql(sql, "SCHEMA", binds)
+          results = internal_exec_query(sql, "SCHEMA", binds)
+
           columns = results.map do |ci|
             ci = ci.symbolize_keys
             ci[:_type] = ci[:type]

lib/active_record/connection_adapters/sqlserver/showplan.rb

@@ -14,7 +14,7 @@ module Showplan
 
         def explain(arel, binds = [], options = [])
           sql = to_sql(arel)
-          result = with_showplan_on { sp_executesql(sql, "EXPLAIN", binds) }
+          result = with_showplan_on { internal_exec_query(sql, "EXPLAIN", binds) }
           printer = showplan_printer.new(result)
           printer.pp
         end
@@ -30,7 +30,7 @@ def with_showplan_on
 
         def set_showplan_option(enable = true)
           sql = "SET #{showplan_option} #{enable ? 'ON' : 'OFF'}"
-          raw_connection_do(sql)
+          raw_execute(sql, "SCHEMA")
         rescue Exception
           raise ActiveRecordError, "#{showplan_option} could not be turned #{enable ? 'ON' : 'OFF'}, perhaps you do not have SHOWPLAN permissions?"
         end

lib/active_record/connection_adapters/sqlserver_adapter.rb

@@ -234,10 +234,7 @@ def disable_referential_integrity
       # === Abstract Adapter (Connection Management) ================== #
 
       def active?
-        return false unless @raw_connection
-
-        raw_connection_do "SELECT 1"
-        true
+        @raw_connection&.active?
       rescue *connection_errors
         false
       end
@@ -504,7 +501,7 @@ def version_year
       end
 
       def sqlserver_version
-        @sqlserver_version ||= _raw_select("SELECT @@version", fetch: :rows).first.first.to_s
+        @sqlserver_version ||= _raw_select("SELECT @@version", @raw_connection, fetch: :rows).first.first.to_s
       end
 
       private
@@ -529,7 +526,7 @@ def configure_connection
         @raw_connection.execute("SET TEXTSIZE 2147483647").do
         @raw_connection.execute("SET CONCAT_NULL_YIELDS_NULL ON").do
 
-        @spid = _raw_select("SELECT @@SPID", fetch: :rows).first.first
+        @spid = _raw_select("SELECT @@SPID", @raw_connection, fetch: :rows).first.first
 
         initialize_dateformatter
         use_database