From 8b3c4e53dd7819beb69f10a577f3915e3b6f56f3 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Mon, 9 Dec 2024 14:51:03 +0000 Subject: [PATCH 1/3] Add affected_rows to sql.active_record --- .../sqlserver/database_statements.rb | 11 ++++++++++- test/cases/coerced_tests.rb | 7 +++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 9e36d61aa..a4bb95831 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -26,6 +26,8 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif end verified! + + notification_payload[:affected_rows] = affected_rows(result) notification_payload[:row_count] = result.count result end @@ -39,7 +41,7 @@ def cast_result(raw_result) end def affected_rows(raw_result) - raw_result.first['AffectedRows'] + raw_result&.first&.fetch('AffectedRows', 0) || 0 end def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false) @@ -68,6 +70,11 @@ def exec_update(sql, name = nil, binds = []) super(sql, name, binds) end + def exec_insert_all(sql, name) + sql = sql.dup << "; SELECT @@ROWCOUNT AS AffectedRows" + super(sql, name) + end + def begin_db_transaction internal_execute("BEGIN TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false) end @@ -179,6 +186,8 @@ def execute_procedure(proc_name, *variables) end result = result.each.map { |row| row.is_a?(Hash) ? row.with_indifferent_access : row } + + notification_payload[:affected_rows] = affected_rows(result) notification_payload[:row_count] = result.count result end diff --git a/test/cases/coerced_tests.rb b/test/cases/coerced_tests.rb index 1ffa0694c..01d08ddfd 100644 --- a/test/cases/coerced_tests.rb +++ b/test/cases/coerced_tests.rb @@ -372,6 +372,13 @@ def test_payload_row_count_on_raw_sql_coerced Book.where(author_id: nil, name: 'row count book 3').delete_all Book.lease_connection.add_index(:books, [:author_id, :name], unique: true) end + + # Fix randomly failing test. The loading of the model's schema was affecting the test. + coerce_tests! :test_payload_affected_rows + def test_payload_affected_rows_coerced + Book.send(:load_schema!) + original_test_payload_affected_rows + end end end From 17d0b781b4181f5dce4bad706e29455627b4f318 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Tue, 10 Dec 2024 11:32:59 +0000 Subject: [PATCH 2/3] Update database_statements.rb --- .../sqlserver/database_statements.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index a4bb95831..8622cc026 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -27,6 +27,8 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif verified! + # binding.pry if $DEBUG + notification_payload[:affected_rows] = affected_rows(result) notification_payload[:row_count] = result.count result @@ -41,6 +43,9 @@ def cast_result(raw_result) end def affected_rows(raw_result) + + # raw_result.first['AffectedRows'] + raw_result&.first&.fetch('AffectedRows', 0) || 0 end @@ -70,10 +75,10 @@ def exec_update(sql, name = nil, binds = []) super(sql, name, binds) end - def exec_insert_all(sql, name) - sql = sql.dup << "; SELECT @@ROWCOUNT AS AffectedRows" - super(sql, name) - end + # def exec_insert_all(sql, name) + # sql = sql.dup << "; SELECT @@ROWCOUNT AS AffectedRows" + # super(sql, name) + # end def begin_db_transaction internal_execute("BEGIN TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false) From 8269e39fa5a214d63fc381b813fd3c545e96693b Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 11 Dec 2024 15:29:51 +0000 Subject: [PATCH 3/3] Read affected rows from either results or handle --- .../sqlserver/database_statements.rb | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 8622cc026..fe286a268 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -14,22 +14,19 @@ def write_query?(sql) # :nodoc: 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) - # If the table name is a view, we need to get the base table name for enabling identity insert. - id_insert_table_name = view_table_name(id_insert_table_name) if view_exists?(id_insert_table_name) + result, affected_rows = if id_insert_table_name = query_requires_identity_insert?(sql) + # If the table name is a view, we need to get the base table name for enabling identity insert. + id_insert_table_name = view_table_name(id_insert_table_name) if view_exists?(id_insert_table_name) - 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 + 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! - - # binding.pry if $DEBUG - - notification_payload[:affected_rows] = affected_rows(result) + notification_payload[:affected_rows] = affected_rows notification_payload[:row_count] = result.count result end @@ -42,11 +39,18 @@ def cast_result(raw_result) end end + # Returns the affected rows from results. def affected_rows(raw_result) + raw_result&.first&.fetch('AffectedRows', nil) + end - # raw_result.first['AffectedRows'] - - raw_result&.first&.fetch('AffectedRows', 0) || 0 + # Returns the affected rows from results or handle. + def affected_rows_from_results_or_handle(raw_result, handle) + if affected_rows_from_result = affected_rows(raw_result) + affected_rows_from_result + else + handle.affected_rows + end end def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false) @@ -60,7 +64,9 @@ def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow def internal_exec_sql_query(sql, conn) handle = internal_raw_execute(sql, conn) - handle_to_names_and_values(handle, ar_result: true) + results = handle_to_names_and_values(handle, ar_result: true) + + return results, affected_rows_from_results_or_handle(results, handle) ensure finish_statement_handle(handle) end @@ -75,11 +81,6 @@ def exec_update(sql, name = nil, binds = []) super(sql, name, binds) end - # def exec_insert_all(sql, name) - # sql = sql.dup << "; SELECT @@ROWCOUNT AS AffectedRows" - # super(sql, name) - # end - def begin_db_transaction internal_execute("BEGIN TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false) end @@ -191,8 +192,6 @@ def execute_procedure(proc_name, *variables) end result = result.each.map { |row| row.is_a?(Hash) ? row.with_indifferent_access : row } - - notification_payload[:affected_rows] = affected_rows(result) notification_payload[:row_count] = result.count result end @@ -446,12 +445,15 @@ def handle_to_names_and_values(handle, options = {}) end results = handle.each(query_options) - columns = handle.fields - # If query returns multiple result sets, only return the columns of the last one. - columns = columns.last if columns.any? && columns.all? { |e| e.is_a?(Array) } - columns = columns.map(&:downcase) if lowercase_schema_reflection + if options[:ar_result] + columns = handle.fields + columns = columns.last if columns.any? && columns.all? { |e| e.is_a?(Array) } # If query returns multiple result sets, only return the columns of the last one. + columns = columns.map(&:downcase) if lowercase_schema_reflection - options[:ar_result] ? ActiveRecord::Result.new(columns, results) : results + ActiveRecord::Result.new(columns, results) + else + results + end end def finish_statement_handle(handle)