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

.apply_scopes MUST be called before .where #152

Open
enwood opened this issue Apr 22, 2015 · 1 comment
Open

.apply_scopes MUST be called before .where #152

enwood opened this issue Apr 22, 2015 · 1 comment

Comments

@enwood
Copy link

enwood commented Apr 22, 2015

Ignacio Responds:
Nice find! The implementation of apply_scopes is unfortunately old and
fragile. We really need to replace it with something stronger based on
the latest ActiveRecord/Arel syntax.

Tim's Submission:
It seems that it is very important to call .apply_scopes BEFORE calling a .where clause, especially if the scope is effectively empty.

Calling .apply_scopes with an empty scope AFTER a .where clause causes the .where clause to be ignored!

An example:

In an #index function, I watch for search and sorting scopes:

  def index
    scopes = {                                                               
      :search => [params[:search], :user_name, :search_text], 
      :order_by => parse_sort_param(:id, :date_searched, :user_name, :matches, :search_text)
    }
    @searches = Search
         .where('date_searched >= ? AND date_searched <= ?', params[:start_date], params[:end_date] )
         .apply_scopes(scopes).order('date_searched DESC')
         .paginate(:per_page => 200, :page => params[:page])

I've found, though, that if my request does not include a search and an order_by parameter, the call to apply_scopes causes the .where clause to be ignored!

For example, this is the request to filter by date:

GET "/admin/searches?start_date=04%2F01%2F2015&end_date=04%2F30%2F2015"

And this is the SQL that's generated:

[INFO ] Parameters: {"start_date"=>"04/01/2015", "end_date"=>"04/30/2015"} [DEBUG] Search Load (98.4ms) EXEC sp_executesql N'SELECT TOP (200) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY date_searched DESC) AS [__rn], [searches].* FROM [searches] ) AS [__rnt] WHERE [__rnt].[__rn] > (0) ORDER BY [__rnt].[__rn] ASC'

How bizarre!

But, if it does include a search parameter to be used by apply_scopes:

[INFO ] GET "/admin/searches?start_date=04%2F01%2F2015&end_date=04%2F30%2F2015&search=Tim"

I get this SQL:

[DEBUG] Search Load (8.8ms) EXEC sp_executesql N'SELECT TOP (200) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY date_searched DESC) AS [__rn], [searches].* FROM [searches] WHERE (date_searched >= N''04/01/2015'' AND date_searched <= N''04/30/2015'') AND (((searches.user_name LIKE N''%Tim%'') OR (searches.search_text LIKE N''%Tim%''))) ) AS [__rnt] WHERE [__rnt].[__rn] > (0) ORDER BY [__rnt].[__rn] ASC'

SOLUTION:

If I reverse the order of the .where and the .apply_scopes, it works as expected, which is completely unintuitive!

@searches = Search
  .apply_scopes(scopes)
  .order('date_searched DESC')
  .where('date_searched >= ? AND date_searched <= ?', params[:start_date], params[:end_date] )

[DEBUG]   Search Load (6.5ms)  EXEC sp_executesql N'SELECT TOP (200) [__rnt].* FROM ( SELECT ROW_NUMBER() OVER (ORDER BY date_searched DESC) AS [__rn], [searches].* FROM [searches] WHERE (date_searched >= N''04/01/2015'' AND date_searched <= N''04/30/2015'') ) AS [__rnt] WHERE [__rnt].[__rn] > (0) ORDER BY [__rnt].[__rn] ASC'

In thinking about crafting this query, it made more sense to me to reduce the overall set first (with the .where clause), and then apply any search (or sorting) within that.

@iox
Copy link
Member

iox commented Apr 25, 2015

Thanks for the great bug report!

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

No branches or pull requests

2 participants