Merge pull request #135 from flackou/arel

Use Arel instead of raw SQL

lib/geokit-rails/acts_as_mappable.rb

@@ -153,9 +153,9 @@ def by_distance(options = {})
         units   = extract_units_from_options(options)
         formula = extract_formula_from_options(options)
         distance_column_name = distance_sql(origin, units, formula)
-        with_latlng.order(Arel.sql(
-          "#{distance_column_name} #{options[:reverse] ? 'DESC' : 'ASC'}"
-        ))
+        with_latlng.order(
+          Arel.sql(distance_column_name).send(options[:reverse] ? 'desc' : 'asc')
+        )
       end
 
       def with_latlng
@@ -233,7 +233,7 @@ def distance_sql(origin, units=default_units, formula=default_formula)
       # If it's a :within query, add a bounding box to improve performance.
       # This only gets called if a :bounds argument is not otherwise supplied.
       def formulate_bounds_from_distance(options, origin, units)
-        distance = options[:within] if options.has_key?(:within)
+        distance = options[:within] if options.has_key?(:within) && options[:within].is_a?(Numeric)
         distance = options[:range].last-(options[:range].exclude_end?? 1 : 0) if options.has_key?(:range)
         if distance
           Geokit::Bounds.from_point_and_radius(origin,distance,:units=>units)
@@ -248,22 +248,32 @@ def distance_conditions(options)
         formula = extract_formula_from_options(options)
         distance_column_name = distance_sql(origin, units, formula)
 
-        res = if options.has_key?(:within)
-          "#{distance_column_name} <= #{options[:within]}"
+        if options.has_key?(:within)
+          Arel.sql(distance_column_name).lteq(options[:within])
         elsif options.has_key?(:beyond)
-          "#{distance_column_name} > #{options[:beyond]}"
+          Arel.sql(distance_column_name).gt(options[:beyond])
         elsif options.has_key?(:range)
-          "#{distance_column_name} >= #{options[:range].first} AND #{distance_column_name} <#{'=' unless options[:range].exclude_end?} #{options[:range].last}"
+          min_condition = Arel.sql(distance_column_name).gteq(options[:range].begin)
+          max_condition = if options[:range].exclude_end?
+                            Arel.sql(distance_column_name).lt(options[:range].end)
+                          else
+                            Arel.sql(distance_column_name).lteq(options[:range].end)
+                          end
+          min_condition.and(max_condition)
         end
-        Arel::Nodes::SqlLiteral.new("(#{res})") if res.present?
       end
 
       def bound_conditions(bounds)
+        return nil unless bounds
         sw,ne = bounds.sw, bounds.ne
-        lng_sql = bounds.crosses_meridian? ? "(#{qualified_lng_column_name}<#{ne.lng} OR #{qualified_lng_column_name}>#{sw.lng})" : "#{qualified_lng_column_name}>#{sw.lng} AND #{qualified_lng_column_name}<#{ne.lng}"
-        res = "#{qualified_lat_column_name}>#{sw.lat} AND #{qualified_lat_column_name}<#{ne.lat} AND #{lng_sql}"
-        #Arel::Nodes::SqlLiteral.new("(#{res})") if res.present?
-        res if res.present?
+        lat, lng = Arel.sql(qualified_lat_column_name), Arel.sql(qualified_lng_column_name)
+        lat.gt(sw.lat).and(lat.lt(ne.lat)).and(
+          if bounds.crosses_meridian?
+            lng.lt(ne.lng).or(lng.gt(sw.lng))
+          else
+            lng.gt(sw.lng).and(lng.lt(ne.lng))
+          end
+        )
       end
 
       # Extracts the origin instance out of the options if it exists and returns

test/acts_as_mappable_test.rb

@@ -108,6 +108,12 @@ def test_find_within_with_coordinates
     assert_equal 5, locations.count
   end
 
+  def test_find_within_with_column_as_distance
+    locations = Location.joins(:company).within(Company.arel_table[:max_distance], origin: @loc_a)
+    assert_equal 4, locations.to_a.size
+    assert_equal 4, locations.count
+  end
+
   def test_find_with_compound_condition
     #locations = Location.geo_scope(:origin => @loc_a).where("distance < 5 and city = 'Coppell'")
     locations = Location.within(5, :origin => @loc_a).where("city = 'Coppell'")

test/fixtures/companies.yml

@@ -1,7 +1,9 @@
 starbucks:
   id: 1
   name: Starbucks
+  max_distance: 1.7
 
 barnes_and_noble:
   id: 2
-  name: Barnes & Noble
+  name: Barnes & Noble
+  max_distance: 2.0

test/schema.rb

@@ -1,6 +1,7 @@
 ActiveRecord::Schema.define(:version => 0) do
   create_table :companies, :force => true do |t|
     t.column :name, :string
+    t.column :max_distance, :float
   end  
 
   create_table :locations, :force => true do |t|