This PR removes unneeded `CASE` clauses which converts `toll` tags to booleans. However, the `toll` tag is already mapped as a boolean by imposm, therefore this processing can be simplified.
This PR is a bugfix for the `transportation_name` layer. Currently, the updates process does not work, as noted by @zstadler in #1230. The issue is that the computed `concurrency_index` column in `osm_route_member` is never updated after the initial updates. Therefore, whenever route member ways are updated, they will lose their route concurrency index, and hence route concurrencies will be lost in the `transportation_name` tiles. This PR adds the missing concurrency index update code to the transportation/network update triggers.
This PR as written also incorporates the fixes in #1230 and #1233.
The following SQL is a unit test to demonstrate that these triggers work correctly:
```
select count(*) from osm_route_member where concurrency_index > 5;
select count(*) from osm_transportation_name_network where route_6 is not null;
update osm_route_member set concurrency_index=NULL where concurrency_index > 5;
select count(*) from osm_route_member where concurrency_index > 5;
select count(*) from osm_transportation_name_network where route_6 is not null;
```
If working correctly, both pairs of `count(*)` values should return the same pair of numbers.
While troubleshooting #1230, I discovered that there were cases where a way was a member of the same route relation more than once, such as: https://www.openstreetmap.org/way/17439235
<img src="https://user-images.githubusercontent.com/3254090/133726616-acd01332-343e-4930-ad3d-1fb3c3190a4c.png" width=280/>
This segment would end up showing duplicate highway shields in a rendered style, which is not desirable. While I personally think this style of tagging is wrong, it's a relatively easy fix to remove those duplicates. This PR replaces `ROW_NUMBER()` with `DENSE_RANK()` in the concurrency index generation code and adds a `DISTINCT` constraint on the concurrency join to handle multiple member rows with the same concurrency index.
Since I was in this file, I also fixed the copy/paste error in generating the route names.
Screenshot from database showing correct mapping:
This PR is a refactor. The purpose of this PR is to refactor the logic in the z12-z14 transportation layer to increase readability and reduce code complexity.
In addition, this fixes bugs in the original filter code:
* Create filter parity between `service` and `service_construction` highway classes.
* Removal of unintended `class` values `minor_construction`, `path_construction`, `service_construction`, and `track_construction` from zoom 12 filter by explicitly listing the highway classes that are included.
Fixes#1191
This PR suppresses `highway=service` at zoom 12, where it is not a useful level of detail. This makes OpenMapTiles consistent with openstreetmap-carto, which does not begin showing `highway=service` until raster zoom 14 / vector zoom 13.
Additionally, this PR suppresses `highway=service` + `service=parking_aisle` / `service=driveway` from zooms 12-13, as this detail is excessive below zoom 14. As a point of comparison, openstreetmap-carto does not begin showing `service=parking_aisle` / `service=driveway` until raster zoom 16 (vector zoom 15).
Fixes#366
This PR sets `toll=1` in the `transportation` layer when a road is tagged as a toll road in OSM (with the tag `toll=yes`). If a road is tagged with any other value of `tag=*`, the value is suppressed in the tile, since non-toll roads can be presumed as the default and therefore this PR should have only negligible impact in the tiles.
Support for toll road tagging is of interest in the American mapping community, because toll roads have historically been styled differently on American-style maps, for example:
Screen shot of a toll road being generated in the `transportation` layer tiles:
Fixes#1066
This PR adds a new field `access` in the transportation layer, which will be set to `no` if the `access` tag is either `no` or `private`. While `private` is the more popular value by a 17:1 ratio, I went with `no` because it's smaller in the tiles. In addition, the text `no` opens up the future possibility of other text-based access values such as `destination`, `customers`, or `permit`.
The screenshot below shows an example of access tagging for a road on a military base:
This PR updates the `transportation` layer creation scripts to simplify the SQL and remove unneeded sub-selects, checks and conditionals, fix indexes, and improve inline documentation.
Currently, there are two sequences of materialized view creations. There is one from zoom 11 through 9, and a second one for zoom 8 through 4. This PR removes that break in the sequence of transportation table materialized view creations, in favor of one in which high-zoom views are created first, and then each lower zoom is created from the zoom above.
Instead, the current generalized zoom 8 transportation table is built directly from `osm_transportation_linestring` rather than being built from the zoom 9 transportation table. This means that when building the zoom 8 table, it must scan the entire transportation network rather than just selecting from the pre-filtered zoom 9 table.
This PR removes an unneeded sub-select in the build of the zoom 8 table, which appears to be a leftover from an old version of the SQL that did some sort of merge. Once this PR is implemented all zooms from 11 through 4 will be linked via a progressive series of materialized views.
Lastly, this adds in missing materialized view refreshes for zooms 9-11, which appear to have been entirely missing, and as far as I can tell aren't getting updated in the current version of this code.
In addition, the following optimizations were added as part of this commit:
1. Updates the `osm_highway_linestring_highway_partial_idx` partial index to match the `SELECT..WHERE` clause actually present in `transportation/update_route_member.sql`, which is where it appears to be actually used, and update inline documentation to reflect this.
2. Collapses unnecessary sub-select block in `osm_transportation_merge_linestring_gen_z11`, and removes unnecessary ST_IsValid() call, which already provided in `mapping.yaml`, and update inline documentation to reflect these assumptions.
3. Updates `WHERE` blocks to remove unnecesary checks and further document assumptions. The `highway=construction` check is unnecessary in all cases, because it is sufficient to check the `construction` key alone. If `construction=*` is set, then `highway=construction` is implied.
4. Two indexes were added to `layers/transportation/update_route_member.sql` to improve route population performance.
In testing locally, I'm seeing performance improvements around 10% in the generation of the `transportation` layer, based on modifying `openmaptiles.yaml` to generate only the transportation layer and then repeatedly running `time make import-sql`, however, this timing might be impacted by docker, so I would ask for confirmation of acceptable performance.
In addition, this PR shortens the length of the transportation update SQL file by 30 lines, which makes it easier for contributors to work with.
PR #1168 removed several `WHERE` clauses in the `transportation_merge_*` table series. With those removed, it appears that `osm_transportation_merge_linestring_gen_z8` and `osm_transportation_merge_linestring` are nearly identical, with the former simply adding an `ST_Simplify()` operation.
A grep of the codebase indicates that the _only_ use for `osm_transportation_merge_linestring` is to hold the intermediate result of an `ST_Dump(geometry))` before it is fed into `ST_Simplify()`. Therefore, it appears that we're holding an entire zoom 8 copy of the transportation layer (all motorway/trunk/primary roads) in a materialized view for absolutely no reason at all.
This PR removes the `osm_transportation_merge_linestring` intermediate table and changes the definition of `osm_transportation_merge_linestring_gen_z8` to perform the `ST_Dump()` and `ST_Simplify()` transformations directly from the `osm_highway_linestring` table in a single operation.
This PR removes several redundant/unnecessary WHERE clauses from the transportation layer. Specifically:
The table `osm_transportation_merge_linestring` is a view of `osm_highway_linestring` which exposes only motorway/trunk/primary roads:
9e4be3e3b0/layers/transportation/update_transportation_merge.sql (L122-L123)
However, the create statement for the table `osm_transportation_merge_linestring_gen_z8`, which is a view of `osm_transportation_merge_linestring`, also contains a `WHERE` clause which selects down to motorway/trunk/primary roads. This `WHERE` is unnecessary:
9e4be3e3b0/layers/transportation/update_transportation_merge.sql (L144-L145)
This unneeded `WHERE` clause is similarly present in the create statement for `osm_transportation_merge_linestring_gen_z7`, which is a view of `osm_transportation_merge_linestring_gen_z8`:
9e4be3e3b0/layers/transportation/update_transportation_merge.sql (L163-L164)
Likewise, there is a similar redundant `WHERE` clause in the `osm_transportation_merge_linestring_gen_z5` and `osm_transportation_merge_linestring_gen_z6` tables, both of which select down to `motorway` and `trunk`. This `WHERE` clause is only needed on the z6 table, and is redundant on the z5 table.
I am not sure what the performance penalty is for these redundant `WHERE` clauses, but there does not appear to be any reason to keep them, and they may incur a performance cost.
This PR adds a layer for `highway=motorway_junction` features.
This implementation of highway exits in the transportation_name layer add to the existing layer table structure, and renames the internal column name from "construction" (which was already overloaded with non-construction usages) to "subclass", which will be less confusing to future developers. The string 'junction' is used as the universal sub-class for highway exits.
A new documentation PR has been opened at openmaptiles/www.openmaptiles.org#69 to reflect these changes in the documentation.
To avoid discontinuous transportation lines between zooms 9 and 11.
- Originally limit geometry by length for z9 - z11 (`ST_Length(geometry) > ZRes(11)`)
- highway z9 to z11 was generalized during import-osm
- now just create a filtered and generalized z11 table
- then merge segments in the same way as from (full-featured) osm_highway_linestring and used this merged z11 for mat.view z10 and z9
Close#1107
On creation of materialized views `osm_transportation_merge_linestring_gen_zX`, an index is created each time the new view and not used else where than the next generalization step.
It does not worth it to to a scan on the table to create index that will be used only once, while the next generalisation step can also be done in one full scan.
So, removing these indices.
https://github.com/openmaptiles/openmaptiles/issues/999
- Point 1 solved only partially - there is `brunnel` value added into zooms 9, 10, 11. For zooms 8 and smaller it would need change in osm_transportation_merge_ tables - I am going to try it but maybe its SQL costs will be to high.
- Point 2 solved.
The index `osm_highway_linestring_highway_idx` is not used. There is also a used partial index `osm_highway_linestring_highway_partial_idx`.
I cross check the usage of the index from postgres query stats and all SQL query code where the table `osm_highway_linestring` is refered.
This index is relatively big.
* All functions that access database must be declared as `STABLE`, not `IMMUTABLE` -- because database can change at any moment, e.g. during an update
* there are a few functions that could be made `STRICT` -- passing `NULL` as a parameter will always result in a `NULL`, but for some reason that causes a significant decrease in perf.
* tagged one function as parallel safe
NOTE: somehow `ST_AsMVT()` method of tile generation is showing 70-90% slowdown with this patch. I am not sure of why this is happening. If the reason is the `IMMUTABLE` -> `STABLE` change, we may have to dig deeper into PG optimization
I would like to reformat all of our SQL to have a concise coding style.
This makes it far easier to understand the code for a casual contributor,
and lets us spot errors more easily.
Most importantly, it makes it much easier to grep (search) the code because it is more likely to be in the same syntax
Some key changes:
* SQL keywords are always UPPERCASE, e.g. `SELECT WHEN AS END ...`
* types, variables, aliases, and field names (identifiers) are always lower case
* `LANGUAGE 'plpgsql'` is now `LANGUAGE plpgsql` (no quotes)
* a few minor spacing/semicolon cleanups
P.S. Per @TomPohys request, `TABLE` is spelled using upper case despite being a type for consistency with PG Docs. Same for `LANGUAGE SQL` vs `LANGUAGE plpgsql`.
This PR allows queries to be parallelized on recent versions of Postgres. The `PARALLEL SAFE` modifier has been added to the layer functions and a PLPGSQL function to convert strings into number has been replaced.
`PARALLEL SAFE` is a modifier for `CREATE FUNCTION` available since Postgres 9.6, so this change does not break current OpenMapTiles supported database version. More details about this topic [here](https://www.postgresql.org/docs/current/parallel-safety.html) and at the reference documentation for [`CREATE FUNCTION`](https://www.postgresql.org/docs/current/sql-createfunction.html).
### Testing procedure
The procedure to test this was:
* Imported `spain.pbf` in a clean environment
* Dumped the OpenMapTiles database from the Postgres Docker image
* Created a clean Postgres 12 database using the default Docker image
* Installed `postgis` 3 from the default Debian package and `osml10n` 2.5.8 from the repository (`make`, etc.)
* Restored the dump
* Lowered the postgres planner parameters for triggering parallel plans:
```sql
set parallel_setup_cost = 5;
set parallel_tuple_cost = 0.005;
```
* Manually added the `PARALLEL SAFE` modifier to each function involved in layer queries (not on updates or inserting functions).
* For each layer, run a testing query to confirm parallel workers were created, something like this:
```sql
explain analyze
select * from layer_aerodrome_label(tilebbox(8,128,95),10,null)
union all
select * from layer_aerodrome_label(tilebbox(8,128,97),10,null);
```
* After all the layers were processed and confirmed to start parallel executions, a more complete example was run. This example just retrieves the geometries for all the layers from the same tile but without using any MVT related function.
<details><summary>Testing query</summary>
```sql
-- Using the function layer_landuse
explain analyze
select geometry from layer_water(tilebbox(14,8020,6178),14)
union all
select geometry from layer_waterway(tilebbox(14,8020,6178),14)
union all
select geometry from layer_landcover(tilebbox(14,8020,6178),14)
union all
select geometry from layer_landuse(tilebbox(14,8020,6178),14)
union all
select geometry from layer_mountain_peak(tilebbox(14,8020,6178),14)
union all
select geometry from layer_park(tilebbox(14,8020,6178),14)
union all
select geometry from layer_boundary(tilebbox(14,8020,6178),14)
union all
select geometry from layer_aeroway(tilebbox(14,8020,6178),14)
union all
select geometry from layer_transportation(tilebbox(14,8020,6178),14)
union all
select geometry from layer_building(tilebbox(14,8020,6178),14)
union all
select geometry from layer_water_name(tilebbox(14,8020,6178),14)
union all
select geometry from layer_transportation_name(tilebbox(14,8020,6178),14)
union all
select geometry from layer_place(tilebbox(14,8020,6178),14)
union all
select geometry from layer_housenumber(tilebbox(14,8020,6178),14)
union all
select geometry from layer_poi(tilebbox(14,8020,6178),14)
union all
select geometry from layer_aerodrome_label(tilebbox(14,8020,6178),14);
```
</details>
You can inspect the execution plan and results on [this page](https://explain.dalibo.com/plan/3z). Also [attaching](https://github.com/openmaptiles/openmaptiles/files/3951822/explain-tile-simple.tar.gz) the query and JSON output for future reference. The website gives a ton of details, but you may want to search for nodes mentioning `workers` or `parallel` like in this area referring to `osm_border` or `osm_aeroway_linestring` entities
### Next steps
Since the execution plan is not showing a parallel append at the top level, meaning it's not running each layer individually, I want to continue experimenting with parameters and queries to see if it's possible to even parallelize more the request.
I will post my finding here, even no change in the code should happen.
cc. @nyurik
Co-authored-by: Yuri Astrakhan <yuriastrakhan@gmail.com>
Make a few more mappings declarative, and removes values declared in both SQL and the yaml file.
Here's the diff comparing `build/tileset.sql` in master vs the new PR. The changes are mostly stylistic, except when a nested `if` statement is expanded into individual `if ... and ...` conditions (logically identical)
```diff
55c55
diff --git a/build/tileset.sql b/build/tileset.sql
index 4e59357..7c6c444 100644
--- a/build/tileset.sql
+++ b/build/tileset.sql
@@ -52,7 +52,7 @@ CREATE INDEX IF NOT EXISTS osm_ocean_polygon_gen4_idx ON osm_ocean_polygon_gen4
CREATE OR REPLACE FUNCTION water_class(waterway TEXT) RETURNS TEXT AS $$
SELECT CASE
WHEN "waterway" IN ('', 'lake') THEN 'lake'
- WHEN "waterway"='dock' THEN 'dock'
+ WHEN "waterway" = 'dock' THEN 'dock'
ELSE 'river'
END;
$$ LANGUAGE SQL IMMUTABLE;
@@ -1813,24 +1813,41 @@ CREATE OR REPLACE FUNCTION highway_class(highway TEXT, public_transport TEXT, co
WHEN "highway" IN ('tertiary', 'tertiary_link') THEN 'tertiary'
WHEN "highway" IN ('unclassified', 'residential', 'living_street', 'road') THEN 'minor'
WHEN "highway" IN ('pedestrian', 'path', 'footway', 'cycleway', 'steps', 'bridleway', 'corridor')
- OR "public_transport"='platform'
+ OR "public_transport" = 'platform'
THEN 'path'
- WHEN "highway"='service' THEN 'service'
- WHEN "highway"='track' THEN 'track'
- WHEN "highway"='raceway' THEN 'raceway'
- WHEN highway = 'construction' THEN CASE
- WHEN construction IN ('motorway', 'motorway_link') THEN 'motorway_construction'
- WHEN construction IN ('trunk', 'trunk_link') THEN 'trunk_construction'
- WHEN construction IN ('primary', 'primary_link') THEN 'primary_construction'
- WHEN construction IN ('secondary', 'secondary_link') THEN 'secondary_construction'
- WHEN construction IN ('tertiary', 'tertiary_link') THEN 'tertiary_construction'
- WHEN construction IN ('', 'unclassified', 'residential', 'living_street', 'road') THEN 'minor_construction'
- WHEN construction IN ('pedestrian', 'path', 'footway', 'cycleway', 'steps', 'bridleway', 'corridor')
- OR public_transport = 'platform' THEN 'path_construction'
- WHEN construction = 'service' THEN 'service_construction'
- WHEN construction = 'track' THEN 'track_construction'
- WHEN construction = 'raceway' THEN 'raceway_construction'
- END
+ WHEN "highway" = 'service' THEN 'service'
+ WHEN "highway" = 'track' THEN 'track'
+ WHEN "highway" = 'raceway' THEN 'raceway'
+ WHEN "highway" = 'construction'
+ AND "construction" IN ('motorway', 'motorway_link')
+ THEN 'motorway_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" IN ('trunk', 'trunk_link')
+ THEN 'trunk_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" IN ('primary', 'primary_link')
+ THEN 'primary_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" IN ('secondary', 'secondary_link')
+ THEN 'secondary_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" IN ('tertiary', 'tertiary_link')
+ THEN 'tertiary_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" IN ('', 'unclassified', 'residential', 'living_street', 'road')
+ THEN 'minor_construction'
+ WHEN "highway" = 'construction'
+ AND ("construction" IN ('pedestrian', 'path', 'footway', 'cycleway', 'steps', 'bridleway', 'corridor') OR "public_transport" = 'platform')
+ THEN 'path_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" = 'service'
+ THEN 'service_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" = 'track'
+ THEN 'track_construction'
+ WHEN "highway" = 'construction'
+ AND "construction" = 'raceway'
+ THEN 'raceway_construction'
END;
$$ LANGUAGE SQL IMMUTABLE;
@@ -4073,6 +4090,12 @@ RETURNS TEXT AS $$
WHEN "subclass" IN ('fast_food', 'food_court') THEN 'fast_food'
WHEN "subclass" IN ('park', 'bbq') THEN 'park'
WHEN "subclass" IN ('bus_stop', 'bus_station') THEN 'bus'
+ WHEN ("subclass" = 'station' AND "mapping_key" = 'railway')
+ OR "subclass" IN ('halt', 'tram_stop', 'subway')
+ THEN 'railway'
+ WHEN "subclass" = 'station'
+ AND "mapping_key" = 'aerialway'
+ THEN 'aerialway'
WHEN "subclass" IN ('subway_entrance', 'train_station_entrance') THEN 'entrance'
WHEN "subclass" IN ('camp_site', 'caravan_site') THEN 'campsite'
WHEN "subclass" IN ('laundry', 'dry_cleaning') THEN 'laundry'
@@ -4082,7 +4105,7 @@ RETURNS TEXT AS $$
WHEN "subclass" IN ('hotel', 'motel', 'bed_and_breakfast', 'guest_house', 'hostel', 'chalet', 'alpine_hut', 'dormitory') THEN 'lodging'
WHEN "subclass" IN ('chocolate', 'confectionery') THEN 'ice_cream'
WHEN "subclass" IN ('post_box', 'post_office') THEN 'post'
- WHEN "subclass"='cafe' THEN 'cafe'
+ WHEN "subclass" = 'cafe' THEN 'cafe'
WHEN "subclass" IN ('school', 'kindergarten') THEN 'school'
WHEN "subclass" IN ('alcohol', 'beverages', 'wine') THEN 'alcohol_shop'
WHEN "subclass" IN ('bar', 'nightclub') THEN 'bar'
@@ -4098,9 +4121,6 @@ RETURNS TEXT AS $$
WHEN "subclass" IN ('bag', 'clothes') THEN 'clothing_store'
WHEN "subclass" IN ('swimming_area', 'swimming') THEN 'swimming'
WHEN "subclass" IN ('castle', 'ruins') THEN 'castle'
- WHEN (subclass = 'station' AND mapping_key = 'railway')
- OR (subclass IN ('halt', 'tram_stop', 'subway')) THEN 'railway'
- WHEN (subclass = 'station' AND mapping_key = 'aerialway') THEN 'aerialway'
ELSE subclass
END;
$$ LANGUAGE SQL IMMUTABLE;
@@ -4301,22 +4321,22 @@ $$
COALESCE(NULLIF(name_de, ''), name, name_en) AS name_de,
tags,
CASE
- WHEN "aerodrome"='international'
- OR "aerodrome_type"='international'
+ WHEN "aerodrome" = 'international'
+ OR "aerodrome_type" = 'international'
THEN 'international'
- WHEN "aerodrome"='public'
- OR "aerodrome_type"='civil'
+ WHEN "aerodrome" = 'public'
+ OR "aerodrome_type" = 'civil'
OR "aerodrome_type" LIKE '%public%'
THEN 'public'
- WHEN "aerodrome"='regional'
- OR "aerodrome_type"='regional'
+ WHEN "aerodrome" = 'regional'
+ OR "aerodrome_type" = 'regional'
THEN 'regional'
- WHEN "aerodrome"='military'
+ WHEN "aerodrome" = 'military'
OR "aerodrome_type" LIKE '%military%'
- OR "military"='airfield'
+ OR "military" = 'airfield'
THEN 'military'
- WHEN "aerodrome"='private'
- OR "aerodrome_type"='private'
+ WHEN "aerodrome" = 'private'
+ OR "aerodrome_type" = 'private'
THEN 'private'
ELSE 'other'
END AS class,
```
quicker and cleaner diagram image generation.
Remove etl-graph and mapping-graph targets - redundant
Also, the obsolete "fields" is still in Imposm's code and both names are accepted,
but "fields" is not documented anywhere (PR submitted), and could be removed at any moment.
Our docs were not supporting it until this PR, so renaming it at the same time.
Several images have been updated due to a more inclusive mapping scan
Requires https://github.com/openmaptiles/openmaptiles-tools/pull/147 (merged)
Minor code cleanup:
SQL already returns NULL in the "WHEN" condition
if it is not matched by any of the cases.
Co-authored-by: Eva Jelinkova <evka.jelinkova@gmail.com>
Mark all tables that should not be populated with the Wikidata
international labels with a special OMT-specific flag.
This should be ok to merge even before the new tools version
is used because imposm seems to ignore anything it doesn't understand.
The next tools version will remove it when generating imposm mapping file.
Tag all SQL materialized views with a machine-readable comment
to indicate that this materialized view can be created without
data:
/* DELAY_MATERIALIZED_VIEW_CREATION */
In the next version of tools this comment can be optionally
replaced with the "WITH NO DATA" parameter, thus allowing
a much faster execution of the SQL script. All materialized
viewes will be populated with data in parallel afterwards
using the `refresh-views` tools script.
Simplify some of the OSM->OMT field value mappings using declarative syntax.
This approach is not for all cases, but in many it removes
the need of storing the same field in both the .yaml and .sql files.
TODO: support more complex AND/OR cases
Seems like etl and mapping diagrams have been neglected
for a long time. Now it regenerates the files and places
them in the source dir.
This PR also fixes two broken files:
* layers/aerodrome_label/mapping_diagram.png
* layers/housenumber/mapping_diagram.png
They were generated using the newest tools version with the fix
https://github.com/openmaptiles/openmaptiles-tools/pull/65
Brian Sperlongano
