refactor: extract HaversineQuery helper, fix LL bind quirk

The 5 haversine SQL fragments duplicated across StationController and
NationalFuelPredictionService disagreed on float-clamping (LEAST only,
GREATEST/LEAST, vs. CASE WHEN). Centralised in App\Services\HaversineQuery
with the safe GREATEST(-1.0, LEAST(1.0, …)) form everywhere.

withinKm() embeds the radius as a numeric literal (sprintf %F) because
PDO + SQLite binds float parameters as strings by default, which breaks
numeric comparison against the haversine expression — a NULL filter would
silently match all rows. Coordinates remain bound positionally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ovidiu U
2026-04-29 19:33:07 +01:00
parent 48af2083f3
commit 00d0f7c8ec
3 changed files with 58 additions and 10 deletions

View File

@@ -9,6 +9,7 @@ use App\Http\Resources\Api\StationResource;
use App\Models\Search; use App\Models\Search;
use App\Models\Station; use App\Models\Station;
use App\Models\User; use App\Models\User;
use App\Services\HaversineQuery;
use App\Services\NationalFuelPredictionService; use App\Services\NationalFuelPredictionService;
use App\Services\PlanFeatures; use App\Services\PlanFeatures;
use App\Services\PostcodeService; use App\Services\PostcodeService;
@@ -43,14 +44,12 @@ class StationController extends Controller
$radius = $request->radius(); $radius = $request->radius();
$sort = $request->sort(); $sort = $request->sort();
[$distanceSql, $distanceBindings] = HaversineQuery::distanceKm($lat, $lng);
$all = Station::query() $all = Station::query()
->selectRaw( ->selectRaw(
'stations.*, spc.price_pence, spc.fuel_type, spc.price_effective_at, "stations.*, spc.price_pence, spc.fuel_type, spc.price_effective_at, {$distanceSql} AS distance_km",
(6371 * acos(GREATEST(-1.0, LEAST(1.0, $distanceBindings,
cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?))
+ sin(radians(?)) * sin(radians(lat))
)))) AS distance_km',
[$lat, $lng, $lat],
) )
->join('station_prices_current as spc', function (JoinClause $join) use ($fuelType): void { ->join('station_prices_current as spc', function (JoinClause $join) use ($fuelType): void {
$join->on('stations.node_id', '=', 'spc.station_id') $join->on('stations.node_id', '=', 'spc.station_id')

View File

@@ -0,0 +1,41 @@
<?php
namespace App\Services;
/**
* Builds canonical haversine SQL fragments for distance and within-radius
* filtering. Centralises the float-clamping (GREATEST/LEAST) and the column
* naming convention used across prediction and station search queries.
*
* Assumes the joined/queried table exposes columns `lat` and `lng`.
*/
final class HaversineQuery
{
private const string DISTANCE_KM_SQL =
'(6371 * acos(GREATEST(-1.0, LEAST(1.0, '
.'cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?)) '
.'+ sin(radians(?)) * sin(radians(lat))))))';
/**
* Bare distance-in-km expression. Caller adds aliasing or comparison.
*
* @return array{0: string, 1: array{float, float, float}}
*/
public static function distanceKm(float $lat, float $lng): array
{
return [self::DISTANCE_KM_SQL, [$lat, $lng, $lat]];
}
/**
* `<= {km}` predicate suitable for whereRaw. The radius is embedded as a
* numeric literal because PDO + SQLite's whereRaw binds floats as strings
* by default, which breaks numeric comparison against the haversine
* expression. The `float` parameter is type-checked and not user input.
*
* @return array{0: string, 1: array{float, float, float}}
*/
public static function withinKm(float $lat, float $lng, float $km): array
{
return [self::DISTANCE_KM_SQL.' <= '.sprintf('%F', $km), [$lat, $lng, $lat]];
}
}

View File

@@ -103,10 +103,12 @@ class NationalFuelPredictionService
private function getCurrentAverage(FuelType $fuelType, ?float $lat, ?float $lng): float private function getCurrentAverage(FuelType $fuelType, ?float $lat, ?float $lng): float
{ {
if ($lat !== null && $lng !== null) { if ($lat !== null && $lng !== null) {
[$radiusSql, $radiusBindings] = HaversineQuery::withinKm($lat, $lng, 50);
$avg = DB::table('station_prices_current') $avg = DB::table('station_prices_current')
->join('stations', 'station_prices_current.station_id', '=', 'stations.node_id') ->join('stations', 'station_prices_current.station_id', '=', 'stations.node_id')
->where('station_prices_current.fuel_type', $fuelType->value) ->where('station_prices_current.fuel_type', $fuelType->value)
->whereRaw('(6371 * acos(LEAST(1.0, cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?)) + sin(radians(?)) * sin(radians(lat))))) <= 50', [$lat, $lng, $lat]) ->whereRaw($radiusSql, $radiusBindings)
->avg('station_prices_current.price_pence'); ->avg('station_prices_current.price_pence');
if ($avg !== null) { if ($avg !== null) {
@@ -392,11 +394,13 @@ class NationalFuelPredictionService
private function computeRegionalMomentumSignal(FuelType $fuelType, float $lat, float $lng): array private function computeRegionalMomentumSignal(FuelType $fuelType, float $lat, float $lng): array
{ {
// Regional momentum: compare trend of stations within 50km vs national trend // Regional momentum: compare trend of stations within 50km vs national trend
[$radiusSql, $radiusBindings] = HaversineQuery::withinKm($lat, $lng, 50);
$rows = DB::table('station_prices') $rows = DB::table('station_prices')
->join('stations', 'station_prices.station_id', '=', 'stations.node_id') ->join('stations', 'station_prices.station_id', '=', 'stations.node_id')
->where('station_prices.fuel_type', $fuelType->value) ->where('station_prices.fuel_type', $fuelType->value)
->where('station_prices.price_effective_at', '>=', now()->subDays(14)) ->where('station_prices.price_effective_at', '>=', now()->subDays(14))
->whereRaw('(6371 * acos(CASE WHEN (cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?)) + sin(radians(?)) * sin(radians(lat))) > 1.0 THEN 1.0 ELSE (cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?)) + sin(radians(?)) * sin(radians(lat))) END)) <= 50', [$lat, $lng, $lat, $lat, $lng, $lat]) ->whereRaw($radiusSql, $radiusBindings)
->selectRaw('DATE(station_prices.price_effective_at) as day, AVG(station_prices.price_pence) as avg_price') ->selectRaw('DATE(station_prices.price_effective_at) as day, AVG(station_prices.price_pence) as avg_price')
->groupBy('day') ->groupBy('day')
->orderBy('day') ->orderBy('day')
@@ -668,11 +672,13 @@ class NationalFuelPredictionService
$dateString = $date->toDateString(); $dateString = $date->toDateString();
if ($lat !== null && $lng !== null) { if ($lat !== null && $lng !== null) {
[$radiusSql, $radiusBindings] = HaversineQuery::withinKm($lat, $lng, 50);
$regional = DB::table('station_prices') $regional = DB::table('station_prices')
->join('stations', 'station_prices.station_id', '=', 'stations.node_id') ->join('stations', 'station_prices.station_id', '=', 'stations.node_id')
->where('station_prices.fuel_type', $fuelType->value) ->where('station_prices.fuel_type', $fuelType->value)
->whereDate('station_prices.price_effective_at', $dateString) ->whereDate('station_prices.price_effective_at', $dateString)
->whereRaw('(6371 * acos(LEAST(1.0, cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?)) + sin(radians(?)) * sin(radians(lat))))) <= 50', [$lat, $lng, $lat]) ->whereRaw($radiusSql, $radiusBindings)
->avg('station_prices.price_pence'); ->avg('station_prices.price_pence');
if ($regional !== null) { if ($regional !== null) {
@@ -697,11 +703,13 @@ class NationalFuelPredictionService
$usedRegional = false; $usedRegional = false;
if ($lat !== null && $lng !== null) { if ($lat !== null && $lng !== null) {
[$radiusSql, $radiusBindings] = HaversineQuery::withinKm($lat, $lng, 50);
$rows = DB::table('station_prices') $rows = DB::table('station_prices')
->join('stations', 'station_prices.station_id', '=', 'stations.node_id') ->join('stations', 'station_prices.station_id', '=', 'stations.node_id')
->where('station_prices.fuel_type', $fuelType->value) ->where('station_prices.fuel_type', $fuelType->value)
->where('station_prices.price_effective_at', '>=', now()->subDays($days)->startOfDay()) ->where('station_prices.price_effective_at', '>=', now()->subDays($days)->startOfDay())
->whereRaw('(6371 * acos(LEAST(1.0, cos(radians(?)) * cos(radians(lat)) * cos(radians(lng) - radians(?)) + sin(radians(?)) * sin(radians(lat))))) <= 50', [$lat, $lng, $lat]) ->whereRaw($radiusSql, $radiusBindings)
->selectRaw('DATE(station_prices.price_effective_at) as day, AVG(station_prices.price_pence) as avg_price') ->selectRaw('DATE(station_prices.price_effective_at) as day, AVG(station_prices.price_pence) as avg_price')
->groupBy('day') ->groupBy('day')
->orderBy('day') ->orderBy('day')