Allow symbol to mean radius comes from a db column

[chak]

Hi Alex, please take a look at this change. There's may be a cleaner way to do it, but this was quick-and-dirty and does what I needed.

The use-case this new feature supports is for the case when each record has its own radius (e.g., a delivery radius for a store), and you as a customer want to see if you qualify. If the radius you pass in to .near(...) is a symbol, e.g., :delivery_radius_miles, then that text gets passed through to the query as a column name rather than getting cast in ruby to a float and used in app-level calculations. There is a side-effect in how I implemented it; even if you pass a numeric value, the calculation is still done in the database layer.

[alexreisner]

In case you didn't see it, there is some discussion about this at #334.

I think I can get behind what you've done in the ActiveRecord module, but what is the goal of the changes in the Calculations module?

Also, is there a reason for using radius.kind_of?(Symbol) instead of radius.is_a?(Symbol)?

[chak]

The simple answer first: "is_a?(Symbol)" is fine and better; thanks for
pointing that out. I have a habit of typing kind_of? in my code to account
for subclasses, but that's not an issue here.

The reason for the changes in the calculations model is that .near calls
near_scope_options, which calls bounding_box, passing radius all the way
through. The original code in bounding_box assumed that radius was a float,
called to_f on it, and did calculations at the app level. Here we want to
pass radius through to the database to generate that bounding box, because
it's going to be different for reach record. This is safe because either
the input was a symbol (which a hacker could not pass in from the
front-end), in which case we use it as a string, or otherwise it first has
to_f called on it, which ensures it is a float; in neither of these
situations can a hacker insert sql.

Dan

On Tue, Apr 23, 2013 at 7:43 PM, Alex Reisner notifications@github.comwrote:

In case you didn't see it, there is some discussion about this at #334#334
.

I think I can get behind what you've done in the ActiveRecord module, but
what is the goal of the changes in the Calculations module?

Also, is there a reason for using radius.kind_of?(Symbol) instead of
radius.is_a?(Symbol)?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/449#issuecomment-16905253
.

[alexreisner]

The problem is that the bounding_box method is public so we can't just change its behavior. (Note that several tests are now failing, which means people's apps may break.) Even if we could, the bounding box method should return numeric coordinates; it should not be made to handle DB column names. What we want, really, is a second method that returns a bounding box suitable for use in an SQL query. However, that's not going to work either.

The problem is this: within near_scope_options, the bounding box is used for optimization. Basically, finding nearby objects is a two-step process:

1. find everything within a bounding box of the given radius (very fast but includes some objects which are outside the desired radius)
2. find, within those, the objects within a circle of the given radius.

I don't see any way to do this first step if the radius of each target object is variable, short of allowing the input of a maximum radius for each query. We could simply eliminate the bounding box step but performance would not be good. Maybe that's OK for now since this is a new feature, but some more thinking about this would probably be a good idea.

[alexreisner]

Closing this. Please re-open if you can present a solution that solves the above issues.