From 5353fea4c963d4db2f429f5c343a202f28109266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leonel=20Gal=C3=A1n?= <leonel@getstealz.com> Date: Fri, 18 Aug 2017 14:51:47 -0400 Subject: [PATCH] Allows radius to be a symbol representing a column in DB (#1107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allows radius to be a symbol representing a column in DB * Proposed changes by @alexreisner: - Keeping branching condition consistent - Not overwrite the method argument: ‘radius’ - Not assigning two variables in a single line, and - Add code comments where needed By “proposed†I meant, I copy/paste them from his comment. But we are in agreement. * Fixes broken test in PR --- lib/geocoder/stores/active_record.rb | 17 +++++++++++++++-- test/db/migrate/001_create_test_schema.rb | 1 + test/test_helper.rb | 4 ++++ test/unit/near_test.rb | 7 +++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/geocoder/stores/active_record.rb b/lib/geocoder/stores/active_record.rb index 7c99e055..0f2187a0 100644 --- a/lib/geocoder/stores/active_record.rb +++ b/lib/geocoder/stores/active_record.rb @@ -131,7 +131,11 @@ module Geocoder::Store distance_column = options.fetch(:distance_column) { 'distance' } bearing_column = options.fetch(:bearing_column) { 'bearing' } - b = Geocoder::Calculations.bounding_box([latitude, longitude], radius, options) + # If radius is a DB column name, bounding box should include + # all rows within the maximum radius appearing in that column. + # Note: performance is dependent on variability of radii. + bb_radius = radius.is_a?(Symbol) ? maximum(radius) : radius + b = Geocoder::Calculations.bounding_box([latitude, longitude], bb_radius, options) args = b + [ full_column_name(latitude_attribute), full_column_name(longitude_attribute) @@ -142,7 +146,16 @@ module Geocoder::Store conditions = bounding_box_conditions else min_radius = options.fetch(:min_radius, 0).to_f - conditions = [bounding_box_conditions + " AND (#{distance}) BETWEEN ? AND ?", min_radius, radius] + # if radius is a DB column name, + # find rows between min_radius and value in column + if radius.is_a?(Symbol) + c = "BETWEEN ? AND #{radius}" + a = [min_radius] + else + c = "BETWEEN ? AND ?" + a = [min_radius, radius] + end + conditions = [bounding_box_conditions + " AND (#{distance}) " + c] + a end { :select => select_clause(options[:select], diff --git a/test/db/migrate/001_create_test_schema.rb b/test/db/migrate/001_create_test_schema.rb index 649f99b8..dfbf7b8e 100644 --- a/test/db/migrate/001_create_test_schema.rb +++ b/test/db/migrate/001_create_test_schema.rb @@ -15,6 +15,7 @@ class CreateTestSchema < superclass t.column :address, :string t.column :latitude, :decimal, :precision => 16, :scale => 6 t.column :longitude, :decimal, :precision => 16, :scale => 6 + t.column :radius_column, :decimal, :precision => 16, :scale => 6 end end diff --git a/test/test_helper.rb b/test/test_helper.rb index e9667f5f..b51b77ea 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -75,6 +75,10 @@ else def primary_key :id end + + def maximum(_field) + 1.0 + end end end end diff --git a/test/unit/near_test.rb b/test/unit/near_test.rb index 53401f4b..7ba3506b 100644 --- a/test/unit/near_test.rb +++ b/test/unit/near_test.rb @@ -18,6 +18,13 @@ class NearTest < GeocoderTestCase assert_match(/BETWEEN \? AND \?$/, result[:conditions][0]) end + def test_near_scope_options_includes_radius_column_max_radius + omit("Not applicable to unextended SQLite") if using_unextended_sqlite? + + result = Place.send(:near_scope_options, 1.0, 2.0, :radius_column) + assert_match(/BETWEEN \? AND radius_column$/, result[:conditions][0]) + end + def test_near_scope_options_includes_radius_default_min_radius omit("Not applicable to unextended SQLite") if using_unextended_sqlite? -- GitLab