Skip to content

Commit

Permalink
Revert "Merge pull request #48188 from eileencodes/revert-48069"
Browse files Browse the repository at this point in the history
This reverts commit 6adaeb8, reversing
changes made to a792a62.

We're going to forward fix this in our application rather than keep this
revert in. Reverting other changes has turned out to be too difficult to
get back to a state where our application is retrying queries.
  • Loading branch information
eileencodes committed May 15, 2023
1 parent 74e870f commit 942785f
Show file tree
Hide file tree
Showing 16 changed files with 79 additions and 43 deletions.
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
Expand Up @@ -74,6 +74,13 @@

*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:
exec_query(sql, name).rows
internal_exec_query(sql, name).rows
end

# Determines whether the SQL statement is a write query.
Expand All @@ -120,8 +120,12 @@ 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 the exec_query
# method may be manually memory managed. Consider using #exec_query
# wrapper instead.
def execute(sql, name = nil, allow_retry: false)
internal_execute(sql, name, allow_retry: allow_retry)
Expand All @@ -130,34 +134,38 @@ 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)
raise NotImplementedError
internal_exec_query(sql, name, binds, prepare: prepare)
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)
exec_query(sql, name, binds)
internal_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 = [])
exec_query(sql, name, binds)
internal_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 = [])
exec_query(sql, name, binds)
internal_exec_query(sql, name, binds)
end

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

def explain(arel, binds = [], options = []) # :nodoc:
Expand Down Expand Up @@ -490,6 +498,10 @@ 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 @@ -606,7 +618,7 @@ def select(sql, name = nil, binds = [], prepare: false, async: false)
return future_result
end

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

class << self
def included(base) # :nodoc:
dirties_query_cache base, :execute, :create, :insert, :update, :delete, :truncate, :truncate_tables,
:rollback_to_savepoint, :rollback_db_transaction, :restart_db_transaction, :exec_insert_all
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

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

def select_all(arel, name = nil, binds = [], preparable: nil, async: false)
def select_all(arel, name = nil, binds = [], preparable: nil, async: false) # :nodoc:
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 = exec_query(<<~SQL, "SCHEMA")
fk_info = internal_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 = exec_query(sql, "SCHEMA")
chk_info = internal_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 = exec_query("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE #{quote(column_name)}", "SCHEMA").first["Type"]
current_type = internal_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:
exec_query("SHOW CREATE TABLE #{quote_table_name(table_name)}", "SCHEMA").first["Create Table"]
internal_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 = exec_query(sql, "EXPLAIN", binds)
result = internal_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.exec_query("SHOW TABLE STATUS LIKE #{@connection.quote(table_name)}", "SCHEMA").first["Collation"]
@connection.internal_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 exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
def internal_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 = exec_query(sql, "EXPLAIN", binds)
result = internal_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 exec_query(sql, name = "SQL", binds = [], prepare: false, async: false, allow_retry: false, materialize_transactions: true) # :nodoc:
def internal_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 = exec_query(sql, name, binds)
result = internal_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)
exec_query("SELECT currval(#{quote(sequence_name)})", "SQL")
internal_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 = exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
fk_info = internal_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 = exec_query(<<-SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
check_info = internal_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 = exec_query(<<-SQL, "SCHEMA")
exclusion_info = internal_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 = exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false)
unique_info = internal_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

exec_query(sql).tap { reload_type_map }
internal_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)
exec_query("DROP EXTENSION IF EXISTS \"#{name}\"#{' CASCADE' if force == :cascade}").tap {
internal_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
exec_query("SELECT extname FROM pg_extension", "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values
internal_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

exec_query(query, "SCHEMA", allow_retry: true, materialize_transactions: false).cast_values.each_with_object({}) do |row, memo|
internal_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
exec_query(query)
internal_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
exec_query(query)
internal_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 = exec_query(sql, "EXPLAIN", [])
result = internal_exec_query(sql, "EXPLAIN", [])
SQLite3::ExplainPrettyPrinter.new.pp(result)
end

def exec_query(sql, name = nil, binds = [], prepare: false, async: false) # :nodoc:
def internal_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 exec_query(sql, name = nil, binds = [], prepare: false, async: false) # :nod
end

def exec_delete(sql, name = "SQL", binds = []) # :nodoc:
exec_query(sql, name, binds)
internal_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)
exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").filter_map do |row|
internal_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 = exec_query("PRAGMA index_info(#{quote(row['name'])})", "SCHEMA").map do |col|
columns = internal_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?
exec_query("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL")
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")
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 = exec_query("PRAGMA foreign_key_list(#{quote(table_name)})", "SCHEMA")
fk_info = internal_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 = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA")
structure = internal_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) } * ","

exec_query("INSERT INTO #{quote_table_name(to)} (#{quoted_columns})
internal_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 exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) # :nodoc:
def internal_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 942785f

Please sign in to comment.