Skip to content

Commit

Permalink
Revert "Merge pull request #48069 from Shopify/ar-exec-query-flush-ca…
Browse files Browse the repository at this point in the history
…che"

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.
  • Loading branch information
eileencodes committed May 10, 2023
1 parent 8d56a0e commit 338e1f7
Show file tree
Hide file tree
Showing 16 changed files with 43 additions and 79 deletions.
7 changes: 0 additions & 7 deletions activerecord/CHANGELOG.md
Expand Up @@ -57,13 +57,6 @@

*Hiroyuki Ishii*

* `AbstractAdapter#execute` and `#exec_query` now clear the query cache

If you need to perform a read only SQL query without clearing the query
cache, use `AbstractAdapter#select_all`.

*Jean Boussier*

* Make `.joins` / `.left_outer_joins` work with CTEs.

For example:
Expand Down
Expand Up @@ -105,7 +105,7 @@ def query_values(sql, name = nil) # :nodoc:
end

def query(sql, name = nil) # :nodoc:
internal_exec_query(sql, name).rows
exec_query(sql, name).rows
end

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

# Executes insert +sql+ statement in the context of this connection using
# +binds+ as the bind substitutes. +name+ is logged along with
# the executed +sql+ statement.
def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil)
sql, binds = sql_for_insert(sql, pk, binds)
internal_exec_query(sql, name, binds)
exec_query(sql, name, binds)
end

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

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

def exec_insert_all(sql, name) # :nodoc:
internal_exec_query(sql, name)
exec_query(sql, name)
end

def explain(arel, binds = [], options = []) # :nodoc:
Expand Down Expand Up @@ -498,10 +490,6 @@ def high_precision_current_timestamp
HIGH_PRECISION_CURRENT_TIMESTAMP
end

def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
raise NotImplementedError
end

private
def internal_execute(sql, name = "SCHEMA", allow_retry: false, materialize_transactions: true)
sql = transform_query(sql)
Expand Down Expand Up @@ -618,7 +606,7 @@ def select(sql, name = nil, binds = [], prepare: false, async: false)
return future_result
end

result = internal_exec_query(sql, name, binds, prepare: prepare)
result = exec_query(sql, name, binds, prepare: prepare)
if async
FutureResult::Complete.new(result)
else
Expand Down
Expand Up @@ -9,9 +9,8 @@ module QueryCache

class << self
def included(base) # :nodoc:
dirties_query_cache base, :exec_query, :execute, :create, :insert, :update, :delete, :truncate,
:truncate_tables, :rollback_to_savepoint, :rollback_db_transaction, :restart_db_transaction,
:exec_insert_all
dirties_query_cache base, :execute, :create, :insert, :update, :delete, :truncate, :truncate_tables,
:rollback_to_savepoint, :rollback_db_transaction, :restart_db_transaction, :exec_insert_all

base.set_callback :checkout, :after, :configure_query_cache!
base.set_callback :checkin, :after, :disable_query_cache!
Expand Down Expand Up @@ -97,7 +96,7 @@ def clear_query_cache
end
end

def select_all(arel, name = nil, binds = [], preparable: nil, async: false) # :nodoc:
def select_all(arel, name = nil, binds = [], preparable: nil, async: false)
arel = arel_from_relation(arel)

# If arel is locked this is a SELECT ... FOR UPDATE or somesuch.
Expand Down
Expand Up @@ -466,7 +466,7 @@ def foreign_keys(table_name)

scope = quoted_scope(table_name)

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

chk_info = internal_exec_query(sql, "SCHEMA")
chk_info = exec_query(sql, "SCHEMA")

chk_info.map do |row|
options = {
Expand Down Expand Up @@ -819,7 +819,7 @@ def rename_column_for_alter(table_name, column_name, new_column_name)
comment: column.comment
}

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

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

def arel_visitor
Expand Down
Expand Up @@ -27,7 +27,7 @@ def high_precision_current_timestamp
def explain(arel, binds = [], options = [])
sql = build_explain_clause(options) + " " + to_sql(arel, binds)
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
result = internal_exec_query(sql, "EXPLAIN", binds)
result = exec_query(sql, "EXPLAIN", binds)
elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start

MySQL::ExplainPrettyPrinter.new.pp(result, elapsed)
Expand Down
Expand Up @@ -66,7 +66,7 @@ def schema_collation(column)
if column.collation
@table_collation_cache ||= {}
@table_collation_cache[table_name] ||=
@connection.internal_exec_query("SHOW TABLE STATUS LIKE #{@connection.quote(table_name)}", "SCHEMA").first["Collation"]
@connection.exec_query("SHOW TABLE STATUS LIKE #{@connection.quote(table_name)}", "SCHEMA").first["Collation"]
column.collation.inspect if column.collation != @table_collation_cache[table_name]
end
end
Expand Down
Expand Up @@ -18,7 +18,7 @@ def select_all(*, **) # :nodoc:
result
end

def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
if without_prepared_statement?(binds)
execute_and_free(sql, name, async: async) do |result|
if result
Expand Down
Expand Up @@ -6,7 +6,7 @@ module PostgreSQL
module DatabaseStatements
def explain(arel, binds = [], options = [])
sql = build_explain_clause(options) + " " + to_sql(arel, binds)
result = internal_exec_query(sql, "EXPLAIN", binds)
result = exec_query(sql, "EXPLAIN", binds)
PostgreSQL::ExplainPrettyPrinter.new.pp(result)
end

Expand Down Expand Up @@ -57,7 +57,7 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac
end
end

def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true) # :nodoc:
def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true) # :nodoc:
execute_and_clear(sql, name, binds, prepare: prepare, async: async, allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |result|
types = {}
fields = result.fields
Expand Down Expand Up @@ -94,7 +94,7 @@ def exec_insert(sql, name = nil, binds = [], pk = nil, sequence_name = nil) # :n
if use_insert_returning? || pk == false
super
else
result = internal_exec_query(sql, name, binds)
result = exec_query(sql, name, binds)
unless sequence_name
table_ref = extract_table_ref_from_insert_sql(sql)
if table_ref
Expand Down Expand Up @@ -169,7 +169,7 @@ def build_truncate_statements(table_names)

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

def suppress_composite_primary_key(pk)
Expand Down
Expand Up @@ -534,7 +534,7 @@ def add_foreign_key(from_table, to_table, **options)

def foreign_keys(table_name)
scope = quoted_scope(table_name)
fk_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
fk_info = exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
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
FROM pg_constraint c
JOIN pg_class t1 ON c.conrelid = t1.oid
Expand Down Expand Up @@ -577,7 +577,7 @@ def foreign_table_exists?(table_name)
def check_constraints(table_name) # :nodoc:
scope = quoted_scope(table_name)

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

exclusion_info = internal_exec_query(<<-SQL, "SCHEMA")
exclusion_info = exec_query(<<-SQL, "SCHEMA")
SELECT conname, pg_get_constraintdef(c.oid) AS constraintdef, c.condeferrable, c.condeferred
FROM pg_constraint c
JOIN pg_class t ON c.conrelid = t.oid
Expand Down Expand Up @@ -637,7 +637,7 @@ def exclusion_constraints(table_name)
def unique_keys(table_name)
scope = quoted_scope(table_name)

unique_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
unique_info = exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
SELECT c.conname, c.conindid, c.condeferrable, c.condeferred
FROM pg_constraint c
JOIN pg_class t ON c.conrelid = t.oid
Expand Down
Expand Up @@ -459,7 +459,7 @@ def enable_extension(name, **)
sql = +"CREATE EXTENSION IF NOT EXISTS \"#{name}\""
sql << " SCHEMA #{schema}" if schema

internal_exec_query(sql).tap { reload_type_map }
exec_query(sql).tap { reload_type_map }
end

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

def extensions
internal_exec_query("SELECT extname FROM pg_extension", "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values
exec_query("SELECT extname FROM pg_extension", "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values
end

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

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

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

# Returns the configured supported identifier length supported by PostgreSQL
Expand Down
Expand Up @@ -17,11 +17,11 @@ def write_query?(sql) # :nodoc:

def explain(arel, binds = [], _options = [])
sql = "EXPLAIN QUERY PLAN " + to_sql(arel, binds)
result = internal_exec_query(sql, "EXPLAIN", [])
result = exec_query(sql, "EXPLAIN", [])
SQLite3::ExplainPrettyPrinter.new.pp(result)
end

def internal_exec_query(sql, name = nil, binds = [], prepare: false, async: false) # :nodoc:
def exec_query(sql, name = nil, binds = [], prepare: false, async: false) # :nodoc:
sql = transform_query(sql)
check_if_write_query(sql)

Expand Down Expand Up @@ -57,7 +57,7 @@ def internal_exec_query(sql, name = nil, binds = [], prepare: false, async: fals
end

def exec_delete(sql, name = "SQL", binds = []) # :nodoc:
internal_exec_query(sql, name, binds)
exec_query(sql, name, binds)
@raw_connection.changes
end
alias :exec_update :exec_delete
Expand Down
Expand Up @@ -6,7 +6,7 @@ module SQLite3
module SchemaStatements # :nodoc:
# Returns an array of indexes for the given table.
def indexes(table_name)
internal_exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").filter_map do |row|
exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").filter_map do |row|
# Indexes SQLite creates implicitly for internal use start with "sqlite_".
# See https://www.sqlite.org/fileformat2.html#intschema
next if row["name"].start_with?("sqlite_")
Expand All @@ -23,7 +23,7 @@ def indexes(table_name)

/\bON\b\s*"?(\w+?)"?\s*\((?<expressions>.+?)\)(?:\s*WHERE\b\s*(?<where>.+))?(?:\s*\/\*.*\*\/)?\z/i =~ index_sql

columns = internal_exec_query("PRAGMA index_info(#{quote(row['name'])})", "SCHEMA").map do |col|
columns = exec_query("PRAGMA index_info(#{quote(row['name'])})", "SCHEMA").map do |col|
col["name"]
end

Expand Down
Expand Up @@ -319,7 +319,7 @@ def change_column_null(table_name, column_name, null, default = nil) # :nodoc:
validate_change_column_null_argument!(null)

unless null || default.nil?
internal_exec_query("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL")
exec_query("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL")
end
alter_table(table_name) do |definition|
definition[column_name].null = null
Expand Down Expand Up @@ -360,7 +360,7 @@ def add_reference(table_name, ref_name, **options) # :nodoc:
alias :add_belongs_to :add_reference

def foreign_keys(table_name)
fk_info = internal_exec_query("PRAGMA foreign_key_list(#{quote(table_name)})", "SCHEMA")
fk_info = exec_query("PRAGMA foreign_key_list(#{quote(table_name)})", "SCHEMA")
fk_info.map do |row|
options = {
column: row["from"],
Expand Down Expand Up @@ -434,7 +434,7 @@ def bind_params_length
end

def table_structure(table_name)
structure = internal_exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA")
structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA")
raise(ActiveRecord::StatementInvalid, "Could not find table '#{table_name}'") if structure.empty?
table_structure_with_collation(table_name, structure)
end
Expand Down Expand Up @@ -598,7 +598,7 @@ def copy_table_contents(from, to, columns, rename = {})
quoted_columns = columns.map { |col| quote_column_name(col) } * ","
quoted_from_columns = from_columns_to_copy.map { |col| quote_column_name(col) } * ","

internal_exec_query("INSERT INTO #{quote_table_name(to)} (#{quoted_columns})
exec_query("INSERT INTO #{quote_table_name(to)} (#{quoted_columns})
SELECT #{quoted_from_columns} FROM #{quote_table_name(from)}")
end

Expand Down
Expand Up @@ -12,7 +12,7 @@ def select_all(*, **) # :nodoc:
result
end

def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
def exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
sql = transform_query(sql)
check_if_write_query(sql)
mark_transaction_written_if_write(sql)
Expand Down

0 comments on commit 338e1f7

Please sign in to comment.