diff --git a/database/migrations.go b/database/migrations.go index 82241cf..a936283 100644 --- a/database/migrations.go +++ b/database/migrations.go @@ -1646,7 +1646,38 @@ func addRegionToAddressUniqueness() error { "load state boundaries first", blanks) } + // Before attributing anything, repair coordinates that are not WGS84. + // + // Three rows in production could not be placed. Two held values like + // (1416737.65, 811329.71) -- Ohio State Plane South in US survey feet, + // loaded without ever being reprojected. A longitude cannot exceed 180, + // so these are unambiguously not degrees. + // + // The reprojection is only applied where it demonstrably fixes the row: + // the result must land inside the polygon of the county the row already + // claims to be in. That check is what makes this a repair rather than a + // guess -- EPSG:3735 was chosen because it is the only candidate that + // satisfies it, with Ohio North, and both zones in metres, all landing + // in Michigan, Maine or Quebec. res, err := tx.Exec(` + UPDATE ohio_addresses a + SET geom = ST_Transform(ST_SetSRID(ST_MakePoint(ST_X(a.geom), ST_Y(a.geom)), 3735), 4326) + FROM ohio_counties c + WHERE (ABS(ST_X(a.geom)) > 180 OR ABS(ST_Y(a.geom)) > 90) + AND c.bounds_geometry IS NOT NULL + AND c.county_name ILIKE a.county + AND ST_Contains( + c.bounds_geometry, + ST_Transform(ST_SetSRID(ST_MakePoint(ST_X(a.geom), ST_Y(a.geom)), 3735), 4326)) + `) + if err != nil { + return fmt.Errorf("failed to reproject state plane coordinates: %w", err) + } + if n, _ := res.RowsAffected(); n > 0 { + log.Printf("Migration 23: reprojected %d row(s) from Ohio State Plane into WGS84", n) + } + + res, err = tx.Exec(` UPDATE ohio_addresses a SET region = s.state_abbr FROM us_states s @@ -1694,6 +1725,31 @@ func addRegionToAddressUniqueness() error { n, shorelineToleranceMeters) } + // Last resort: the county the row already claims. + // + // A row can have unusable coordinates and still say where it is. The + // remaining production row sits at (-0.0001, 0.0001) -- Null Island, + // the placeholder a missing coordinate becomes -- while naming a real + // Ohio county. ohio_counties holds only Ohio counties, so a name match + // there is evidence of the state even when the point is worthless. + // + // The bad coordinate is not hidden by this: it still shows up under + // outside_us_bounds on /admin/data-quality, which is where a wrong + // location belongs rather than blocking a uniqueness migration. + res, err = tx.Exec(` + UPDATE ohio_addresses a + SET region = 'OH' + FROM ohio_counties c + WHERE (a.region IS NULL OR a.region = '') + AND c.county_name ILIKE a.county + `) + if err != nil { + return fmt.Errorf("failed to attribute blank regions from county names: %w", err) + } + if n, _ := res.RowsAffected(); n > 0 { + log.Printf("Migration 23: attributed %d row(s) from their Ohio county name despite unusable coordinates", n) + } + // Whatever is left is a genuine coordinate error, not a missing state. // The details go in the error rather than only the log, because that // error is surfaced on /health -- so whoever has to decide what to do diff --git a/services/multistate_integration_test.go b/services/multistate_integration_test.go index aa7ea63..5b927a7 100644 --- a/services/multistate_integration_test.go +++ b/services/multistate_integration_test.go @@ -614,3 +614,80 @@ func TestShorelineAddressesAttributeToTheNearestState(t *testing.T) { } } } + +// Two of the three rows production could not place held coordinates like +// (1416737.65, 811329.71) -- Ohio State Plane South in US survey feet, loaded +// without ever being reprojected. A longitude cannot exceed 180, so those are +// unambiguously not degrees. +// +// The repair is only applied where it demonstrably works: the reprojected point +// must land inside the polygon of the county the row already claims. That check +// is what separates a repair from a guess -- Ohio North and both zones in +// metres put these same points in Michigan, Maine and Quebec. +func TestStatePlaneCoordinatesAreRepairedOnlyWhenVerified(t *testing.T) { + db := setupMultiStateDB(t, false) + + if _, err := db.Exec(` + CREATE TABLE ohio_counties ( + id BIGSERIAL PRIMARY KEY, + county_name VARCHAR(255) NOT NULL, + bounds_geometry GEOMETRY(MULTIPOLYGON, 4326) + )`); err != nil { + t.Fatalf("create ohio_counties: %v", err) + } + // A box around Darke County, and one around a county the second row does + // not claim, so a wrong-county match cannot pass. + if _, err := db.Exec(` + INSERT INTO ohio_counties (county_name, bounds_geometry) VALUES + ('Darke', ST_Multi(ST_MakeEnvelope(-84.9,40.0,-84.3,40.4,4326))), + ('Lucas', ST_Multi(ST_MakeEnvelope(-84.0,41.5,-83.4,41.8,4326))) + `); err != nil { + t.Fatalf("seed counties: %v", err) + } + + // Same State Plane coordinates; one claims Darke (correct), one claims + // Lucas (wrong -- reprojection lands in Darke, not Lucas). + for _, c := range []struct{ hash, county string }{ + {"sp-right", "Darke"}, + {"sp-wrong", "Lucas"}, + } { + if _, err := db.Exec(` + INSERT INTO ohio_addresses (hash, house_number, street, unit, city, district, region, postcode, county, geom, full_address) + VALUES ($1,'1','Main Street','','Greenville','','','',$2, + ST_SetSRID(ST_MakePoint(1416737.6505, 811329.7090),4326),'1 Main Street') + `, c.hash, c.county); err != nil { + t.Fatalf("seed %s: %v", c.hash, err) + } + } + + if _, err := db.Exec(` + UPDATE ohio_addresses a + SET geom = ST_Transform(ST_SetSRID(ST_MakePoint(ST_X(a.geom), ST_Y(a.geom)), 3735), 4326) + FROM ohio_counties c + WHERE (ABS(ST_X(a.geom)) > 180 OR ABS(ST_Y(a.geom)) > 90) + AND c.bounds_geometry IS NOT NULL + AND c.county_name ILIKE a.county + AND ST_Contains(c.bounds_geometry, + ST_Transform(ST_SetSRID(ST_MakePoint(ST_X(a.geom), ST_Y(a.geom)), 3735), 4326)) + `); err != nil { + t.Fatalf("reproject: %v", err) + } + + var lng, lat float64 + if err := db.QueryRow(`SELECT ST_X(geom), ST_Y(geom) FROM ohio_addresses WHERE hash = 'sp-right'`).Scan(&lng, &lat); err != nil { + t.Fatalf("read repaired row: %v", err) + } + if lng < -85 || lng > -84 || lat < 40 || lat > 40.5 { + t.Errorf("repaired row is at (%.4f, %.4f), which is not in Darke County", lng, lat) + } + + // The row whose county does not corroborate the reprojection must be left + // alone rather than moved somewhere plausible-looking. + if err := db.QueryRow(`SELECT ST_X(geom), ST_Y(geom) FROM ohio_addresses WHERE hash = 'sp-wrong'`).Scan(&lng, &lat); err != nil { + t.Fatalf("read uncorroborated row: %v", err) + } + if lng < 180 { + t.Errorf("a row whose county does not corroborate the reprojection was rewritten to (%.4f, %.4f); "+ + "the county check is what makes this a repair rather than a guess", lng, lat) + } +}