whosonfirst / go-whosonfirst-sqlite-features

Go package for working with Who's On First features and SQLite databases.
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Add alt_label column to geojson table? #7

Closed missinglink closed 3 years ago

missinglink commented 4 years ago

Moving https://github.com/whosonfirst-data/whosonfirst-data/issues/1834 here for discussion.

The current unique index excludes alt geometries in the case where two have the same src:geom but differing src:alt_label.

A workaround is to munge the src:alt_label into the geojson.source column.

This seems wrong to me, having naturalearth-display-terrestrial-zoom6 as the 'source'.

Maybe time to add a new column to geojson as alt_label TEXT and change the index to:

CREATE UNIQUE INDEX geojson_by_id ON %s (id, source, alt_label);

@thisisaaronland thoughts?

sqlite3 test.db 'SELECT id, source, is_alt FROM geojson'
85633041|naturalearth-display-terrestrial-zoom6|1
85633041|naturalearth|1
85633041|quattroshapes|1
85633041|whosonfirst-reversegeo|1
85633041|whosonfirst|0

vs.

sqlite3 test.db 'SELECT id, source, is_alt FROM geojson'
85633041|naturalearth|1
85633041|quattroshapes|1
85633041|whosonfirst|1
missinglink commented 4 years ago

reopening as this repo may wish to port the schema changes.

Turns out an additional column was necessary.

Test case: https://github.com/whosonfirst-data/whosonfirst-data-admin-gb/blob/master/data/101/853/501/101853501-alt-whosonfirst.geojson

thisisaaronland commented 3 years ago

Yeah, the work around the rtree table has demonstrated that it would be better to simply have an explicit alt_label column in all the tables where alt files might be indexed.

Additionally the go-whosonfirst-geojson-v2/properties/whosonfirst package should have its own AltLabel() method. The way things are written now the Source() method gives precedence to the src:alt_label property but that's not obvious:

https://github.com/whosonfirst/go-whosonfirst-geojson-v2/blob/master/properties/whosonfirst/whosonfirst.go#L117-L125

I will add this shortly.

thisisaaronland commented 3 years ago

Fixed as of the v0.4.0 release (https://github.com/whosonfirst/go-whosonfirst-sqlite-features/commit/9c8291d338216d03302c1af5cedb9583d9b33e70)

sqlite> SELECT alt_label, count(id) FROM geojson WHERE is_alt=1 GROUP by alt_label;
can-databc|28
geonames|1
naturalearth|87
naturalearth-display-terrestrial-zoom6|1
ourairports|1
qs_pg|5
quattroshapes|1752
quattroshapes-reversegeo|3
quattroshapes_pg|3303
statcan|2
unknown|550
whosonfirst|1
whosonfirst-reversegeo|1
woedb|13