Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• You flagged this item as spam.
  

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

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

• You flagged this item as spam.
  

  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 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')?

• You flagged this item as spam.
  

  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 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?

• You flagged this item as spam.
  

  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?

• You flagged this item as spam.
  

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

  • no changes were found...
• 

  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 June 17th, 2010 @ 04:56 PM

  Attached is failing test.

  • 4841_test_failure.diff 964 Bytes
• 

  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 June 17th, 2010 @ 06:53 PM

  of course that will do and it is much concise.

• 

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

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

• 

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

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

• 

  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 June 18th, 2010 @ 03:29 AM

  Nice work Neeraj :)

• 

  José Valim June 18th, 2010 @ 09:30 PM

  • Milestone cleared.
• 

  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.

  • wip.diff 1.8 KB
• 

  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 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.

  • multiple_selects_test.patch 939 Bytes
• 

  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 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 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 June 21st, 2010 @ 10:34 PM

  Neeraj in both cases we need to concatenate

• 

  Repository June 25th, 2010 @ 08:07 PM

  • State changed from “open” to “committed”

  (from [0ebb5bf6590b8ac62c53538ade7095676baec3d4]) Support for multiple selects added

  [#4841 state:committed] http://github.com/rails/rails/commit/0ebb5bf6590b8ac62c53538ade7095...

• 

  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 October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”

  [bulk edit]