Railway=tram no longer rendered when the way also has a highway tag

[matthijsmelissen]

Railway=tram is no longer rendered when the way also has a highway tag.

Due to the collapse in the SQL, we render ways that have both a railway tag and a highway tag only as highway. However, until #626, there was an exception for railway=tram, which was rendered even when tagged on a highway way.

Is a way with both a railway and a highway tag correct tagging? I suppose it is, because it is frequently used. In which cases should we render both the railway and the highway tag? The answer 'in all cases' would lead to a combinatorial explosion (number of railway types times number of highway types).

Reported on talk.

[matthijsmelissen]

Taginfo tells us that there are 9 047 objects with both railway=tram and highway=*, out of 68 539 objects with highway=tram.

There are 51 731 objects with both a railway and a highway tag. Out of these, 35 177 are railway=abandoned and 1 568 are railway=tram_stop. That leaves 5939 objects unaccounted for.

[daganzdaanda]

This is an example of a way with highway=service and railway=tram: https://www.openstreetmap.org/way/60112647 I guess it's possible to make two different ways for the road and the tram, see just south of the example. Not sure what is reflecting reality better.

[matthijsmelissen]

I don't like the railway=* and highway=* tagging on a single way very much, because it gets messy when adding additional tags: what do oneway, maxspeed, voltage etc mean on such ways? However, this is the existing scheme, so I guess we will have to support it. Any ideas on how to implement this in a computationally feasible way?

[gravitystorm]

Probably with some sort of UNION ALL e.g. select foo from planet_osm_line where highway in (bar) UNION ALL select foo from planet_osm_line where railway in (baz).

[matthijsmelissen]

Good idea, that would work. Although it would again make the queries more complex.

[kkofler]

Any ETA on this issue? Many cities are now missing some or (almost) all of their tramway rails in the default OSM rendering.

[matthijsmelissen]

I agree this is urgent. I don't have time to fix this right now, we can consider reverting the change that caused it.

[gravitystorm]

I'd rather not - it was #626 that caused this, and unpicking that and reopening all those other bugs would be a nightmare. I'd rather keep the merge and fix this.

[pnorman]

That leaves 5939 objects unaccounted for.

Value of railway tags for objects with a highway tag:

┌───────┬─────────────────────────────────────┐
│ count │              ?column?               │
├───────┼─────────────────────────────────────┤
│ 35242 │ abandoned                           │
│  8667 │ tram                                │
│  1537 │ platform                            │
│   551 │ dismantled                          │
│   526 │ disused                             │
│   415 │ rail                                │
│   329 │ razed                               │
│   138 │ proposed                            │
│    65 │ level_crossing                      │
│    55 │ tram_stop                           │
│    54 │ narrow_gauge                        │
│    39 │ historic                            │
│    39 │ crossing                            │
│    28 │ overbridge                          │
│    28 │ light_rail                          │
│    27 │ preserved                           │
│    25 │ adjacent                            │
│    22 │ subway                              │
│    21 │ abandoned|dismantled                │
│    20 │ construction                        │
│    19 │ abandoned_tram                      │
│    18 │ monorail                            │
│    15 │ subway_entrance                     │
│    12 │ obliterated                         │
│    10 │ no                                  │
│     7 │ spur                                │
│     7 │ yes                                 │
│     6 │ funicular                           │
│     6 │ y                                   │
│     5 │ station                             │
│     4 │ loading_ramp                        │
│     3 │ historical                          │
│     3 │ turntable                           │
│     3 │ abandonned                          │
│     3 │ disused; abandoned                  │
│     2 │ funicular_entrance                  │

[matthijsmelissen]

415 times railway=rail is probably not something to ignore although it's limited compared to the others. It was also never rendered because we only had an exception for railway=tram, so the number will probably increase once we start rendering it. A union is probably the way to go, even though it increases query complexity even more.

[kkofler]

So your line of code currently says this (and there are 2 similar ones): "table": "(select way,coalesce(('highway' || (case when substr(highway, length(highway)-3, 4) = 'link' then substr(highway,0,length(highway)-4) else highway end)), ('railway' ||(case when railway='preserved' and service in ('spur','siding','yard') then 'INT-preserved-ssy'::text when (railway='rail' and service in ('spur','siding','yard')) then 'INT-spur-siding-yard' else railway end)), ('aeroway_' || aeroway)) as feature, horse, foot, bicycle, tracktype, case when access in ('destination') then 'destination'::text when access in ('no', 'private') then 'no'::text else null end as access, construction, case when service in ('parking_aisle','drive-through','driveway') then 'INT-minor'::text else 'INT-normal'::text end as service, case when oneway in ('yes', '-1') and highway in ('motorway','motorway_link','trunk','trunk_link','primary','primary_link','secondary','secondary_link','tertiary','tertiary_link','residential','unclassified','road','service','pedestrian','raceway','living_street','construction') then oneway else null end as oneway, case when substr(highway, length(highway)-3, 4) = 'link' then 'yes' else 'no' end as link, case when layer is null then '0' else layer end as layernotnull from planet_osm_line join ( values ('railway_rail',430), ('railway_spur',430), ('railway_siding',430), ('railway_subway',420), ('railway_narrow_gauge',420), ('railway_light_rail',420), ('railway_preserved',420), ('railway_funicular',420), ('railway_monorail',420), ('railway_miniature',420), ('railway_turntable',420), ('railway_tram',410), ('railway_disused',400), ('railway_construction',400), ('highway_motorway',370), ('highway_trunk',360), ('highway_primary',350), ('highway_secondary',340), ('highway_tertiary',340), ('highway_residential',330), ('highway_unclassified',330), ('highway_road',330), ('highway_living_street',320), ('highway_pedestrian',310), ('highway_raceway',300), ('highway_motorway_link',240), ('highway_trunk_link',230), ('highway_primary_link',220), ('highway_secondary_link',210), ('highway_tertiary_link',200), ('highway_service',150), ('highway_track',110), ('highway_path',100), ('highway_footway',100), ('highway_bridleway',100), ('highway_cycleway',100), ('highway_steps',100), ('railway_platform',100), ('aeroway_runway',60), ('aeroway_taxiway',50), ('highway_proposed',20), ('highwayconstruction',10)) as ordertable (feature, prio) on coalesce(('highway' || planet_osmline.highway), ('railway' || planet_osmline.railway), ('aeroway' || planet_osm_line.aeroway)) = ordertable.feature where (tunnel is null or not tunnel in ('yes','building_passage')) and (covered is null or not covered='yes') and (bridge is null or not bridge in ('yes','true','1','viaduct')) order by ordertable.prio) as roads_fill",

I'd try changing that to: "table": "(select * from ((select way,coalesce(('highway' || (case when substr(highway, length(highway)-3, 4) = 'link' then substr(highway,0,length(highway)-4) else highway end)), ('aeroway' || aeroway)) as feature, horse, foot, bicycle, tracktype, case when access in ('destination') then 'destination'::text when access in ('no', 'private') then 'no'::text else null end as access, construction, case when service in ('parking_aisle','drive-through','driveway') then 'INT-minor'::text else 'INT-normal'::text end as service, case when oneway in ('yes', '-1') and highway in ('motorway','motorway_link','trunk','trunk_link','primary','primary_link','secondary','secondary_link','tertiary','tertiary_link','residential','unclassified','road','service','pedestrian','raceway','living_street','construction') then oneway else null end as oneway, case when substr(highway, length(highway)-3, 4) = 'link' then 'yes' else 'no' end as link, case when layer is null then '0' else layer end as layernotnull from planet_osm_line join ( values ('highway_motorway',370), ('highway_trunk',360), ('highway_primary',350), ('highway_secondary',340), ('highway_tertiary',340), ('highway_residential',330), ('highway_unclassified',330), ('highway_road',330), ('highway_living_street',320), ('highway_pedestrian',310), ('highway_raceway',300), ('highway_motorway_link',240), ('highway_trunk_link',230), ('highway_primary_link',220), ('highway_secondary_link',210), ('highway_tertiary_link',200), ('highway_service',150), ('highway_track',110), ('highway_path',100), ('highway_footway',100), ('highway_bridleway',100), ('highway_cycleway',100), ('highway_steps',100), ('aeroway_runway',60), ('aeroway_taxiway',50), ('highway_proposed',20), ('highwayconstruction',10)) as ordertable (feature, prio) on coalesce(('highway' || planet_osmline.highway), ('aeroway' || planet_osm_line.aeroway)) = ordertable.feature where (tunnel is null or not tunnel in ('yes','buildingpassage')) and (covered is null or not covered='yes') and (bridge is null or not bridge in ('yes','true','1','viaduct'))) union all (select way,('railway' ||(case when railway='preserved' and service in ('spur','siding','yard') then 'INT-preserved-ssy'::text when (railway='rail' and service in ('spur','siding','yard')) then 'INT-spur-siding-yard' else railway end)) as feature, horse, foot, bicycle, tracktype, case when access in ('destination') then 'destination'::text when access in ('no', 'private') then 'no'::text else null end as access, construction, case when service in ('parking_aisle','drive-through','driveway') then 'INT-minor'::text else 'INT-normal'::text end as service, null as oneway, 'no' as link, case when layer is null then '0' else layer end as layernotnull from planet_osm_line join ( values ('railway_rail',430), ('railway_spur',430), ('railway_siding',430), ('railway_subway',420), ('railway_narrow_gauge',420), ('railway_light_rail',420), ('railway_preserved',420), ('railway_funicular',420), ('railway_monorail',420), ('railway_miniature',420), ('railway_turntable',420), ('railway_tram',410), ('railway_disused',400), ('railway_construction',400), ('railwayplatform',100)) as ordertable (feature, prio) on ('railway' || planet_osm_line.railway) = ordertable.feature where (tunnel is null or not tunnel in ('yes','building_passage')) and (covered is null or not covered='yes') and (bridge is null or not bridge in ('yes','true','1','viaduct')))) order by prio) as roads_fill", and likewise for the other 2 cases. (What I did is that I put all the highway and aeroway stuff in one select, all the railway stuff in the other and did a union all across both. The outer select just does the ordering. The where clause could be moved to the outer select to avoid the copy&paste, but I suspect that to decrease performance, it would have to be tried.)

Of course, I haven't tested this at all. I'm a programmer with some SQL knowledge, but I'm not familiar with your code and don't have a testing setup for it.

[pnorman]

So your line of code currently says this

The mml file is JSON, so you really need to be careful with converting newlines, quotes, and backslashes. I don't believe the roads query is all as one line.

[matthijsmelissen]

@kkofler Thanks for your help. I get:

Postgis Plugin: ERROR:  subquery in FROM must have an alias
LINE 1: SELECT * FROM (select * from ((select way,coalesce(('highway...
                                     ^
HINT:  For example, FROM (SELECT ...) [AS] foo.

I have no time now to do extensive debugging, do you know where I should add the 'AS blabla' in the query? Possibly a MySQL / PostgreSQL dialect difference?

[kkofler]

Between "))))" and "order by prio". (And I think it's not a dialect difference, but just me not having enough SQL practice to get such details right. :-) )

Don't forget that the queries for the bridges and the tunnels need the same changes. (I think you can just paste my query and replace the 2 where clauses with copies of the where clause that was originally there.)

[matthijsmelissen]

Don't forget that the queries for the bridges and the tunnels need the same changes.

And maybe roads_casing as well for consistency.

(I think you can just paste my query and replace the 2 where clauses with copies of the where clause that was originally there.)

The ORDER BY are also different, but apart from that I think they should be the same.

[woodpeck]

Would it be totally over the top to start working with views for such situations? @pnorman will probably be able to say more but in my experience, the performance hit is negligible. And while having to create views before one can use the style might seem cumbersome, having the aforementioned SQL in three or four different places would certainly not make the whole thing more accessible to newcomers. ("Duh, is this the same query here or is it slightly different...?)

[pnorman]

@pnorman will probably be able to say more but in my experience, the performance hit is negligible.

Afaik only hit is a very minor one in the stage which converts the query into a representation that can be planned from. The issue is it we then need to deal with database schema migrations.

[matthijsmelissen]

Nearly, we also needed to add prio in the select part of the two subqueries. Now I think it works, but I still need to merge the recent changes in.

[Cotta-R]

It's bad, that rendering of tramlines was broken. It's worse, that this isn't fixed for long time now. This bug affects openstreetmap really big. Priority=Blocker!

[gravitystorm]

@Cotta-R Your comment is unnecessary and is not helpful either.

[kkofler]

There is a pull request now, what is it waiting for? I can understand Cotta-R's frustration somehow, considering that the regression has been ongoing for 2 weeks now. And I already did what I could to help get this fixed.

[systemed]

There has been a pull request for just three days and one that has some performance issues (see the comments on #894), so it's not an unequivocal easy merge. Regardless, if you think that project maintainers should commit to an SLA that requires them to fix everything within three days, especially during a popular holiday period, then you're in the wrong project.

[matthijsmelissen]

Hmm. We are a volunteering project, but that doesn't mean we shouldn't aim for quality. I would welcome any suggestions on how to minimize regressions (and the time before they are resolved) in the future - taking into account the limited personal resources we have.

[kkofler]

There's a performance decrease of only 0.38%, and pnorman, who measured it, also wrote that he thinks it's probably worth it. It was expected that the more complex query to get this right would take longer. That doesn't mean it isn't the right thing to do. I also don't see any benefit in waiting, I don't think we can realistically speed this up significantly without reintroducing the bug, and even if we could, the speedup can always be applied later.

[kkofler]

Hmmm, I still don't see the tram rails getting rendered in the affected places, are we sure the fix works? Or did the change not go live on osm.org yet?

[HolgerJeromin]

release is not active right now: https://lists.openstreetmap.org/pipermail/dev/2014-August/028043.html

[kkofler]

The fix finally got deployed yesterday: https://lists.openstreetmap.org/pipermail/dev/2014-September/028048.html and it seems to be working. (Not everything got repainted yet, but what did looks fine now.)