Skip to content

Commit 4432403

Browse files
jenshennyskipkayhil
andcommitted
Fix affected_rows for SQLite sql.active_record notifications
SQLite's connection.changes only reflects the number of rows affected by the last DELETE/UPDATE/INSERT, and the number would not change for any other query. Therefore, SELECT statements would have the affected row count of a previous query. To solve this, we are now calculating the different between the total changes before and after the query has been executed. Co-authored-by: Hartley McGuire <[email protected]>
1 parent a3b37ff commit 4432403

File tree

2 files changed

+12
-4
lines changed

2 files changed

+12
-4
lines changed

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ def internal_begin_transaction(mode, isolation)
8484
end
8585

8686
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch: false)
87+
total_changes_before_query = raw_connection.total_changes
88+
8789
if batch
8890
raw_connection.execute_batch2(sql)
8991
elsif prepare
@@ -114,10 +116,10 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
114116
stmt.close
115117
end
116118
end
117-
@last_affected_rows = raw_connection.changes
119+
@affected_rows = raw_connection.total_changes - total_changes_before_query
118120
verified!
119121

120-
notification_payload[:affected_rows] = @last_affected_rows
122+
notification_payload[:affected_rows] = @affected_rows
121123
notification_payload[:row_count] = result&.length || 0
122124
result
123125
end
@@ -129,7 +131,7 @@ def cast_result(result)
129131
end
130132

131133
def affected_rows(result)
132-
@last_affected_rows
134+
@affected_rows
133135
end
134136

135137
def execute_batch(statements, name = nil, **kwargs)

activerecord/test/cases/instrumentation_test.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,14 +165,20 @@ def test_payload_affected_rows
165165
# INSERT ... RETURNING
166166
Book.insert_all!([{ name: "One" }, { name: "Two" }, { name: "Three" }, { name: "Four" }], returning: false)
167167

168+
Book.where(name: ["One", "Two", "Three"]).pluck(:id)
169+
168170
Book.where(name: ["One", "Two", "Three"]).update_all(status: :published)
169171

170172
Book.where(name: ["Three", "Four"]).delete_all
171173

172174
Book.where(name: ["Three", "Four"]).delete_all
173175
end
174176

175-
assert_equal [4, 3, 2, 0], affected_row_values
177+
assert_equal 4, affected_row_values.first
178+
assert_not_equal affected_row_values.first, affected_row_values.second
179+
assert_equal 3, affected_row_values.third
180+
assert_equal 2, affected_row_values.fourth
181+
assert_equal 0, affected_row_values.fifth
176182
end
177183

178184
def test_no_instantiation_notification_when_no_records

0 commit comments

Comments
 (0)