Skip to content

Commit

Permalink
Restore to_sql to return only SQL (#29945)
Browse files Browse the repository at this point in the history
Because `to_sql` is public API. I introduced `to_sql_and_binds` internal
API to return SQL and binds.
  • Loading branch information
kamipo committed Aug 18, 2017
1 parent e9ba12f commit 893ccb3
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 13 deletions.
Expand Up @@ -10,6 +10,11 @@ def initialize

# Converts an arel AST to SQL
def to_sql(arel_or_sql_string, binds = [])
sql, _ = to_sql_and_binds(arel_or_sql_string, binds)
sql
end

def to_sql_and_binds(arel_or_sql_string, binds = []) # :nodoc:
if arel_or_sql_string.respond_to?(:ast)
unless binds.empty?
raise "Passing bind parameters with an arel AST is forbidden. " \
Expand All @@ -21,6 +26,7 @@ def to_sql(arel_or_sql_string, binds = [])
[arel_or_sql_string.dup.freeze, binds]
end
end
private :to_sql_and_binds

# This is used in the StatementCache object. It returns an object that
# can be used to query the database repeatedly.
Expand All @@ -39,7 +45,7 @@ def cacheable_query(klass, arel) # :nodoc:
# Returns an ActiveRecord::Result instance.
def select_all(arel, name = nil, binds = [], preparable: nil)
arel = arel_from_relation(arel)
sql, binds = to_sql(arel, binds)
sql, binds = to_sql_and_binds(arel, binds)
if !prepared_statements || (arel.is_a?(String) && preparable.nil?)
preparable = false
else
Expand Down Expand Up @@ -139,21 +145,21 @@ def exec_update(sql, name = nil, binds = [])
# If the next id was calculated in advance (as in Oracle), it should be
# passed in as +id_value+.
def insert(arel, name = nil, pk = nil, id_value = nil, sequence_name = nil)
sql, binds = to_sql(arel)
sql, binds = to_sql_and_binds(arel)
value = exec_insert(sql, name, binds, pk, sequence_name)
id_value || last_inserted_id(value)
end
alias create insert

# Executes the update statement and returns the number of rows affected.
def update(arel, name = nil)
sql, binds = to_sql(arel)
sql, binds = to_sql_and_binds(arel)
exec_update(sql, name, binds)
end

# Executes the delete statement and returns the number of rows affected.
def delete(arel, name = nil)
sql, binds = to_sql(arel)
sql, binds = to_sql_and_binds(arel)
exec_delete(sql, name, binds)
end

Expand Down
Expand Up @@ -95,7 +95,7 @@ def clear_query_cache
def select_all(arel, name = nil, binds = [], preparable: nil)
if @query_cache_enabled && !locked?(arel)
arel = arel_from_relation(arel)
sql, binds = to_sql(arel, binds)
sql, binds = to_sql_and_binds(arel, binds)
cache_sql(sql, name, binds) { super(sql, name, binds, preparable: preparable) }
else
super
Expand Down
Expand Up @@ -177,8 +177,7 @@ def clear_cache!
#++

def explain(arel, binds = [])
sql, binds = to_sql(arel, binds)
sql = "EXPLAIN #{sql}"
sql = "EXPLAIN #{to_sql(arel, binds)}"
start = Time.now
result = exec_query(sql, "EXPLAIN", binds)
elapsed = Time.now - start
Expand Down
Expand Up @@ -5,8 +5,7 @@ module ConnectionAdapters
module PostgreSQL
module DatabaseStatements
def explain(arel, binds = [])
sql, binds = to_sql(arel, binds)
sql = "EXPLAIN #{sql}"
sql = "EXPLAIN #{to_sql(arel, binds)}"
PostgreSQL::ExplainPrettyPrinter.new.pp(exec_query(sql, "EXPLAIN", binds))
end

Expand Down
Expand Up @@ -203,8 +203,7 @@ def disable_referential_integrity # :nodoc:
#++

def explain(arel, binds = [])
sql, binds = to_sql(arel, binds)
sql = "EXPLAIN QUERY PLAN #{sql}"
sql = "EXPLAIN QUERY PLAN #{to_sql(arel, binds)}"
SQLite3::ExplainPrettyPrinter.new.pp(exec_query(sql, "EXPLAIN", []))
end

Expand Down
3 changes: 1 addition & 2 deletions activerecord/lib/active_record/relation.rb
Expand Up @@ -571,8 +571,7 @@ def to_sql

conn = klass.connection
conn.unprepared_statement {
sql, _ = conn.to_sql(relation.arel)
sql
conn.to_sql(relation.arel)
}
end
end
Expand Down

0 comments on commit 893ccb3

Please sign in to comment.