This project is archived and is in readonly mode.
Change :select precedence in construct_finder_sql to favor options[:select]
Reported by Andrew White | May 22nd, 2008 @ 03:32 PM
Currently construct_finder_sql gives precedence to the :select from the current scope over :select passed in the options parameter. Not only is this counterintuitive it makes it harder to optimize performance by reducing the columns returned or to construct sql within the context of one model and then feed it into find_by_sql in another model.
The attached patch simply switches the order in construct_finder_sql and adds a couple of tests to check that construct_finder_sql respects the :select option
Comments and changes to this ticket
-
Pratik May 22nd, 2008 @ 08:06 PM
- Title changed from [Patch] Change :select precedence in construct_finder_sql to favor options[:select] to Change :select precedence in construct_finder_sql to favor options[:select]
-
Pratik May 29th, 2008 @ 11:44 AM
- State changed from new to incomplete
- Assigned user set to Pratik
Looks good.
Could you please change tests to check both :
1) Query fired ( as you've already done )
2) Assert result of a find. For example Developer.find(:all, :select => 'developers.not_in_scope')
Thanks.
-
Andrew White May 29th, 2008 @ 01:06 PM
Okay I've modified the patch to check both the sql generated and whether attributes are/are not present in the attributes hash after doing a find.
-
Pratik May 29th, 2008 @ 01:35 PM
- State changed from incomplete to open
-
Repository May 29th, 2008 @ 02:26 PM
- State changed from open to resolved
(from [235d635708dd72bee0828457af5397c79750483a]) Ensure :select passed in options overrides the one from the scope. [#239 state:resolved]
Signed-off-by: Pratik Naik
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>