BUGFIX: Fix name-based way fragmentation in transportation_name (#1295)

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.
2021-11-25 04:45:11 -05:00
parent 0cff3449b5
commit ec74480414
9 changed files with 159 additions and 154 deletions
--- a/tests/import/500_import-highway.osm
+++ b/tests/import/500_import-highway.osm
@@ -16,8 +16,16 @@
  <node id="500062" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.5" lon="-90.06" />
  <node id="500071" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.0" lon="-90.07" />
  <node id="500072" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.5" lon="-90.07" />
-  <node id="500081" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.0" lon="-90.07" />
-  <node id="500082" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.5" lon="-90.07" />
+  <node id="500081" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="34.0" lon="-90.07" />
+  <node id="500091" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="34.5" lon="-90.07" />
+  <node id="500101" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.0" lon="-90.08" />
+  <node id="500102" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.5" lon="-90.08" />
+  <node id="500111" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="34.0" lon="-90.08" />
+  <node id="500121" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="34.5" lon="-90.08" />
+  <node id="500131" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.0" lon="-90.09" />
+  <node id="500132" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.5" lon="-90.09" />
+  <node id="500141" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.0" lon="-90.10" />
+  <node id="500142" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" lat="33.5" lon="-90.10" />

  <way id="5000" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
    <nd ref="500001" />
@@ -69,27 +77,71 @@
    <tag k="highway" v="track"/>
    <tag k="name" v="OpenMapTiles Track"/>
  </way>
+
+  <!-- Test way stitching -->
  <way id="5007" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
    <nd ref="500071" />
    <nd ref="500072" />
+    <tag k="highway" v="secondary"/>
+    <tag k="name" v="OpenMapTiles Secondary 2"/>
+  </way>
+  <way id="5008" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500072" />
+    <nd ref="500081" />
+    <tag k="highway" v="secondary"/>
+    <tag k="name" v="OpenMapTiles Secondary 2"/>
+  </way>
+  <way id="5009" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500081" />
+    <nd ref="500091" />
+    <tag k="highway" v="secondary"/>
+    <tag k="name" v="OpenMapTiles Secondary 2"/>
+  </way>
+
+  <!-- Test disrupted way stitching -->
+  <way id="5010" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500101" />
+    <nd ref="500102" />
+    <tag k="highway" v="secondary"/>
+    <tag k="bicycle" v="yes"/>
+    <tag k="name" v="OpenMapTiles Secondary 3"/>
+  </way>
+  <way id="5011" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500102" />
+    <nd ref="500111" />
+    <tag k="highway" v="secondary"/>
+    <tag k="bridge" v="yes"/>
+    <tag k="layer" v="1"/>
+    <tag k="toll" v="yes"/>
+    <tag k="name" v="OpenMapTiles Secondary 3 (Toll)"/>
+  </way>
+  <way id="5012" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500111" />
+    <nd ref="500121" />
+    <tag k="highway" v="secondary"/>
+    <tag k="name" v="OpenMapTiles Secondary 3"/>
+  </way>
+  <way id="5013" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500131" />
+    <nd ref="500132" />
    <tag k="highway" v="path"/>
    <tag k="name" v="OpenMapTiles Path z13"/>
  </way>
-  <way id="5008" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
-    <nd ref="500081" />
-    <nd ref="500082" />
+  <way id="5014" version="1" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <nd ref="500141" />
+    <nd ref="500142" />
    <tag k="highway" v="path"/>
    <tag k="name" v="OpenMapTiles Track z12"/>
  </way>

-  <relation id="507" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" changeset="1" user="u" uid="1">
-    <member type="way" ref="5007" role=""/>
+  <relation id="513" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" changeset="1" user="u" uid="1">
+    <member type="way" ref="5013" role=""/>
    <tag k="type" v="route"/>
    <tag k="route" v="hiking"/>
    <tag k="colour" v="red"/>
  </relation>
-  <relation id="508" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" changeset="1" user="u" uid="1">
-    <member type="way" ref="5008" role=""/>
+  <relation id="514" visible="true" timestamp="2019-01-01T00:00:00Z" version="1" changeset="1" user="u" uid="1">
+    <member type="way" ref="5014" role=""/>
    <tag k="type" v="route"/>
    <tag k="route" v="hiking"/>
    <tag k="network" v="nwn"/>
--- a/tests/test-post-import.sql
+++ b/tests/test-post-import.sql
@@ -73,8 +73,8 @@ BEGIN

  -- Verify that road classifications show up at the right zoom level
  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z4 WHERE highway='motorway';
-  IF cnt <> 1 THEN
-    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z4 motorway count expected 1, got ' || cnt);
+  IF cnt < 1 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z4 motorway count expected >=1, got ' || cnt);
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z4 WHERE highway='trunk';
@@ -83,8 +83,8 @@ BEGIN
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z5 WHERE highway='trunk';
-  IF cnt <> 1 THEN
-    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z5 trunk count expected 1, got ' || cnt);
+  IF cnt < 1 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z5 trunk count expected >=1, got ' || cnt);
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z6 WHERE highway='primary';
@@ -93,8 +93,8 @@ BEGIN
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z7 WHERE highway='primary';
-  IF cnt <> 1 THEN
-    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z7 primary count expected 1, got ' || cnt);
+  IF cnt < 1 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z7 primary count expected >=1, got ' || cnt);
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z8 WHERE highway='secondary';
@@ -103,8 +103,8 @@ BEGIN
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z9 WHERE highway='secondary';
-  IF cnt <> 1 THEN
-    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z9 secondary count expected 1, got ' || cnt);
+  IF cnt < 1 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z9 secondary count expected >=1, got ' || cnt);
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z10 WHERE highway='tertiary';
@@ -113,8 +113,8 @@ BEGIN
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z11 WHERE highway='tertiary';
-  IF cnt <> 1 THEN
-    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z11 tertiary count expected 1, got ' || cnt);
+  IF cnt < 1 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z11 tertiary count expected >=1, got ' || cnt);
  END IF;

  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z11 WHERE highway IN ('service', 'track');
@@ -133,6 +133,12 @@ BEGIN
    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring z9 import tags expected 1, got ' || cnt);
  END IF;

+  -- Same-named road split into 3 parts, because the middle segment is tagged toll=yes
+  SELECT COUNT(*) INTO cnt FROM osm_transportation_name_linestring WHERE tags->'name' = 'OpenMapTiles Secondary 3';
+  IF cnt <> 2 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'import', 'osm_transportation_linestring split road count expected 2, got ' || cnt);
+  END IF;
+
  SELECT COUNT(*) INTO cnt FROM osm_transportation_name_linestring
    WHERE tags->'name' = 'OpenMapTiles Path z13'
      AND route_rank = 2;
--- a/tests/test-post-update.sql
+++ b/tests/test-post-update.sql
@@ -48,7 +48,14 @@ BEGIN
    INSERT INTO omt_test_failures VALUES(400, 'update', 'osm_border_linestring city count expected 2, got ' || cnt);
  END IF;

-  -- Test 500: Verify tags changed
+  -- Test 500: Highways
+  -- Same-named road previous split into 3 parts, now merged because the middle segment had toll=yes removed
+  SELECT COUNT(*) INTO cnt FROM osm_transportation_name_linestring WHERE tags->'name' = 'OpenMapTiles Secondary 3';
+  IF cnt <> 1 THEN
+    INSERT INTO omt_test_failures VALUES(500, 'update', 'osm_transportation_linestring unsplit road count expected 1, got ' || cnt);
+  END IF;
+
+  -- Verify tags changed
  SELECT COUNT(*) INTO cnt FROM osm_transportation_merge_linestring_gen_z9
    WHERE is_tunnel = TRUE
      AND is_bridge = FALSE
--- a/tests/update/500_update-highway.osc
+++ b/tests/update/500_update-highway.osc
@@ -6,7 +6,7 @@
  -->
  <modify>
    <!-- Change tags -->
-    <way id="5003" version="2" timestamp="2019-01-01T00:00:00Z" visible="true">
+    <way id="5003" version="2" timestamp="2020-01-02T00:00:00Z" visible="true">
      <nd ref="500031" />
      <nd ref="500032" />
      <tag k="highway" v="secondary"/>
@@ -19,5 +19,19 @@
      <tag k="horse" v="yes"/>
      <tag k="foot" v="yes"/>
    </way>
+
+    <!-- Remove toll to make way match adjoining ways -->
+    <way id="5010" version="2" timestamp="2019-01-01T00:00:00Z" visible="true">
+      <nd ref="500101" />
+      <nd ref="500102" />
+      <tag k="highway" v="secondary"/>
+      <tag k="name" v="OpenMapTiles Secondary 3"/>
+    </way>
+    <way id="5011" version="2" timestamp="2020-01-02T00:00:00Z" visible="true">
+      <nd ref="500102" />
+      <nd ref="500111" />
+      <tag k="highway" v="secondary"/>
+      <tag k="name" v="OpenMapTiles Secondary 3"/>
+    </way>
  </modify>
 </osmChange>