Skip to content
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

Order amenity_lines by layer #3420

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

johsin18
Copy link
Contributor

This way, we respect the layer tag for water slides, and it should not harm for fords.

Fixes #3413

Changes proposed in this pull request:
Order amenity_lines by layer

Test rendering with links to the example places:
https://www.openstreetmap.org/#map=19/48.51073/9.03904

Before
screenshot_20180929_145854

After
screenshot_20180929_145912

This way, we respect the layer tag for water slides, and it should not harm for fords.
@kocio-pl
Copy link
Collaborator

Looks very good and simple, should not do any harm for both current features.

@kocio-pl kocio-pl changed the title Order amenity_lines by layer (#3413). Order amenity_lines by layer Sep 29, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Oct 1, 2018

Interesting - my local rendering from the current version works as expected without this fix. Could anyone else check it?

I guess however that sorting should be safe anyway. At least it does not break my local rendering.

@johsin18
Copy link
Contributor Author

johsin18 commented Oct 2, 2018

Without the explicit ordering, the DB will return the segments in an undefined order. So with only two segments, you still have a 50% chance to get the right result. With 3 segments, the probablity goes down to 16%, however, it might not really be random, but depend on the points in time the segments were added.
Try this one with 3 segments:
https://www.openstreetmap.org/#map=19/48.52361/8.08900&layers=N
Or change the code to "ORDER BY COALESCE(layer,0) DESC" to break it for sure.

@kocio-pl kocio-pl merged commit be01b48 into gravitystorm:master Oct 3, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Oct 3, 2018

Thanks! With DESC it was inverted, but without it it worked as it should. I don't know why my setup works different than OSMF servers, but that should be OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants