Skip to content

Commit 338e1f7

Browse files
eileencodeseileencodes
committed
Revert "Merge pull request rails#48069 from Shopify/ar-exec-query-flush-cache"
This reverts commit 663df3a, reversing changes made to 9b4fff2. The changes here forced our code through a different codepath, circumventing the patch on `execute` to retry queries. Since we also patch `execute` from Semian it's not clear the correct path forward and we're going to revert for now.
1 parent 8d56a0e commit 338e1f7

16 files changed

+43
-79
lines changed

‎activerecord/CHANGELOG.md

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,6 @@
5757

5858
*Hiroyuki Ishii*
5959

60-
* `AbstractAdapter#execute` and `#exec_query` now clear the query cache
61-
62-
If you need to perform a read only SQL query without clearing the query
63-
cache, use `AbstractAdapter#select_all`.
64-
65-
*Jean Boussier*
66-
6760
* Make `.joins` / `.left_outer_joins` work with CTEs.
6861

6962
For example:

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

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def query_values(sql, name = nil) # :nodoc:
105105
end
106106

107107
def query(sql, name = nil) # :nodoc:
108-
internal_exec_query(sql, name).rows
108+
exec_query(sql, name).rows
109109
end
110110

111111
# Determines whether the SQL statement is a write query.
@@ -120,12 +120,8 @@ def write_query?(sql)
120120
# executing the SQL statement in case of a connection-related exception.
121121
# This option should only be enabled for known idempotent queries.
122122
#
123-
# Note: the query is assumed to have side effects and the query cache
124-
# will be cleared. If the query is read-only, consider using #select_all
125-
# instead.
126-
#
127123
# Note: depending on your database connector, the result returned by this
128-
# method may be manually memory managed. Consider using #exec_query
124+
# method may be manually memory managed. Consider using the exec_query
129125
# wrapper instead.
130126
def execute(sql, name = nil, allow_retry: false)
131127
internal_execute(sql, name, allow_retry: allow_retry)
@@ -134,38 +130,34 @@ def execute(sql, name = nil, allow_retry: false)
134130
# Executes +sql+ statement in the context of this connection using
135131
# +binds+ as the bind substitutes. +name+ is logged along with
136132
# the executed +sql+ statement.
137-
#
138-
# Note: the query is assumed to have side effects and the query cache
139-
# will be cleared. If the query is read-only, consider using #select_all
140-
# instead.
141133
def exec_query(sql, name = "SQL", binds = [], prepare: false)
142-
internal_exec_query(sql, name, binds, prepare: prepare)
134+
raise NotImplementedError
143135
end
144136

145137
# Executes insert +sql+ statement in the context of this connection using
146138
# +binds+ as the bind substitutes. +name+ is logged along with
147139
# the executed +sql+ statement.
148140
def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil)
149141
sql, binds = sql_for_insert(sql, pk, binds)
150-
internal_exec_query(sql, name, binds)
142+
exec_query(sql, name, binds)
151143
end
152144

153145
# Executes delete +sql+ statement in the context of this connection using
154146
# +binds+ as the bind substitutes. +name+ is logged along with
155147
# the executed +sql+ statement.
156148
def exec_delete(sql, name = nil, binds = [])
157-
internal_exec_query(sql, name, binds)
149+
exec_query(sql, name, binds)
158150
end
159151

160152
# Executes update +sql+ statement in the context of this connection using
161153
# +binds+ as the bind substitutes. +name+ is logged along with
162154
# the executed +sql+ statement.
163155
def exec_update(sql, name = nil, binds = [])
164-
internal_exec_query(sql, name, binds)
156+
exec_query(sql, name, binds)
165157
end
166158

167159
def exec_insert_all(sql, name) # :nodoc:
168-
internal_exec_query(sql, name)
160+
exec_query(sql, name)
169161
end
170162

171163
def explain(arel, binds = [], options = []) # :nodoc:
@@ -498,10 +490,6 @@ def high_precision_current_timestamp
498490
HIGH_PRECISION_CURRENT_TIMESTAMP
499491
end
500492

501-
def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
502-
raise NotImplementedError
503-
end
504-
505493
private
506494
def internal_execute(sql, name = "SCHEMA", allow_retry: false, materialize_transactions: true)
507495
sql = transform_query(sql)
@@ -618,7 +606,7 @@ def select(sql, name = nil, binds = [], prepare: false, async: false)
618606
return future_result
619607
end
620608

621-
result = internal_exec_query(sql, name, binds, prepare: prepare)
609+
result = exec_query(sql, name, binds, prepare: prepare)
622610
if async
623611
FutureResult::Complete.new(result)
624612
else

‎activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@ module QueryCache
99

1010
class << self
1111
def included(base) # :nodoc:
12-
dirties_query_cache base, :exec_query, :execute, :create, :insert, :update, :delete, :truncate,
13-
:truncate_tables, :rollback_to_savepoint, :rollback_db_transaction, :restart_db_transaction,
14-
:exec_insert_all
12+
dirties_query_cache base, :execute, :create, :insert, :update, :delete, :truncate, :truncate_tables,
13+
:rollback_to_savepoint, :rollback_db_transaction, :restart_db_transaction, :exec_insert_all
1514

1615
base.set_callback :checkout, :after, :configure_query_cache!
1716
base.set_callback :checkin, :after, :disable_query_cache!
@@ -97,7 +96,7 @@ def clear_query_cache
9796
end
9897
end
9998

100-
def select_all(arel, name = nil, binds = [], preparable: nil, async: false) # :nodoc:
99+
def select_all(arel, name = nil, binds = [], preparable: nil, async: false)
101100
arel = arel_from_relation(arel)
102101

103102
# If arel is locked this is a SELECT ... FOR UPDATE or somesuch.

‎activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ def foreign_keys(table_name)
466466

467467
scope = quoted_scope(table_name)
468468

469-
fk_info = internal_exec_query(<<~SQL, "SCHEMA")
469+
fk_info = exec_query(<<~SQL, "SCHEMA")
470470
SELECT fk.referenced_table_name AS 'to_table',
471471
fk.referenced_column_name AS 'primary_key',
472472
fk.column_name AS 'column',
@@ -513,7 +513,7 @@ def check_constraints(table_name)
513513
SQL
514514
sql += " AND cc.table_name = #{scope[:name]}" if mariadb?
515515

516-
chk_info = internal_exec_query(sql, "SCHEMA")
516+
chk_info = exec_query(sql, "SCHEMA")
517517

518518
chk_info.map do |row|
519519
options = {
@@ -819,7 +819,7 @@ def rename_column_for_alter(table_name, column_name, new_column_name)
819819
comment: column.comment
820820
}
821821

822-
current_type = internal_exec_query("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE #{quote(column_name)}", "SCHEMA").first["Type"]
822+
current_type = exec_query("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE #{quote(column_name)}", "SCHEMA").first["Type"]
823823
td = create_table_definition(table_name)
824824
cd = td.new_column_definition(new_column_name, current_type, **options)
825825
schema_creation.accept(ChangeColumnDefinition.new(cd, column.name))
@@ -909,7 +909,7 @@ def column_definitions(table_name) # :nodoc:
909909
end
910910

911911
def create_table_info(table_name) # :nodoc:
912-
internal_exec_query("SHOW CREATE TABLE #{quote_table_name(table_name)}", "SCHEMA").first["Create Table"]
912+
exec_query("SHOW CREATE TABLE #{quote_table_name(table_name)}", "SCHEMA").first["Create Table"]
913913
end
914914

915915
def arel_visitor

‎activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def high_precision_current_timestamp
2727
def explain(arel, binds = [], options = [])
2828
sql = build_explain_clause(options) + " " + to_sql(arel, binds)
2929
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
30-
result = internal_exec_query(sql, "EXPLAIN", binds)
30+
result = exec_query(sql, "EXPLAIN", binds)
3131
elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
3232

3333
MySQL::ExplainPrettyPrinter.new.pp(result, elapsed)

‎activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def schema_collation(column)
6666
if column.collation
6767
@table_collation_cache ||= {}
6868
@table_collation_cache[table_name] ||=
69-
@connection.internal_exec_query("SHOW TABLE STATUS LIKE #{@connection.quote(table_name)}", "SCHEMA").first["Collation"]
69+
@connection.exec_query("SHOW TABLE STATUS LIKE #{@connection.quote(table_name)}", "SCHEMA").first["Collation"]
7070
column.collation.inspect if column.collation != @table_collation_cache[table_name]
7171
end
7272
end

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def select_all(*, **) # :nodoc:
1818
result
1919
end
2020

21-
def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
21+
def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
2222
if without_prepared_statement?(binds)
2323
execute_and_free(sql, name, async: async) do |result|
2424
if result

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module PostgreSQL
66
module DatabaseStatements
77
def explain(arel, binds = [], options = [])
88
sql = build_explain_clause(options) + " " + to_sql(arel, binds)
9-
result = internal_exec_query(sql, "EXPLAIN", binds)
9+
result = exec_query(sql, "EXPLAIN", binds)
1010
PostgreSQL::ExplainPrettyPrinter.new.pp(result)
1111
end
1212

@@ -57,7 +57,7 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac
5757
end
5858
end
5959

60-
def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true) # :nodoc:
60+
def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true) # :nodoc:
6161
execute_and_clear(sql, name, binds, prepare: prepare, async: async, allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |result|
6262
types = {}
6363
fields = result.fields
@@ -94,7 +94,7 @@ def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil) # :n
9494
if use_insert_returning? || pk == false
9595
super
9696
else
97-
result = internal_exec_query(sql, name, binds)
97+
result = exec_query(sql, name, binds)
9898
unless sequence_name
9999
table_ref = extract_table_ref_from_insert_sql(sql)
100100
if table_ref
@@ -169,7 +169,7 @@ def build_truncate_statements(table_names)
169169

170170
# Returns the current ID of a table's sequence.
171171
def last_insert_id_result(sequence_name)
172-
internal_exec_query("SELECT currval(#{quote(sequence_name)})", "SQL")
172+
exec_query("SELECT currval(#{quote(sequence_name)})", "SQL")
173173
end
174174

175175
def suppress_composite_primary_key(pk)

‎activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ def add_foreign_key(from_table, to_table, **options)
534534

535535
def foreign_keys(table_name)
536536
scope = quoted_scope(table_name)
537-
fk_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
537+
fk_info = exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
538538
SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid, c.condeferrable AS deferrable, c.condeferred AS deferred
539539
FROM pg_constraint c
540540
JOIN pg_class t1 ON c.conrelid = t1.oid
@@ -577,7 +577,7 @@ def foreign_table_exists?(table_name)
577577
def check_constraints(table_name) # :nodoc:
578578
scope = quoted_scope(table_name)
579579

580-
check_info = internal_exec_query(<<-SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
580+
check_info = exec_query(<<-SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
581581
SELECT conname, pg_get_constraintdef(c.oid, true) AS constraintdef, c.convalidated AS valid
582582
FROM pg_constraint c
583583
JOIN pg_class t ON c.conrelid = t.oid
@@ -603,7 +603,7 @@ def check_constraints(table_name) # :nodoc:
603603
def exclusion_constraints(table_name)
604604
scope = quoted_scope(table_name)
605605

606-
exclusion_info = internal_exec_query(<<-SQL, "SCHEMA")
606+
exclusion_info = exec_query(<<-SQL, "SCHEMA")
607607
SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.condeferrable, c.condeferred
608608
FROM pg_constraint c
609609
JOIN pg_class t ON c.conrelid = t.oid
@@ -637,7 +637,7 @@ def exclusion_constraints(table_name)
637637
def unique_keys(table_name)
638638
scope = quoted_scope(table_name)
639639

640-
unique_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
640+
unique_info = exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
641641
SELECT c.conname, c.conindid, c.condeferrable, c.condeferred
642642
FROM pg_constraint c
643643
JOIN pg_class t ON c.conrelid = t.oid

‎activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ def enable_extension(name, **)
459459
sql = +"CREATE EXTENSION IF NOT EXISTS \"#{name}\""
460460
sql << " SCHEMA #{schema}" if schema
461461

462-
internal_exec_query(sql).tap { reload_type_map }
462+
exec_query(sql).tap { reload_type_map }
463463
end
464464

465465
# Removes an extension from the database.
@@ -468,7 +468,7 @@ def enable_extension(name, **)
468468
# Set to +:cascade+ to drop dependent objects as well.
469469
# Defaults to false.
470470
def disable_extension(name, force: false)
471-
internal_exec_query("DROP EXTENSION IF EXISTS \"#{name}\"#{' CASCADE' if force == :cascade}").tap {
471+
exec_query("DROP EXTENSION IF EXISTS \"#{name}\"#{' CASCADE' if force == :cascade}").tap {
472472
reload_type_map
473473
}
474474
end
@@ -482,7 +482,7 @@ def extension_enabled?(name)
482482
end
483483

484484
def extensions
485-
internal_exec_query("SELECT extname FROM pg_extension", "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values
485+
exec_query("SELECT extname FROM pg_extension", "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values
486486
end
487487

488488
# Returns a list of defined enum types, and their values.
@@ -500,7 +500,7 @@ def enum_types
500500
GROUP BY type.OID, n.nspname, type.typname;
501501
SQL
502502

503-
internal_exec_query(query, "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values.each_with_object({}) do |row, memo|
503+
exec_query(query, "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values.each_with_object({}) do |row, memo|
504504
name, schema = row[0], row[2]
505505
schema = nil if schema == current_schema
506506
full_name = [schema, name].compact.join(".")
@@ -527,7 +527,7 @@ def create_enum(name, values, **options)
527527
END
528528
$$;
529529
SQL
530-
internal_exec_query(query)
530+
exec_query(query)
531531
end
532532

533533
# Drops an enum type.
@@ -543,7 +543,7 @@ def drop_enum(name, values = nil, **options)
543543
query = <<~SQL
544544
DROP TYPE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(name)};
545545
SQL
546-
internal_exec_query(query)
546+
exec_query(query)
547547
end
548548

549549
# Returns the configured supported identifier length supported by PostgreSQL

0 commit comments

Comments
 (0)