This project is archived and is in readonly mode.

#4841 ✓committed
Cedric Fung

multiple select only keep the last one

Reported by Cedric Fung | June 12th, 2010 @ 10:18 AM | in 3.0.2

I use rails 3b4

this will return location_title

irb(main):001:0> Event.select('title').select('location_title').limit(1)
=> [#<Event location_title: "Shanghai">]

but this

irb(main):002:0> Event.select('location_title').select('title').limit(1)
=> [#<Event title: "Party ON">]

Comments and changes to this ticket

  • Uģis Ozols

    Uģis Ozols June 14th, 2010 @ 02:13 PM

    Combine those two selects into one: Event.select('title, location_title').limit(1)

  • Cedric Fung

    Cedric Fung June 14th, 2010 @ 02:28 PM

    this just a demo.
    I won't use Event.select('title').select('location_title') directly. I just demonstrate the situation.

  • Rohit Arondekar

    Rohit Arondekar June 17th, 2010 @ 08:21 AM

    • Assigned user set to “Rohit Arondekar”

    Cedric, take a look at the following:

    ruby-1.9.2-head > Post
     => Post(id: integer, title: string, content: text, created_at: datetime, updated_at: datetime) 
    ruby-1.9.2-head > a = Post.select('title')
     => [#<Post title: "Test">, #<Post title: "blah">, #<Post title: "123">] 
    ruby-1.9.2-head > a.select('content')
     => [#<Post content: "Testing threadsafe">, #<Post content: "blah blah">, #<Post content: "amazing">]
    

    And can you explain why you want to use select('title').select('location_title') instead of select('title, location_title')?

  • Cedric Fung

    Cedric Fung June 17th, 2010 @ 08:59 AM

    I have two named scope

    I don't wanna transfer all attributes through the mobile network, so I create a 'simple' scope

    scope :tiny, select('title, location')
    

    if the client parameters include 'latitude' and 'longitude', then I'd like get a distance

    scope :near, lambda { |lat, lon| select("((latitude-#{lat})(latitude-#{lat}) + (longitude-#{lon})(longitude-#{lon})) AS distance").order('distance DESC') }
    

    now, I wanna tiny and near(30, 120)

    irb(main):008:0> e = Event.tiny.near(30,120).first
    => #<Event >
    irb(main):009:0> e.distance
    => 1660.7118672780507
    irb(main):010:0> e.title
    ActiveModel::MissingAttributeError: missing attribute: title
    
  • Rohit Arondekar

    Rohit Arondekar June 17th, 2010 @ 09:07 AM

    Shouldn't this

    select("((latitude-#{lat})(latitude-#{lat}) + (longitude-#{lon})(longitude-#{lon})) AS distance")
    

    be

    select("(title, location, (latitude-#{lat})(latitude-#{lat}) + (longitude-#{lon})(longitude-#{lon})) AS distance")
    

    Because as the example I gave above, select overrides previous select. Can you try the above and see if it works?

    Also are you using some gem/plugin?

  • Cedric Fung

    Cedric Fung June 17th, 2010 @ 09:13 AM

    yes, this work

    select("(title, location, (latitude-#{lat})(latitude-#{lat}) + (longitude-#{lon})(longitude-#{lon})) AS distance")
    

    but if I have many scope, very_tiny, tiny, fast, slow

    then I wanna 'very_tiny and near', 'tiny and near', 'fast and tiny and near'...

    I must duplicate so many times in select, why I can't combine select just like combine where?

  • Cedric Fung

    Cedric Fung June 17th, 2010 @ 10:27 AM

    • no changes were found...
  • Santiago Pastorino

    Santiago Pastorino June 17th, 2010 @ 03:39 PM

    • Milestone cleared.
    • Assigned user changed from “Rohit Arondekar” to “Santiago Pastorino”

    Can someone provide a test case?

  • Neeraj Singh
  • Santiago Pastorino

    Santiago Pastorino June 17th, 2010 @ 06:02 PM

    • State changed from “new” to “open”

    Neeraj what about this ...

    +  def test_multiple_selects
    +    post = Post.scoped.select('comments_count').select('title').first
    +    assert_equal "Welcome to the weblog", post.title
    +    assert_equal 2, post.comments_count
    +  end
    
  • Neeraj Singh

    Neeraj Singh June 17th, 2010 @ 06:53 PM

    of course that will do and it is much concise.

  • Santiago Pastorino

    Santiago Pastorino June 17th, 2010 @ 07:37 PM

    Neeraj do you want to upload the patch? is yours ;)

  • Rohit Arondekar

    Rohit Arondekar June 18th, 2010 @ 01:33 AM

    Ignore me if I'm wrong but isn't this an Arel issue?

  • Neeraj Singh

    Neeraj Singh June 18th, 2010 @ 03:23 AM

    In arel this is what you need to do

    Table(:users).predicate(users[:id],users[:name])
    

    AR needs to be fixed to pass columns in the right way.

    I am working on a fix. The fix so far I have is

    last = selects.last
    if last.is_a?(Arel::Count) || (last.is_a?(String) && last.downcase[0,5] =~ /count/)
      arel = arel.project(selects.last)
    else          
      arel = arel.project(*selects)
    end
    

    Above fix is passing all the tests (old and new) except one.

     Developer.send(:with_scope, :find => { :select => "id, name" }) do
        developer = Developer.find(:first, :select => 'id, salary', :conditions => "name = 'David'")
        assert_equal 80000, developer.salary
        assert !developer.has_attribute?(:name)
      end
    end
    

    I have a feeling that fixing the above test would require a lot of effort.

    Will update on my findings soon.

  • Rohit Arondekar
  • José Valim
  • Neeraj Singh

    Neeraj Singh June 21st, 2010 @ 08:23 PM

    Attached is a patch which fixes the issue and the test provided by Santiago(with order column added for postgresql).

    However the patch is failing following test.

      def test_options_select_replaces_scope_select
        Developer.send(:with_scope, :find => { :select => "id, name" }) do
          developer = Developer.find(:first, :select => 'id, salary', :conditions => "name = 'David'")
          assert_equal 80000, developer.salary
          assert !developer.has_attribute?(:name)
        end
      end
    

    This brings to question that I would like to discuss with larger community. Take a look at following two cases.

    Post.select('title').select('author_name').first
    
    Post.send(:with_scope, :find => {:select => 'title'}) do
      Post.find(:first, :select => 'author_name')
    end
    

    In the first case we would like to have both the selects. In the second case we would like to have only the second select.

    When the execution for second select starts then all there is to act upon is a relation object which is already built using first select. Now there is no way to know if the second select should override the first select or should it be appeneded.

    Current code only picks up the last select and hence the above mentioned test passes. However my patch picks up all the selects and test fails.

    In order to fix this issue ,it seems, relation object needs to be aware of in what context a select was built into relation object.

    In my first example relation object is being built using standard way and in that case second select should add to the first select.

    In the second example relation object is being built using 'with_scope' and in that case second select should override first select. Howver in order to do that relation object needs to be aware that select was built using 'with_scope'. And given the level of indirection that is no easy task.

    Would like to know thoughts of others on this issue. I just started looking whole AR, AM and Arel . So others are likely to have more insight into this issue.

  • José Valim

    José Valim June 21st, 2010 @ 08:41 PM

    Neeraj,

    I discussed this issue with JK and we agreed on the following methods:

    • select - should always concatenate
    • order - should always concatenate
    • reorder - overwrite order

    We decided not to provide something as "reselect" yet, because we would like to wait a real use case coming from it. That said, it's ok to break the case in your example above. And we can provide a solution in the future if required.

    Just one note, Pastorino is also working on a solution so I would love if you guys could sync. I'm also available if you guys need help.

  • Santiago Pastorino

    Santiago Pastorino June 21st, 2010 @ 08:44 PM

    Neeraj i was digging on this bug and the issue is on ARel.
    I will fix this thing on ARel and then we need to apply only this patch on Rails.

  • Santiago Pastorino

    Santiago Pastorino June 21st, 2010 @ 09:10 PM

    I've done the new reorder method and i'm working on the order and select concatenation that's an ARel issue so perhaps Neeraj we can chat and see if you have a nice advance on the issue we can merge and if not i don't know :P. Perhaps try to solve another one or keep digging on this :D.

  • Neeraj Singh

    Neeraj Singh June 21st, 2010 @ 09:16 PM

    Santiago: Can you briefly outline what kind of change is forthcoming in Arel.

    It seems that your arel patch will support both select cases I outlined in my previous comment.

    When a select is invoked on an existing relation object then how are you going to decide whether to append or to override given that a relation object does not carry the context in which it applied 'select'.

  • Neeraj Singh

    Neeraj Singh June 21st, 2010 @ 09:39 PM

    A new and simpler API of ARel will be really nice. In this way people not using rails will be able to take advantage of good stuff ARel has to offer.

    However I have a different question.

    José Valim suggested that we can drop the offending test. You are suggesting that with your Arel fix that test does not need to be dropped.

    # case 1
    Post.select('title').select('author_name').first
    
    # case 2
    Post.send(:with_scope, :find => {:select => 'title'}) do
      Post.find(:first, :select => 'author_name')
    end
    

    In case 1 we need concatenation. In case 2 the second select should override.

    What I would like to know is how would you find out that in case2 the relation object is built within 'with_scope' given that relation object does not carry the context in which it was create.

    Yes we should do further talking offline :-)

  • Santiago Pastorino

    Santiago Pastorino June 21st, 2010 @ 10:34 PM

    Neeraj in both cases we need to concatenate

  • Repository

    Repository June 25th, 2010 @ 08:07 PM

    • State changed from “open” to “committed”
  • Santiago Pastorino

    Santiago Pastorino June 25th, 2010 @ 08:18 PM

    I've created another ticket to push the order related things here https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets...
    I'm about to finish it.

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2

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>

Attachments

Referenced by

Pages