Skip to content

Commit 906d8f4

Browse files
byrootadrianna-chang-shopify
authored andcommitted
Get rid of with_multi_statement
It's a noop on all adapters except MySQL which doesn't allow multi statements by default. Also `batch_execute` in Sqlite3Adapter has to use many ow level APIs because while sqlite3 allow multi statement by default, you have to use a different API for that. So overall `with_multi_statement` was always a lie, because it wouldn't work on sqlite3 and it's simpler to pass a `batch: true` argument to `raw_execute` so that each adapter can do the necessary work to allow multi-statement. This will simplify a future PR that Adrianna will open soon. Co-Authored-By: Adrianna Chang <[email protected]>
1 parent 116e8f0 commit 906d8f4

File tree

5 files changed

+36
-66
lines changed

5 files changed

+36
-66
lines changed

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

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -224,11 +224,9 @@ def truncate_tables(*table_names) # :nodoc:
224224

225225
return if table_names.empty?
226226

227-
with_multi_statements do
228-
disable_referential_integrity do
229-
statements = build_truncate_statements(table_names)
230-
execute_batch(statements, "Truncate Tables")
231-
end
227+
disable_referential_integrity do
228+
statements = build_truncate_statements(table_names)
229+
execute_batch(statements, "Truncate Tables")
232230
end
233231
end
234232

@@ -490,11 +488,9 @@ def insert_fixtures_set(fixture_set, tables_to_delete = [])
490488
table_deletes = tables_to_delete.map { |table| "DELETE FROM #{quote_table_name(table)}" }
491489
statements = table_deletes + fixture_inserts
492490

493-
with_multi_statements do
494-
transaction(requires_new: true) do
495-
disable_referential_integrity do
496-
execute_batch(statements, "Fixtures Load")
497-
end
491+
transaction(requires_new: true) do
492+
disable_referential_integrity do
493+
execute_batch(statements, "Fixtures Load")
498494
end
499495
end
500496
end
@@ -553,11 +549,11 @@ def internal_exec_query(...) # :nodoc:
553549

554550
private
555551
# Lowest level way to execute a query. Doesn't check for illegal writes, doesn't annotate queries, yields a native result object.
556-
def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true)
552+
def raw_execute(sql, name = nil, binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true, batch: false)
557553
type_casted_binds = type_casted_binds(binds)
558554
log(sql, name, binds, type_casted_binds, async: async) do |notification_payload|
559555
with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn|
560-
perform_query(conn, sql, binds, type_casted_binds, prepare: prepare, notification_payload: notification_payload)
556+
perform_query(conn, sql, binds, type_casted_binds, prepare: prepare, notification_payload: notification_payload, batch: batch)
561557
end
562558
end
563559
end
@@ -667,10 +663,6 @@ def build_truncate_statements(table_names)
667663
end
668664
end
669665

670-
def with_multi_statements
671-
yield
672-
end
673-
674666
def combine_multi_statements(total_sql)
675667
total_sql.join(";\n")
676668
end

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

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ def select_all(*, **) # :nodoc:
1616
private
1717
def execute_batch(statements, name = nil)
1818
combine_multi_statements(statements).each do |statement|
19-
with_raw_connection do |conn|
20-
raw_execute(statement, name)
21-
end
19+
raw_execute(statement, name, batch: true)
2220
end
2321
end
2422

@@ -40,21 +38,12 @@ def multi_statements_enabled?
4038
end
4139
end
4240

43-
def with_multi_statements
44-
if multi_statements_enabled?
45-
return yield
46-
end
47-
48-
with_raw_connection do |conn|
49-
conn.set_server_option(::Mysql2::Client::OPTION_MULTI_STATEMENTS_ON)
50-
51-
yield
52-
ensure
53-
conn.set_server_option(::Mysql2::Client::OPTION_MULTI_STATEMENTS_OFF)
41+
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch: false)
42+
reset_multi_statement = if batch && !multi_statements_enabled?
43+
raw_connection.set_server_option(::Mysql2::Client::OPTION_MULTI_STATEMENTS_ON)
44+
true
5445
end
55-
end
5646

57-
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:)
5847
# Make sure we carry over any changes to ActiveRecord.default_timezone that have been
5948
# made since we established the connection
6049
raw_connection.query_options[:database_timezone] = default_timezone
@@ -84,6 +73,10 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
8473
verified!
8574
handle_warnings(sql)
8675
result
76+
ensure
77+
if reset_multi_statement && active?
78+
raw_connection.set_server_option(::Mysql2::Client::OPTION_MULTI_STATEMENTS_OFF)
79+
end
8780
end
8881

8982
def cast_result(result)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ def cancel_any_running_query
132132
rescue PG::Error
133133
end
134134

135-
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:)
135+
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch: false)
136136
update_typemap_for_default_timezone
137137
result = if prepare
138138
begin
@@ -193,7 +193,7 @@ def affected_rows(result)
193193
end
194194

195195
def execute_batch(statements, name = nil)
196-
raw_execute(combine_multi_statements(statements), name)
196+
raw_execute(combine_multi_statements(statements), name, batch: true)
197197
end
198198

199199
def build_truncate_statements(table_names)

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,10 @@ def internal_begin_transaction(mode, isolation)
7575
end
7676
end
7777

78-
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:)
79-
if prepare
78+
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch: false)
79+
if batch
80+
raw_connection.execute_batch2(sql)
81+
elsif prepare
8082
stmt = @statements[sql] ||= raw_connection.prepare(sql)
8183
stmt.reset!
8284
stmt.bind_params(type_casted_binds)
@@ -107,7 +109,7 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
107109
@last_affected_rows = raw_connection.changes
108110
verified!
109111

110-
notification_payload[:row_count] = result.length
112+
notification_payload[:row_count] = result&.length || 0
111113
result
112114
end
113115

@@ -123,13 +125,7 @@ def affected_rows(result)
123125

124126
def execute_batch(statements, name = nil)
125127
sql = combine_multi_statements(statements)
126-
127-
log(sql, name) do
128-
with_raw_connection do |conn|
129-
conn.execute_batch2(sql)
130-
verified!
131-
end
132-
end
128+
raw_execute(sql, name, batch: true)
133129
end
134130

135131
def build_fixture_statements(fixture_set)

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

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,12 @@ def exec_insert(sql, name, binds, pk = nil, sequence_name = nil, returning: nil)
1010
end
1111

1212
private
13-
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:)
13+
def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notification_payload:, batch: false)
14+
reset_multi_statement = if batch && !@config[:multi_statement]
15+
raw_connection.set_server_option(::Trilogy::SET_SERVER_MULTI_STATEMENTS_ON)
16+
true
17+
end
18+
1419
# Make sure we carry over any changes to ActiveRecord.default_timezone that have been
1520
# made since we established the connection
1621
if default_timezone == :local
@@ -27,6 +32,10 @@ def perform_query(raw_connection, sql, binds, type_casted_binds, prepare:, notif
2732
handle_warnings(sql)
2833
notification_payload[:row_count] = result.count
2934
result
35+
ensure
36+
if reset_multi_statement && active?
37+
raw_connection.set_server_option(::Trilogy::SET_SERVER_MULTI_STATEMENTS_OFF)
38+
end
3039
end
3140

3241
def cast_result(result)
@@ -51,27 +60,7 @@ def last_inserted_id(result)
5160

5261
def execute_batch(statements, name = nil)
5362
combine_multi_statements(statements).each do |statement|
54-
with_raw_connection do |conn|
55-
raw_execute(statement, name)
56-
end
57-
end
58-
end
59-
60-
def multi_statements_enabled?
61-
!!@config[:multi_statement]
62-
end
63-
64-
def with_multi_statements
65-
if multi_statements_enabled?
66-
return yield
67-
end
68-
69-
with_raw_connection do |conn|
70-
conn.set_server_option(::Trilogy::SET_SERVER_MULTI_STATEMENTS_ON)
71-
72-
yield
73-
ensure
74-
conn.set_server_option(::Trilogy::SET_SERVER_MULTI_STATEMENTS_OFF) if active?
63+
raw_execute(statement, name, batch: true)
7564
end
7665
end
7766
end

0 commit comments

Comments
 (0)