Move plaque rendering to higher zoom levels

[kocio-pl]

Related to #1560.

With hstore we can finally push such small memorial objects as plaques to other small amenities. In my city there are so many of them, that I skip mapping them to not clutter the map.

In this code I've used icon from osmic, because it's depicting plaque.

Warsaw
z18
Before

After

z19
Before

After

[kocio-pl]

What other developers think of this change? This code probably needs at least one formal review to be merged.

[boothym]

Agree with the changes. Might be worth also adding blue_plaque to the code as well, only 122 uses but they should be the same as plaque.

[pnorman]

I'm worried that adding another column to the amenity layers will make the combinatorial explosion worse. Can we use the feature column for this?

[kocio-pl]

OK, I will test it this way.

[kocio-pl]

Hm, I'm not sure how to write proper SQL for this. I guess this line:

'historic_' || CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') THEN historic ELSE NULL END,

should be edited so we select non-plaque memorials as feature_historic_memorial and memorials with memorial=plaque tag as feature_historic_plaque. Could you hint me how to do it?

[pnorman]

What comes to mind is

'historic_' 
  || CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') THEN historic ELSE NULL END 
  || CASE WHEN historic = 'plaque' THEN '_plaque' END

[kocio-pl]

There's no such tagging as historic = 'plaque'(it's memorial=plaque and memorial tag is in hstore).

It's hard for me to understand how SQL works here. When I take this line:

'historic_' || 
CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') THEN historic ELSE NULL END,

and add plaque indicator:

'historic_' || 
CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') THEN historic ELSE NULL END || 
CASE WHEN tags->'memorial' IN ('plaque') THEN '_plaque' ELSE NULL END,

all the objects tagged as historic=memorial + memorial=plaque are seen as feature = 'historic_memorial_plaque' (which is OK), but all the memorials without memorial=plaque are gone (so they are not seen as feature = 'historic_memorial', which is strange).

It looks like "memorial" part is indicated inside "historic_memorial_plaque", but not within "historic_memorial" features. Even when I make this part explicit:

CASE WHEN historic IN ('memorial') THEN 'memorial' ELSE NULL END

memorials without plaque are still not seen.

What could be the problem? Maybe some other line in project.mml makes a difference?

[matthijsmelissen]

I'm not sure if it solves your problem, but

CASE WHEN tags->'memorial' IN ('plaque') THEN '_plaque' ELSE NULL END,

should be

CASE WHEN tags->'memorial' IN ('plaque') THEN '_plaque' ELSE '' END,

In SQL, we have 'foo'||NULL equals NULL while 'foo'||'' equals 'foo'.

[kocio-pl]

Now it works - thanks a lot!

[pnorman]

Ah, || doesn't ignore nulls, so 'foo' || NULL evaluates to NULL.

It helps to write down what we need out of this expression.

The entire historic expression needs to evaluate to null if it doesn't match one of the historic tags for the outer COALESCE to work.

The new properties we need are
historic=foo memorial=plaque objects evaluate to NULL
historic=memorial memorial=foo objects evaluate to historic_memorial
historic=memorial memorial=plaque objects evaluate to historic_memorial_plaque

I'm not concerned about historic=archaeological_site memorial=plaque in order to make the expression simpler.

'historic_' ||  -- feature prefix, || is needed so the entire expression can go to NULL
  CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') -- check for one of the
                                                                        -- historic keys
    THEN concat_ws('_', -- This is the start of another expression. concat_ws makes it easier to
                        -- handle _ separated text
      historic, -- start with the value of historic, which we know to be not NULL
      CASE WHEN tags->'memorial' IN ('plaque') -- For a single value tags @> 'memorial=>plaque' is
                                               -- better, but this allows consistent formatting
                                               -- with multi sub-tag cases
        THEN tags->'memorial' -- always plaque for this case, but allows for more general usage
        ELSE NULL -- an explicit null. This isn't required, but is consistent with the other CASEs
      END -- end memorial tag handling
      ) -- end the concat_ws
    ELSE NULL -- another explicit null
  END -- end overall case

[kocio-pl]

Thanks for making all the details clear, this way I can learn a bit about how we use SQL.

Does this formatting follow our SQL style guidelines and would be acceptable for you?:

'historic_' || CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') 
               THEN concat_ws('_', historic, CASE WHEN tags->'memorial' IN ('plaque') THEN tags->'memorial' ELSE NULL END) 
               ELSE NULL END

[kocio-pl]

I have updated the code and it works.

BTW - should I also add something like:

OR tags->'memorial' IN ('plaque')

This comment on the top of the section reads:

-- The upcoming where clause is needed for performance only, as the CASE statements would end up doing the equivalent filtering

and I'm not sure if any new tags included in SQL query should be also placed here or not.