Skip to content

Commit

Permalink
Ensure symbols passed to select are always quoted
Browse files Browse the repository at this point in the history
Our general contract in Active Record is that strings are assumed to be
SQL literals, and symbols are assumed to reference a column. If a from
clause is given, we shouldn't include the table name, but we should
still quote the value as if it were a column.

Upon fixing this, the tests were still failing on SQLite. This was
because the column name being returned by the query was `"\"join\""`
instead of `"join"`. This is actually a bug in SQLite that was fixed a
long time ago, but I was using the version of SQLite included by OS X
which has this bug. Since I'm guessing this will be a common case for
contributors, I also added an explicit check with a more helpful error
message.

Fixes #20360
  • Loading branch information
sgrif committed May 30, 2015
1 parent 6bd2573 commit 0ef7e73
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Expand Up @@ -308,4 +308,4 @@ DEPENDENCIES
w3c_validators

BUNDLED WITH
1.10.1
1.10.2
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
* Ensure symbols passed to `ActiveRecord::Relation#select` are always treated
as columns.

Fixes #20360.

*Sean Griffin*

* Do not set `sql_mode` if `strict: :default` is specified.

```
Expand Down
16 changes: 7 additions & 9 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -1001,15 +1001,13 @@ def build_select(arel)
end

def arel_columns(columns)
if from_clause.value
columns
else
columns.map do |field|
if (Symbol === field || String === field) && columns_hash.key?(field.to_s)
arel_table[field]
else
field
end
columns.map do |field|
if (Symbol === field || String === field) && columns_hash.key?(field.to_s) && !from_clause.value
arel_table[field]
elsif Symbol === field
connection.quote_table_name(field.to_s)
else
field
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions activerecord/test/cases/relation_test.rb
Expand Up @@ -242,6 +242,13 @@ def test_respond_to_for_non_selected_element
assert_equal false, post.respond_to?(:title), "post should not respond_to?(:body) since invoking it raises exception"
end

def test_select_quotes_when_using_from_clause
ensure_sqlite3_version_doesnt_include_bug
quoted_join = ActiveRecord::Base.connection.quote_table_name("join")
selected = Post.select(:join).from(Post.select("id as #{quoted_join}")).map(&:join)
assert_equal Post.pluck(:id), selected
end

def test_relation_merging_with_merged_joins_as_strings
join_string = "LEFT OUTER JOIN #{Rating.quoted_table_name} ON #{SpecialComment.quoted_table_name}.id = #{Rating.quoted_table_name}.comment_id"
special_comments_with_ratings = SpecialComment.joins join_string
Expand Down Expand Up @@ -276,5 +283,20 @@ def test_update_all_goes_through_normal_type_casting

assert_equal "type cast from database", UpdateAllTestModel.first.body
end

private

def ensure_sqlite3_version_doesnt_include_bug
if current_adapter?(:SQLite3Adapter)
selected_quoted_column_names = ActiveRecord::Base.connection.exec_query(
'SELECT "join" FROM (SELECT id AS "join" FROM posts) subquery'
).columns
assert_equal ["join"], selected_quoted_column_names, <<-ERROR.squish
You are using an outdated version of SQLite3 which has a bug in
quoted column names. Please update SQLite3 and rebuild the sqlite3
ruby gem
ERROR
end
end
end
end

0 comments on commit 0ef7e73

Please sign in to comment.