Proposed solution to the AR includes problem

[gjb83]

I would like to propose the following solution to the AR includes problem (see 'Known Issue' at the end of the README), and to open a discussion with Alex and the geocoder community.

Proposed solution:

1. Remove the :select from the AR query.
2. Instead, add distance and bearing instance methods to the AR model, to be calculated application-side. The formulae would translate easily into pure Ruby code.

This would primarily enable the includes scope to work correctly, but would also improve performance:

• The bearing is currently always calculated, but may not be required.
• The database must currently calculate the distance twice per row: once for the SELECT and again for the ORDER BY.
• When used in conjunction with pagination, many of the calculated distances and bearings are discarded if they don't appear on the current page, only to be recalculated when the page changes.

Comments welcome.

[alexreisner]

You're right that calculating distance twice per row is sub-optimal. However, unless I'm missing something, doesn't this require you to load all geocoded objects from the database to find which are within a given radius? I think instantiating ActiveRecord objects is a much worse performance problem than linearly increasing the number of DB calculations.

Also, doesn't this kind of kill the scope functionality? Let me know if I'm missing something.

[gjb83]

Removing the :select would stop the query from returning the distance and bearing columns, but it would still be possible to filter by radius and sort by distance.

The scope functionality would work as expected:

@items = Item.recent.near('London', 50).includes(:user).paginate

<% @items.each do |item| %>
  <%= item.user.name # Eager loaded %>
  <%= item.description %>
  <%= item.distance # Calls the distance method on Item %>
  <%= item.bearing  # Calls the bearing method on Item %>
<% end %>

[alexreisner]

Interesting. Would item.distance and item.bearing actually work as above or would they need to take an argument? Requiring an argument (basically using distance_to or bearing_to instead) isn't a problem in itself but it would break current implementations so we'd need a plan for a gentle transition.

It's a good idea...let's keep discussing.

[gjb83]

Ideally, it would retain compatibility by requiring no argument and would read the origin location from the near scope. I'm not sure whether it is possible to access the scope from an object (item) or a collection of objects (@items). Do you know?

[gjb83]

I've just had a quick play, and it seems to be possible:

class Item < ActiveRecord::Base
  scope :test, where(:name => 'Test')
end

> items = Item.test
> items.where_values_hash
=> {:name=>"Test"}

or

> items = Item.test
> items.scope_for_create
=> {:name=>"Test"}

I'm not sure of the difference at this stage.

[alexreisner]

Offhand, I can't think of a way to do it. Once the scope is "executed" an array of objects is returned and I don't know of a way to add data to those objects except with a select in the scope. I'm not sure there isn't a way, but I don't know of one right now.

It's not necessarily a showstopper, though. We could introduce deprecation warnings and plan on changing the implementation in version 1.1.

My next task is to move the tests from geocoder_test (the ones that run against a real database) into the main geocoder repository and make it easier to run them against the various back-end stores. When that's done let's work to make sure related functionality is thoroughly tested and then start changing the implementation.

[alexreisner]

Posted that last comment before I saw yours. As you've shown, it's possible to get information about the scope from the scope itself, but how do we give each of the returned objects access to that scope? Seems like we need to inject something into each one...

[jerrett]

+1 on this, from my digging around it seems using select at all is just asking for problems - too many parts of AR modify/replace/depend on the select

[alexreisner]

@jerrett: Do you have any ideas for accessing distance and bearing calculations for each returned object?

[jerrett]

Good ideas? Not really, but you could probably get away with a subquery in a :conditions.. :D

I'll give it some thought and poke around a bit - doing it in the model as suggested doesn't seem terrible if that's possible, but for sure if it's possible to do it in sql (esp. if it could be done once for order and selection) does seem ideal...

[jerrett]

and that won't work either, cause it'd be in the where :|

[phstc]

I have the same problem using with AR.includes

  Restaurant.includes(:dishes).near(lat, lng, 100).where("dishes.name = ?", dish_name)

Geocorder doesn't add the distance column in the query if I add AR.includes.

How can I use near and includes together? I would like to retrieve all restaurants near to a specific lat and lng AND containing a specific dish ORDERED by distance.

[kevinelliott]

@phstc: I think you could get away with what you are trying to do without includes, but with joins (which would actually be a more optimized version of the initial query, since you're trying to filter based on a relationship.