Skip to content

Commit

Permalink
BUGFIX: Fix name-based way fragmentation in transportation_name (#1295)
Browse files Browse the repository at this point in the history
I discovered this bug while investigating issues with the updates process related to #1190 #1292, and #814.

The `transportation_name` layer produces slightly different `tags` hstore values in the `osm_transportation_name_linestring` table during the initial import versus when running an update.  As currently written, the import code produces null-value keys in the `tags` column, while the update code suppresses them.  This PR removes that difference and makes the import code use same method that is currently used in the update code.

With a test case I've written, the import code produces a tags hstore that looks like this:
`"name"=>"OpenMapTiles Secondary 2", "name:de"=>NULL, "name:en"=>NULL, "name_int"=>"OpenMapTiles Secondary 2", "name:latin"=>"OpenMapTiles Secondary 2"`

...while the update code produces a tags hstore that looks like this:

`"name"=>"OpenMapTiles Secondary 2", "name_int"=>"OpenMapTiles Secondary 2", "name:latin"=>"OpenMapTiles Secondary 2"`

Note the missing NULL values.

This bug causes a small amount of space wastage after an update is run, because the update matching code detects the `tags` value as different, resulting in a duplicate copy of the tags value if that row is updated.  This causes duplicate objects and breaks GROUP BY clauses that expect to group same-tagged features together.  I've tested this by inspection of a generated mbtiles, database spot checks, and the unit test code included in this PR.
  • Loading branch information
ZeLonewolf committed Nov 25, 2021
1 parent 0cff344 commit ec74480
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 154 deletions.
15 changes: 15 additions & 0 deletions layers/transportation/highway_name.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
CREATE OR REPLACE FUNCTION transportation_name_tags(geometry geometry, tags hstore, name text, name_en text, name_de text) RETURNS hstore AS
$$
SELECT hstore(string_agg(nullif(slice_language_tags(tags ||
hstore(ARRAY [
'name', CASE WHEN length(name) > 15 THEN osml10n_street_abbrev_all(name) ELSE NULLIF(name, '') END,
'name:en', CASE WHEN length(name_en) > 15 THEN osml10n_street_abbrev_en(name_en) ELSE NULLIF(name_en, '') END,
'name:de', CASE WHEN length(name_de) > 15 THEN osml10n_street_abbrev_de(name_de) ELSE NULLIF(name_de, '') END
]))::text,
''), ','))
|| get_basic_names(tags, geometry);
$$ LANGUAGE SQL IMMUTABLE
STRICT
PARALLEL SAFE;


1 change: 1 addition & 0 deletions layers/transportation/transportation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ layer:
schema:
- ./network_type.sql
- ./class.sql
- ./highway_name.sql
- ./update_route_member.sql
- ./update_transportation_merge.sql
- ./transportation.sql
Expand Down
10 changes: 2 additions & 8 deletions layers/transportation/update_transportation_merge.sql
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@ CREATE TABLE IF NOT EXISTS osm_transportation_name_network AS
SELECT
geometry,
osm_id,
name,
name_en,
name_de,
tags,
ref,
highway,
Expand All @@ -32,10 +29,7 @@ FROM (
SELECT DISTINCT ON (hl.osm_id)
hl.geometry,
hl.osm_id,
CASE WHEN length(hl.name) > 15 THEN osml10n_street_abbrev_all(hl.name) ELSE NULLIF(hl.name, '') END AS "name",
CASE WHEN length(hl.name_en) > 15 THEN osml10n_street_abbrev_en(hl.name_en) ELSE NULLIF(hl.name_en, '') END AS "name_en",
CASE WHEN length(hl.name_de) > 15 THEN osml10n_street_abbrev_de(hl.name_de) ELSE NULLIF(hl.name_de, '') END AS "name_de",
slice_language_tags(hl.tags) AS tags,
transportation_name_tags(hl.geometry, hl.tags, hl.name, hl.name_en, hl.name_de) AS tags,
rm1.network_type,
CASE
WHEN rm1.network_type IS NOT NULL AND rm1.ref::text <> ''
Expand Down Expand Up @@ -68,7 +62,7 @@ FROM (
AND hl.highway <> ''
) AS t;
CREATE UNIQUE INDEX IF NOT EXISTS osm_transportation_name_network_osm_id_idx ON osm_transportation_name_network (osm_id);
CREATE INDEX IF NOT EXISTS osm_transportation_name_network_name_ref_idx ON osm_transportation_name_network (coalesce(name, ''), coalesce(ref, ''));
CREATE INDEX IF NOT EXISTS osm_transportation_name_network_name_ref_idx ON osm_transportation_name_network (coalesce(tags->'name', ''), coalesce(ref, ''));
CREATE INDEX IF NOT EXISTS osm_transportation_name_network_geometry_idx ON osm_transportation_name_network USING gist (geometry);

-- Improve performance of the sql in transportation/update_route_member.sql
Expand Down
26 changes: 7 additions & 19 deletions layers/transportation_name/transportation_name.sql
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ CREATE OR REPLACE FUNCTION layer_transportation_name(bbox geometry, zoom_level i
AS
$$
SELECT geometry,
name,
COALESCE(name_en, name) AS name_en,
COALESCE(name_de, name, name_en) AS name_de,
tags->'name' AS name,
COALESCE(tags->'name:en', tags->'name') AS name_en,
COALESCE(tags->'name:de', tags->'name', tags->'name:en') AS name_de,
tags,
ref,
NULLIF(LENGTH(ref), 0) AS ref_length,
Expand Down Expand Up @@ -93,9 +93,6 @@ FROM (

-- etldoc: osm_transportation_name_linestring -> layer_transportation_name:z12
SELECT geometry,
name,
name_en,
name_de,
"tags",
ref,
highway,
Expand All @@ -109,7 +106,7 @@ FROM (
indoor
FROM osm_transportation_name_linestring
WHERE zoom_level = 12
AND LineLabel(zoom_level, COALESCE(name, ref), geometry)
AND LineLabel(zoom_level, COALESCE(tags->'name', ref), geometry)
AND NOT highway_is_link(highway)
AND
CASE WHEN highway_class(highway, NULL::text, NULL::text) NOT IN ('path', 'minor') THEN TRUE
Expand All @@ -120,9 +117,6 @@ FROM (

-- etldoc: osm_transportation_name_linestring -> layer_transportation_name:z13
SELECT geometry,
name,
name_en,
name_de,
"tags",
ref,
highway,
Expand All @@ -136,11 +130,11 @@ FROM (
indoor
FROM osm_transportation_name_linestring
WHERE zoom_level = 13
AND LineLabel(zoom_level, COALESCE(name, ref), geometry)
AND LineLabel(zoom_level, COALESCE(tags->'name', ref), geometry)
AND
CASE WHEN highway <> 'path' THEN TRUE
WHEN highway = 'path' AND (
name <> ''
tags->'name' <> ''
OR network IS NOT NULL
OR sac_scale <> ''
OR route_rank <= 2
Expand All @@ -151,9 +145,6 @@ FROM (

-- etldoc: osm_transportation_name_linestring -> layer_transportation_name:z14_
SELECT geometry,
name,
name_en,
name_de,
"tags",
ref,
highway,
Expand All @@ -172,11 +163,8 @@ FROM (
-- etldoc: osm_highway_point -> layer_transportation_name:z10
SELECT
p.geometry,
p.name,
p.name_en,
p.name_de,
p.tags,
p.tags->'ref',
p.ref,
(
SELECT highest_highway(l.tags->'highway')
FROM osm_highway_linestring l
Expand Down

0 comments on commit ec74480

Please sign in to comment.