Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly handle parameterized find_by_sql selects #26

Closed
wants to merge 1 commit into from
Closed

Correctly handle parameterized find_by_sql selects #26

wants to merge 1 commit into from

Conversation

joshtabak
Copy link
Contributor

Parameterzied find_by_sql selects were erroring e.g:
User.find_by_sql(["select * from users where login = ?",'josh'])

Parameterzied find_by_sql selects were erroring e.g:
User.find_by_sql(["select * from users where login = ?",'josh'])
@robinroestenburg
Copy link
Contributor

This (and the original) implementation is incorrect.

The call Apple.find_by_sql(["select * from apples where id = ?", 2]) will enter find_by_sql_with_record_cache with:

  • args: [["select * from apples where id = ?", 2]]
    Because args[0] is no String, it will not consider this query cacheable - which is incorrect.

Take a look at #11, which changes the implementation of find_by_sql_with_record_cache method to match the Rails signature and fixes (if I am correct) this problem in the proces.

@orslumen
Copy link
Owner

Fixed.

Note that custom find_by_sql queries are never cacheable, as record-cache is based on AREL and will not try to parse such custom SQL strings. Most of the time when you need custom SQL the query will be too complex anyways for the cache.

@orslumen orslumen closed this Jun 11, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants