Improved update performance of mountain_peak layer
- Refactored IDs to be unique in mountain_linestring.osm_ids and mountain_peak_point.osm_ids
- Restricted updates to INSERT and UPDATE operations during mountain_linestring.refresh and mountain_peak_point.refresh
- Added analyze statements before update queries during mountain_linestring.refresh and mountain_peak_point.refresh
This PR addresses that by:
* rendering oneway arrows on more than `subclass=cycleway`
* less opacity for path/footway casing
* adding the thin black casing to bridges
* adding `subclass=steps` & `subclass=bridleway` rendering
* render `bicycle=designated` as `subclass=cycleway` and `horse=designated` as `subclass=bridleway` (bike taking priority)
* improved tunnel rendering with double casing & adding main line
* made casing min zoom consistent across subclasses
All of these changes were made to mimic OSM Carto.
* OpenMapTiles style
* Rename style snippets from layer_name.json to defaultstyle.json
* Add README to icons dir.
* Add README to icons dir.
* Rename snippets to style.json
* Fix spritezero - move icons to svg subdir
* Move icons from style/icons/svg directly to style/icons
Co-authored-by: Tomas Pohanka <TomPohys@gmail.com>
This PR adds support for several features in the `natural` key, including `saddle`, `ridge`, `arete`, and `cliff`. These features all currently render in openstreetmap-carto, so adding these features would advance OpenMapTiles towards its goal of parity with the standard renderer, as indicated in https://github.com/openstreetmap/operations/issues/565#issuecomment-907303115.
This PR also adds the features requested in #274, with the exception of valleys, which I've left out of this PR because there are different complexities to mapping valleys that should be addressed as a separate (future) PR.
### Examples of mountain features in paper maps
These features are regularly found in American-style maps and thus is of interest to the openstreetmap-americana map style. Below are examples of these mountain-related features found in general purpose maps:
<img src="https://user-images.githubusercontent.com/3254090/131270340-af80c7bf-9416-40a3-9b64-56418abb2aef.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270375-b9eb2095-7708-443d-b7bc-22bf8adab721.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270423-64447ad3-d90a-4615-82e1-91175a8f8a6b.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270506-2248db9f-ded5-443f-82ed-b0e8d4d12a70.png" width=400/>
### Approach
This PR extends the existing `mountain_peak` layer by adding other mountain features. We may want to consider renaming this layer in version 4.0 to be more inclusive of other mountain features including the potential future addition of valleys. However, the features added in this PR are associated with mountains and so their inclusion in this layer is the most appropriate location.
A new `osm_mountain_linestring` mapping maps the new linear mountain features, with similar ranking logic as is used for the existing `peak`/`volcano` features. Additionally, `natural=saddle` is added to the `osm_peak_point` mapping and ranked using the formula for peaks. Since saddles will have lower elevations than peaks, important saddles will be preempted by important peaks.
The new linestring features are rendered only at zoom 13-14, in order to match the zoom at which they appear in openstreetmap-carto. However, it may be appropriate in a future PR to extend the rendering of these features as generalized linestrings at lower zooms.
### Test Renderings
Below is a test rendering showing aretes and peaks in Austria, just north of the Swiss border, followed by a screen shot of the [same location](https://www.openstreetmap.org/#map=14/46.8682/10.0863) in openstreetmap-carto:
**OpenMapTiles, zoom 14**
<img src="https://user-images.githubusercontent.com/3254090/131271258-5cd90bdb-cac2-41d8-887f-b4bf6be83673.png" width=400/>
**openstreetmap-carto, zoom r14/v13:**
<img src="https://user-images.githubusercontent.com/3254090/131271332-32d5bcfc-41c6-4625-829c-df063b7af523.png" width=400/>
Replacing update on the whole table with an update only on changed rows.
The goal is to update more quickly by just updating the changing content.
The update now focus on osm_id of changed rows, it use index. Add a where clause tags != update_tags(tags, geometry) en ensure only update when changed.
It requires one more trigger and a table to store changed osm_id.
The UPDATE is keep in a function to be reusable for initial setup and trigger update.
I try many code layout before done it in this way with the goal to keep the code for initial pass and for update. It should have low impact on initial data load. Better performance for row update can be achieve with BEFORE UPDATE, but require to duplicate the logic.
It is not based on the already merged https://github.com/openmaptiles/openmaptiles/pull/896 because calling and update within a function for each updated row was not efficient for larger table (like housenumber).
It addresses #814.
* Remake update_peak_point use incremental update #814
* Make update_aerodrome_label_point use incremental update #814
* Make housenumber_centroid use incremental update #814
* Make update_continent_point use incremental update #814
* Make update_island_point use incremental update #814
* Make update_island_polygon use incremental update #814
* Remove dead code in update_state_point.sql
* Make update_state_point use incremental update #814
* Remove dead code in update_country_point.sql
* Make update_country_point use incremental update #814
* Make osm_poi_polygon use incremental update #814
Thanks @frodrigo
* 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>
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)
aerodrome_label & mountain_peak queries return `osm_id`, but they
are not declared in the data source. I think we should either remove
the `osm_id` from the query result, or declare the data source.
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
benedikt-brandtner-bikemap
