Change the problem_find_nearby function.

rename old occurences to help when changing code later.

6 files changed, 75 insertions, 15 deletions

diff --git a/db/alert_types.sql b/db/alert_types.sql
index 82a8c71f8..45c81fdab 100644
--- a/db/alert_types.sql
+++ b/db/alert_types.sql
@@ -39,7 +39,7 @@ insert into alert_type
     item_title, item_link, item_description, template)
 values ('local_problems', '', '',
     'New local problems on FixMyStreet', '/', 'The latest local problems reported by users',
-    'problem_find_nearby(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (''confirmed'', ''fixed'')', 'created desc',
+    'problem_find_nearby_easting_northing(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (''confirmed'', ''fixed'')', 'created desc',
     '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-nearby');
 
 -- New problems around a location
@@ -50,7 +50,7 @@ insert into alert_type
     item_title, item_link, item_description, template)
 values ('local_problems_state', '', '',
     'New local problems on FixMyStreet', '/', 'The latest local problems reported by users',
-    'problem_find_nearby(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (?)', 'created desc',
+    'problem_find_nearby_easting_northing(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (?)', 'created desc',
     '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-nearby');
 
 -- New problems sent to a particular council
diff --git a/db/alert_types_eha.sql b/db/alert_types_eha.sql
index c949ea2f8..4f16dc5ed 100644
--- a/db/alert_types_eha.sql
+++ b/db/alert_types_eha.sql
@@ -39,7 +39,7 @@ insert into alert_type
     item_title, item_link, item_description, template)
 values ('local_problems', '', '',
     'New local reports on reportemptyhomes.com', '/', 'The latest local reports reported by users',
-    'problem_find_nearby(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (''confirmed'', ''fixed'')', 'created desc',
+    'problem_find_nearby_easting_northing(?, ?, ?) as nearby,problem', 'nearby.problem_id = problem.id and problem.state in (''confirmed'', ''fixed'')', 'created desc',
     '{{title}}, {{confirmed}}', '/report/{{id}}', '{{detail}}', 'alert-problem-nearby');
 
 -- New problems sent to a particular council
diff --git a/db/migrate_from_osgb36_to_wgs84.pl b/db/migrate_from_osgb36_to_wgs84.pl
index 7ea8fd419..9ac3cf72e 100644
--- a/db/migrate_from_osgb36_to_wgs84.pl
+++ b/db/migrate_from_osgb36_to_wgs84.pl
@@ -30,6 +30,7 @@ BEGIN {
 }
 
 migrate_problem_table();
+migrate_problem_find_nearby_function();
 
 =head2 problem table
 
@@ -101,6 +102,49 @@ sub migrate_problem_table {
 Convert to use lat and long.
 Also swap parameter order so that it is lat,lon rather than lon,lat to be consistent with pledgebank etc
 
+=cut
+
+sub migrate_problem_find_nearby_function {
+    my $dbh = dbh();
+
+    print "drop the existing problem_find_nearby function\n";
+    $dbh->do(
+"DROP FUNCTION problem_find_nearby ( double precision, double precision, double precision)"
+    );
+
+    print "create the new one\n";
+    $dbh->do(<<'SQL_END');
+    create function problem_find_nearby(double precision, double precision, double precision)
+        returns setof problem_nearby_match as
+    '
+        -- trunc due to inaccuracies in floating point arithmetic
+        select problem.id,
+               R_e() * acos(trunc(
+                    (sin(radians($1)) * sin(radians(latitude))
+                    + cos(radians($1)) * cos(radians(latitude))
+                        * cos(radians($2 - longitude)))::numeric, 14)
+                ) as distance
+            from problem
+            where
+                longitude is not null and latitude is not null
+                and radians(latitude) > radians($1) - ($3 / R_e())
+                and radians(latitude) < radians($1) + ($3 / R_e())
+                and (abs(radians($1)) + ($3 / R_e()) > pi() / 2     -- case where search pt is near pole
+                        or angle_between(radians(longitude), radians($2))
+                                < $3 / (R_e() * cos(radians($1 + $3 / R_e()))))
+                -- ugly -- unable to use attribute name "distance" here, sadly
+                and R_e() * acos(trunc(
+                    (sin(radians($1)) * sin(radians(latitude))
+                    + cos(radians($1)) * cos(radians(latitude))
+                        * cos(radians($2 - longitude)))::numeric, 14)
+                    ) < $3
+            order by distance desc
+    ' language sql
+SQL_END
+
+    $dbh->commit;
+}
+
 =head2 alert table
 
 NOTE: only for alert_types 'local_problems' or 'local_problems_state'
diff --git a/db/schema.sql b/db/schema.sql
index 59aacce6e..ba5ce4121 100644
--- a/db/schema.sql
+++ b/db/schema.sql
@@ -196,25 +196,41 @@ create type problem_nearby_match as (
     distance double precision   -- km
 );
 
--- problem_find_nearby EASTING NORTHING DISTANCE
--- Find problems within DISTANCE (km) of (EASTING, NORTHING).
+-- problem_find_nearby LATITUDE LONGITUDE DISTANCE
+-- Find locations within DISTANCE (km) of (LATITUDE, LONGITUDE).
 create function problem_find_nearby(double precision, double precision, double precision)
     returns setof problem_nearby_match as
     -- Write as SQL function so that we don't have to construct a temporary
     -- table or results set in memory. That means we can't check the values of
     -- the parameters, sadly.
+    -- Through sheer laziness, just use great-circle distance; that'll be off
+    -- by ~0.1%:
+    --  http://www.ga.gov.au/nmd/geodesy/datums/distance.jsp
+    -- We index locations on lat/lon so that we can select the locations which lie
+    -- within a wedge of side about 2 * DISTANCE. That cuts down substantially
+    -- on the amount of work we have to do.
 '
     -- trunc due to inaccuracies in floating point arithmetic
     select problem.id,
-           sqrt(($1 - easting) ^ 2
-                + ($2 - northing) ^ 2)
-            as distance
+           R_e() * acos(trunc(
+                (sin(radians($1)) * sin(radians(latitude))
+                + cos(radians($1)) * cos(radians(latitude))
+                    * cos(radians($2 - longitude)))::numeric, 14)
+            ) as distance
         from problem
         where
+            longitude is not null and latitude is not null
+            and radians(latitude) > radians($1) - ($3 / R_e())
+            and radians(latitude) < radians($1) + ($3 / R_e())
+            and (abs(radians($1)) + ($3 / R_e()) > pi() / 2     -- case where search pt is near pole
+                    or angle_between(radians(longitude), radians($2))
+                            < $3 / (R_e() * cos(radians($1 + $3 / R_e()))))
             -- ugly -- unable to use attribute name "distance" here, sadly
-            sqrt(($1 - easting) ^ 2
-                + ($2 - northing) ^ 2)
-                < $3 * 1000
+            and R_e() * acos(trunc(
+                (sin(radians($1)) * sin(radians(latitude))
+                + cos(radians($1)) * cos(radians(latitude))
+                    * cos(radians($2 - longitude)))::numeric, 14)
+                ) < $3
         order by distance desc
 ' language sql; -- should be "stable" rather than volatile per default?
 
diff --git a/perllib/FixMyStreet/Alert.pm b/perllib/FixMyStreet/Alert.pm
index 9996f03c8..e8532a7d4 100644
--- a/perllib/FixMyStreet/Alert.pm
+++ b/perllib/FixMyStreet/Alert.pm
@@ -186,7 +186,7 @@ sub email_alerts ($) {
         $d = int($d*10+0.5)/10;
         my $testing_email_clause = "and problem.email <> '$testing_email'" if $testing_email;        
         my %data = ( template => $template, data => '', alert_id => $alert->{id}, alert_email => $alert->{email}, lang => $alert->{lang}, cobrand => $alert->{cobrand}, cobrand_data => $alert->{cobrand_data} );
-        my $q = "select * from problem_find_nearby(?, ?, ?) as nearby, problem
+        my $q = "select * from problem_find_nearby_easting_northing(?, ?, ?) as nearby, problem
             where nearby.problem_id = problem.id and problem.state in ('confirmed', 'fixed')
             and problem.confirmed >= ? and problem.confirmed >= ms_current_timestamp() - '7 days'::interval
             and (select whenqueued from alert_sent where alert_sent.alert_id = ? and alert_sent.parameter::integer = problem.id) is null
diff --git a/perllib/Problems.pm b/perllib/Problems.pm
index 1556b7724..263d58bc9 100644
--- a/perllib/Problems.pm
+++ b/perllib/Problems.pm
@@ -94,7 +94,7 @@ sub recent_photos {
         $probs = Memcached::get($key);
         unless ($probs) {
             $probs = select_all("select id, title
-                from problem_find_nearby(?, ?, ?) as nearby, problem
+                from problem_find_nearby_easting_northing(?, ?, ?) as nearby, problem
                 where nearby.problem_id = problem.id
                 and state in ('confirmed', 'fixed') and photo is not null
                 $site_restriction
@@ -193,7 +193,7 @@ sub nearby {
     mySociety::Locale::in_gb_locale { select_all(
         "select id, title, easting, northing, distance, state,
             extract(epoch from confirmed) as time
-        from problem_find_nearby(?, ?, $dist) as nearby, problem
+        from problem_find_nearby_easting_northing(?, ?, $dist) as nearby, problem
         where nearby.problem_id = problem.id " .
         ($interval ? " and ms_current_timestamp()-lastupdate < '$interval'::interval" : '') .
         " and state in ('confirmed', 'fixed')" . ($ids ? ' and id not in (' . $ids . ')' : '') . "
@@ -206,7 +206,7 @@ sub fixed_nearby {
     my ($dist, $mid_e, $mid_n) = @_;
     mySociety::Locale::in_gb_locale { select_all(
         "select id, title, easting, northing, distance
-        from problem_find_nearby(?, ?, $dist) as nearby, problem
+        from problem_find_nearby_easting_northing(?, ?, $dist) as nearby, problem
         where nearby.problem_id = problem.id and state='fixed'
         $site_restriction
         order by lastupdate desc", $mid_e, $mid_n);