Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• 

  Jeremy Kemper May 4th, 2010 @ 06:48 PM

  • Milestone changed from “2.x” to “3.x”

  [bulk edit]

• You flagged this item as spam.
  

  Blane Dabney May 5th, 2010 @ 09:42 PM

  I've attached a patch without tests as an example fix that only covers the basic case of chaining select calls (i.e. Topic.select(:id).select(:name)). There are a few issues surrounding it that may require discussion before implementing. Once I get some feedback I'll submit a full patch with tests.

  The first issue is DISTINCTs. I think these really fall outside of the scope of this method (select) and should probably be handled with a new method or possibly two to properly handle DISTINCT ON. Adding a distinct method would just need an instance variable to capture the set state when its called so that the DISTINCT keyword can be prepended to the first @select_values element when build_arel gets called. DISTINCT ON is more complex, as some (all?) SQL implementations require the leftmost ORDER BY expressions to match the DISTINCT ON expressions.

  The second issue is merge/&. Currently, the instance variables backing all multi-value methods except @join_values and @where_values are overwritten during a merge. I don't see why @select_values, @group_values and @order_values can't simply be appended, and @having_values handled like @where_values.

  The third issue (non-issue) is handling this properly when raw SQL fragments are used. I'd say not handling it is the appropriate action to take here, but I thought I'd toss that out for consideration.

• You flagged this item as spam.
  

  Blane Dabney May 5th, 2010 @ 09:45 PM

  Having issues attaching the patch. It's short, so here it is:

  .../lib/active_record/relation/query_methods.rb | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)

  diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
  index 7bca12d..89edd1e 100644
  --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -175,10 +175,11 @@ module ActiveRecord

     quoted_table_name = @klass.quoted_table_name
  
     if selects.present?
  

  •  selects.each do |s|
    

  •  selects.reject! do |s|
       @implicit_readonly = false
    

  •    arel = arel.project(s) if s.present?
    

  •    !s.present?
     end
    

  •  arel = arel.project(selects)
    

    else

     arel = arel.project(quoted_table_name + '.*')
    

    end

• You flagged this item as spam.
  

  Blane Dabney May 5th, 2010 @ 09:49 PM

   .../lib/active_record/relation/query_methods.rb    |    5 +++--
   1 files changed, 3 insertions(+), 2 deletions(-)
  
  diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
  index 7bca12d..89edd1e 100644
  --- a/activerecord/lib/active_record/relation/query_methods.rb
  +++ b/activerecord/lib/active_record/relation/query_methods.rb
  @@ -175,10 +175,11 @@ module ActiveRecord
         quoted_table_name = @klass.quoted_table_name
   
         if selects.present?
  -        selects.each do |s|
  +        selects.reject! do |s|
             @implicit_readonly = false
  -          arel = arel.project(s) if s.present?
  +          !s.present?
           end
  +        arel = arel.project(selects)
         else
           arel = arel.project(quoted_table_name + '.*')
         end
  

• 

  Rohit Arondekar July 9th, 2010 @ 03:13 AM

  • State changed from “new” to “resolved”
  • Importance changed from “” to “”

  Multiple selects are now merged in Rails master. Ticket 4841 and Commit http://github.com/rails/rails/commit/0ebb5bf6590b8ac62c53538ade7095... for reference.