-
Notifications
You must be signed in to change notification settings - Fork 819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to Lua transforms and change database schema #2533
Conversation
**This is a pull-request against the lua branch** This PR proposes a database-reload. It changes the documentation on the use of osm2pgsql, and adds a .lua file for preprocessing. This database import aims to be backwards compatible with older versions of the style. This resolves (at least part of) #1504. #Hstore Adding the hstore option to osm2pgsql allows the database to use all keys. This PR proposes using hstore for new keys, and using traditional columns for old keys. This allows us to stay compatible with old versions of the style. According to [@pnorman's benchmarks](http://www.paulnorman.ca/blog/2014/03/osm2pgsql-and-hstore/), using hstore without dropping columns would lead to a 9% size increase and a 0.38% speed decrease. Given the benefits of having all columns available, I would consider this acceptable. # Make polygon/linestring decision based on value This is based on the following unmerged PR to osm2pgsql: osm2pgsql-dev/osm2pgsql#346 It makes the polygon/linestring decision based on the tag value, in addition to the tag key. In particular, highway=services and junction=yes are now treated as polygon, while leisure=track, man_made=embankment, man_made=breakwater, man_made=groyne, natural=cliff, natural=tree_row, historic=citywalls, waterway=derelict_canal, waterway=ditch, waterway=drain, waterway=river, waterway=stream, waterway=wadi, waterway=weir, power=line, and power=minor_line are now treated as linestring. This resolves #1362, resolves #137, resolves #268, and resolves #892. # Rendering order The new .lua file creates a osmcarto_z_order value in the database, which stores the rendering order we use. This will allow us to simplify the road queries. # Recommend -G flag for multipolygons This resolves #59. # Import ele on buildings This resolves #101. # Delete more meta-tags from imports The .lua file drops some more meta-information from imports that is of no need for rendering. This is based on the following unmerged PR to osm2pgsql: * osm2pgsql-dev/osm2pgsql#532
* Convert layer to integer and use it in queries * Use layer instead of z_order in queries where possible
Conflicts: openstreetmap-carto.style project.mml
Regarding old-style MPs I would be much in favour of that. I'm sure that the cleanup will progress to a point where this is no problem anymore. |
That's great news. I would also be strongly in favour of dropping support for old-style multipolygons. |
Remove support for old-style multipolygons
This is now ready. Before merging we want to
I'd also like to move religion to hstore, but if that doesn't get done it won't hold up the release. |
I squeezed some more disk space out of one of my servers, so it looks like I can do a test over the weekend, then we can proceed. |
https://lua.osm-carto.paulnorman.ca is running with the latest code, I'm looking to tag 3.3.0 and freeze in a couple days. |
Nice. Comparison is somewhat difficult since the style version is different in the standard map at the moment and the low zoom tiles are also very old there right now. Am i right to assume that this does not yet use libosmium for geometry building (osm2pgsql-dev/osm2pgsql#684)? |
Looks good! I had no time to analyse this yet, but here is a difference: |
No, it's using released osm2pgsql. |
Object in question is http://www.openstreetmap.org/way/77686091 with leisure=track, so this is an expected difference, and, as it happens, a tagging error. |
We might want to add a rendering rule though. We could render it equal to racetracks for now, so there is at least something. |
Yes, we should render leisure=track on lines but no, not like racetracks. The latter is for motor sports, the former for running sports, very different things we also render very differently. Ideally i think leisure=track on lines should be the same as on areas, i.e. line with thin casing. And preferably in a way that combines with areas, i.e. render casings below and fills above areas. |
Ok, i inspected the map a bit more and it seems to me most of the differences that are not either due to the style version difference or due to data changes are
http://www.openstreetmap.org/way/100827142 This water area tag is now interpreted while previously it was ignored because it was redundant with the relation tag so the islands in the river vanish. I expected this effect to be much more widespread, especially for water areas since in the past it had been common practice to simply add |
I guess the question is, do we want to hold up 3.3.0/4.0.0 to make any of these changes? |
I don't think so. If anyone wants to add rendering of And the faulty double tagging can be fixed as easily after the change than before - in fact if the change is rolled out the motivation for fixing is probably higher since it would fix a visible error while right now it would be an invisible change. |
As far as I'm concerned, no. |
I'll look at tagging 3.3.0 tonight, which will also involve a feature freeze until 4.0.0. |
A big “thanks” for making this working! |
Not yet ready for merge
What this does
This PR accomplishes four main tasks
Lua transforms
It switches to Lua transforms, which allows pre-processing of tags like layer. This allows SQL like
Instead of the longer
There are also more significant changes for z_order, but that can be done post-merge
Disjoint areas
It uses
-G
, so disjoint areas are imported as PostGIS MULTIPOLYGONs instead of multiple rows as POLYGONs. These are often found on administrative boundary relations where islands are disjoint.Tag columns
It uses
--hstore
, so tags that don't have a column end up in a specialtags
column. This allows for more flexability, and the number of columns has been significantly reducedMultipolygon handling
The multipolygon handling is in the transforms, so instead of the C transforms, we are responsible for defining the handling.
For the purposes of this PR and the transforms, a multipolygon is either new-style or old-style. A new-style multipolygon has a tag other than
type=multipolygon
on the relation, and tags on its members are ignored. An old-style multipolygon is one with only atype=multipolygon
and all the member ways have the same tags, or a member may be untagged. For technical reasons, deleted tags (e.g.source=*
) are ignored.There is also implicitly the definition of an invalid multipolygon, which is a type=multipolygon relation which doesn't meet the above criteria.
Some properties of MPs as defined like this are
Any new-style MP will never change to an old-style MP by only changing tags on its member ways, nor will it become invalid. Sudden changes to interpretation were possible with the C transform logic, where adding
building=yes
to a small object would set the building column to yes for a large object generated by a relation.Any old-style MP will never change to an new-style MP by only changing tags on its member ways, but may become invalid if the tags become conflicting
type=multipolygon relations are always areas
Processing mulitpolygons does not require any knowledge of tagging semantics like is required for area formation from polygons. For technical reasons, deleted keys are ignored, which does involve tagging semantics.
Area handling
We can now determine if a way corresponds to an area feature or a linear feature however we want, instead of just based on tag keys. We want to avoid the interpretation of a correctly mapped feature depending on if a way is closed or not.
Open issues
Add comments to
openstreetmap-carto.style
pointing to INSTALL.mdAdd something on using hstore to the code style guide (
tags->'foo'
instead oftags -> 'foo'
, etc) (Add SQL style notes on hstore #2552)The interim need to backport PRs against both master and 3.x needs documenting in CONTRIBUTING.md
Define more precisely how long we will maintain 3.x and 4.x compatibility
Better document MPs somewhere
Pre-merge changes
The changelog and README.md version information need changing
A new git branch for 3.x.x work needs creating
Post-merge changes
Mark database layout milestone as finished
Fixes #101, #1362, #1504
Preview
I have a demo at https://lua.osm-carto.paulnorman.ca/. It is not consuming OSM updates. Before reporting a bug please