Database-reload #2128

matthijsmelissen · 2016-05-16T00:51:24Z

This is a pull-request against the lua branch

This is a new version of #2105. No changes except for dropping some commented lines.

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,
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:

Add more tags from imports to list of deleted tags osm2pgsql-dev/osm2pgsql#532

matthijsmelissen · 2016-05-16T00:52:47Z

@gravitystorm Would you be able to do a test-run of the import on one of your servers? Doesn't need to be full-planet, but it would be good to have done at least some testing.

pnorman · 2016-05-16T02:08:05Z

Layer should be an integer.

👎 to having two z_orders, 👎 to trying to maintain compatibility

pnorman · 2016-05-16T02:12:25Z

openstreetmap-carto.lua

+   end
+
+   -- Filter out objects with 0 tags
+   if numberofkeys == 0 then


Move this branch first as it's a shorter exit and very common

I don't think we can do this, as line 257-267 might change the numberofkeys variable? Objects with for example only a note of fixme tag are quite common. Or would you be ok with leaving some objects with 0 tags in the database?

the numberofkeys == 0 case is so common it needs to be handled first, even if another check is done at the end

pnorman · 2016-05-16T02:32:23Z

The area determination logic is duplicated, put it in its own function

pnorman · 2016-05-16T03:44:40Z

Can you edit the git commit message to have a more useful first line? This is a major change, so we'll be seeing it a lot in git blame and other commands.

git commit --amend, edit the message, and git push -f should do it

@pnorman

**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) gravitystorm#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 gravitystorm#1362, resolves gravitystorm#137, resolves gravitystorm#268, and resolves gravitystorm#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 gravitystorm#59. # Import ele on buildings This resolves gravitystorm#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

matthijsmelissen · 2016-05-16T23:04:27Z

Can you edit the git commit message to have a more useful first line?

Done.

I will come back to your other points later.

pnorman · 2016-05-16T23:10:44Z

openstreetmap-carto.lua

+   { 'bridge', 1, 10, 0, 0 },
+   { 'tunnel', 'yes', -10, 0, 0},
+   { 'tunnel', 'true', -10, 0, 0},
+   { 'tunnel', 1, -10, 0, 0},


these lines have an extra leading space

matthijsmelissen · 2016-05-17T21:32:05Z

The area determination logic is duplicated, put it in its own function

Fixed

…ua file

matthijsmelissen · 2016-05-17T23:32:38Z

the z_order you have in this table is not compatible with osm2pgsql z_order.

We specify in the documentation that osm2pgsql should be run with openstreetmap-carto.style, so if the OSMF tile server maintainers decide to use a different .style, in a way that's their problem. In practice of course they are our main 'customer', so it's not something we can ignore. I wonder what other tile servers that run openstreetmap-carto do - use our style or use the osm2pgsql default one? For context, both styles were identical until osm2pgsql diverged a couple of months ago.

Basically we have three options:

1) Drop the column z_order altogether (we still keep osmcarto_z_order)
Positive: lower complexity (less chance for confusion due to the presence of two similar columns).
Negative: no backwards compatibility with older versions of the stylesheet; harder to share a database with other styles that use the z_order column such as openstreetmap-de and openstreetmap-fr.

2) Import z_order as currently defined by osm2pgsql and used on the OSMF servers (in addition to osmcarto_z_order)
Positive: Backwards compatibility (largely) with older versions of the stylesheet. Easy to share a database with other styles that use the default z_order column from osm2pgsql, and even (accepting some small changes in behaviour) with styles that use z_order column from openstreetmap-carto.style.
Negative: We might lose backwards compatibility anyway if we eventually decide to drop columns for which we will use hstore.

3) Import z_order as currently defined by openstreetmap-carto.style (in addition to osmcarto_z_order)
Positive: Backwards compatibility with older versions of the stylesheet. Easy to share a database with other styles that use the default z_order column from openstreetmap-carto.style, and (accepting some small changes in behaviour) with styles that use z_order column from osm2pgsql.
Negative: Same as for 2.

Feel free to extend.

@gravitystorm Would you be able to make the final decision? The current proposal implements 3), but on changing the proposal to 1) or 2) is trivial.

pnorman · 2016-05-18T01:32:50Z

the z_order you have in this table is not compatible with osm2pgsql z_order.

We specify in the documentation that osm2pgsql should be run with openstreetmap-carto.style, so if the OSMF tile server maintainers decide to use a different .style, in a way that's their problem. In practice of course they are our main 'customer', so it's not something we can ignore. I wonder what other tile servers that run openstreetmap-carto do - use our style or use the osm2pgsql default one? For context, both styles were identical until osm2pgsql diverged a couple of months ago.

The styles are still identical with respect to z_order - and z_order isn't defined in the .style file anyways.

pnorman · 2016-06-02T09:18:57Z

With the C transforms and osm2pgsql's style.lua example I have 28662 rows in planet_osm_roads with the extract I'm testing with, but 1667987 with this code, very close to the number of lines. Could 1 and 0 be flipped in what you've got? This would also account for some of the speed difference we found, since it has to do a lot more DB work

pnorman · 2016-06-15T09:25:15Z

openstreetmap-carto.lua

+   {'railway', 'miniature', 0, 420, 1},
+   {'railway', 'turntable', 0, 420, 1},
+   {'railway', 'tram', 0, 410, 1},
+   {'railway', 'tram-service', 0, 405, 1},


does this line work?

No - not sure now where it comes from actually.

… into lua_changes

…p-carto into lua_changes

pnorman · 2016-06-19T09:11:52Z

ping @math1985, I have two open PRs against this branch.

matthijsmelissen · 2016-06-19T09:39:12Z

I'm currently reviewing them.

matthijsmelissen · 2016-06-25T20:21:38Z

@pnorman's PR's have been merged.

From my side this is (again) ready for review/merge.

pnorman · 2016-06-25T23:14:23Z

From my side this is (again) ready for review/merge.

Have you fixed the problem with the roads table?

Also, this is failing CI

matthijsmelissen · 2016-06-26T00:34:48Z

Have you fixed the problem with the roads table?

I must have missed that, what problem are you referring to?

Also, this is failing CI

It failed because of 'The command "./scripts/travis_check_project_files" exited with 1.', that's a new check as far as I know, so I think this should fix itself when merging.

pnorman · 2016-06-26T04:42:12Z

Have you fixed the problem with the roads table?
I must have missed that, what problem are you referring to?

#2128 (comment)

It failed because of 'The command "./scripts/travis_check_project_files" exited with 1.', that's a new check as far as I know, so I think this should fix itself when merging.

It looks like you didn't run yaml2mml.py

matthijsmelissen · 2016-06-26T22:55:49Z

With the C transforms and osm2pgsql's style.lua example I have 28662 rows in planet_osm_roads with the extract I'm testing with, but 1667987 with this code, very close to the number of lines

I cannot reproduce this.

For Luxembourg, I get these numbers (old/new):
planet_osm_nodes 1572472/1572472
planet_osm_ways 201180/201180
planet_osm_rels 3804/3804

planet_osm_point 45338/48484
planet_osm_line 84918/85127
planet_osm_polygon 115719/113992
planet_osm_roads 13481/13056

@pnorman Can you reproduce this problem with the Luxembourg abstract?

matthijsmelissen · 2016-06-26T23:11:45Z

It looks like you didn't run yaml2mml.py

Fixed.

pnorman · 2016-06-27T05:40:33Z

@pnorman Can you reproduce this problem with the Luxembourg abstract?

I'm having trouble reproducing, I'll try further debugging

matthijsmelissen changed the title ~~**This is a pull-request against the lua branch**~~ Database-reload May 16, 2016

matthijsmelissen mentioned this pull request May 16, 2016

Database-reload #2127

Closed

matthijsmelissen mentioned this pull request May 16, 2016

Database-reload #2105

Closed

pnorman reviewed May 16, 2016
View reviewed changes

matthijsmelissen force-pushed the lua_changes branch from d767022 to d48106e Compare May 16, 2016 23:04

pnorman reviewed May 16, 2016
View reviewed changes

matthijsmelissen added 2 commits May 17, 2016 20:06

Add highway=rest_area to list of tags treated as polygon

e0aa01d

Merge duplicate code to determine if object is polygon

1ec1cfc

matthijsmelissen added 4 commits May 17, 2016 23:39

Explicitly make variables in .lua script local

02edb0a

Whitespace in .lua file

e5660e6

Move filtering out objects with 0 tags to beginning of function in .l…

d524487

…ua file

Code simplification

d276c90

Typo

1b82cb6

pnorman added 2 commits June 2, 2016 01:48

Name polygon function isarea

659840c

Use local vars for polygon stuff

10bff06

pnorman mentioned this pull request Jun 14, 2016

Rendering of valleys, ridges and other mountain elements #2138

Closed

Search through possible polygon tags

5175195

pnorman reviewed Jun 15, 2016
View reviewed changes

pnorman and others added 5 commits June 15, 2016 18:04

Refactor lua z_order and roads code

be2eb57

Add testing framework

4499350

Merge branch 'isarea' of https://github.com/pnorman/openstreetmap-carto…

3621bd7

… into lua_changes

Merge branch 'lua_z_order' of https://github.com/pnorman/openstreetma…

dc36874

…p-carto into lua_changes

Use spaces, not tabs

f2030f2

matthijsmelissen closed this Jun 26, 2016

matthijsmelissen reopened this Jun 26, 2016

Regenerate .mml

2ec88f3

matkoniecz mentioned this pull request Jun 30, 2016

Add rendering for boundary=protected_area #603

Closed

pnorman unassigned matthijsmelissen Jun 30, 2016

pnorman merged commit 2ec88f3 into gravitystorm:lua Jul 1, 2016

matthijsmelissen deleted the lua_changes branch August 5, 2016 08:33

HolgerJeromin mentioned this pull request Feb 26, 2018

natural=cliff is ignored on closed ways #3084

Closed

imagico mentioned this pull request Jun 16, 2024

Flex version of the osm2pgsql configuration #4978

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Database-reload #2128

Database-reload #2128

matthijsmelissen commented May 16, 2016 •

edited

Loading

matthijsmelissen commented May 16, 2016

pnorman commented May 16, 2016

pnorman May 16, 2016

matthijsmelissen May 16, 2016 •

edited

Loading

pnorman May 16, 2016

matthijsmelissen May 17, 2016

pnorman commented May 16, 2016

pnorman commented May 16, 2016

matthijsmelissen commented May 16, 2016

pnorman May 16, 2016

matthijsmelissen May 17, 2016

matthijsmelissen commented May 17, 2016

matthijsmelissen commented May 17, 2016 •

edited

Loading

pnorman commented May 18, 2016 •

edited

Loading

pnorman commented Jun 2, 2016

pnorman Jun 15, 2016

matthijsmelissen Jun 15, 2016

pnorman commented Jun 19, 2016

matthijsmelissen commented Jun 19, 2016

matthijsmelissen commented Jun 25, 2016

pnorman commented Jun 25, 2016 •

edited

Loading

matthijsmelissen commented Jun 26, 2016

pnorman commented Jun 26, 2016

matthijsmelissen commented Jun 26, 2016 •

edited

Loading

matthijsmelissen commented Jun 26, 2016

pnorman commented Jun 27, 2016

Database-reload #2128

Database-reload #2128

Conversation

matthijsmelissen commented May 16, 2016 • edited Loading

Hstore

Make polygon/linestring decision based on value

Rendering order

Recommend -G flag for multipolygons

Import ele on buildings

Delete more meta-tags from imports

matthijsmelissen commented May 16, 2016

pnorman commented May 16, 2016

pnorman May 16, 2016

Choose a reason for hiding this comment

matthijsmelissen May 16, 2016 • edited Loading

Choose a reason for hiding this comment

pnorman May 16, 2016

Choose a reason for hiding this comment

matthijsmelissen May 17, 2016

Choose a reason for hiding this comment

pnorman commented May 16, 2016

pnorman commented May 16, 2016

matthijsmelissen commented May 16, 2016

pnorman May 16, 2016

Choose a reason for hiding this comment

matthijsmelissen May 17, 2016

Choose a reason for hiding this comment

matthijsmelissen commented May 17, 2016

matthijsmelissen commented May 17, 2016 • edited Loading

pnorman commented May 18, 2016 • edited Loading

pnorman commented Jun 2, 2016

pnorman Jun 15, 2016

Choose a reason for hiding this comment

matthijsmelissen Jun 15, 2016

Choose a reason for hiding this comment

pnorman commented Jun 19, 2016

matthijsmelissen commented Jun 19, 2016

matthijsmelissen commented Jun 25, 2016

pnorman commented Jun 25, 2016 • edited Loading

matthijsmelissen commented Jun 26, 2016

pnorman commented Jun 26, 2016

matthijsmelissen commented Jun 26, 2016 • edited Loading

matthijsmelissen commented Jun 26, 2016

pnorman commented Jun 27, 2016

matthijsmelissen commented May 16, 2016 •

edited

Loading

matthijsmelissen May 16, 2016 •

edited

Loading

matthijsmelissen commented May 17, 2016 •

edited

Loading

pnorman commented May 18, 2016 •

edited

Loading

pnorman commented Jun 25, 2016 •

edited

Loading

matthijsmelissen commented Jun 26, 2016 •

edited

Loading