Bug in parsing of nested integer selectors

[matthijsmelissen]

Consider a project.mml consisting of a single layer with the following SQL query:
(select way, name, 1 as int from planet_osm_polygon) as data

Now compare the following two code samples:

[int < 22222] {
  text-name: "[name]";
  text-face-name: "DejaVu Sans Book";
  text-size: 12;
  [int > 2223] { text-size: 13; }
}

[int < 22222] {
  text-name: "[name]";
  text-face-name: "DejaVu Sans Book";
  text-size: 12;
  [int > 2222] { text-size: 13; }
}

The examples only differ in the second value, which is 2222 versus 2223.

The first sample gives an error: style.mss:5:24 Property text-name required for defining text styles. The second sample parses fines.

More in general, it seems that an error is generated whenever the second value has one digit less than the first value, and the second value is larger than the first value of which the last digit is truncated.

[springmeyer]

Wow, odd bug! Thanks for the report.

[pnorman]

Does this trigger it?

[int < 22222.0] {
  text-name: "[name]";
  text-face-name: "DejaVu Sans Book";
  text-size: 12;
  [int > 2223.0] { text-size: 13; }
}

gravitystorm/openstreetmap-carto#703 (comment) implies yes, but I thought I'd move the discussion over here and make sure.

[tmcw]

Smells like zoom levels are being compared as strings: http://mistakes.io/#aadc1497d5f06fe14392

[pnorman]

Smells like zoom levels are being compared as strings: http://mistakes.io/#aadc1497d5f06fe14392

zoom levels?

[matthijsmelissen]

Does this trigger it?

It does.

[matthijsmelissen]

Smells like zoom levels are being compared as strings: http://mistakes.io/#aadc1497d5f06fe14392

Good guess, that indeed seems to match the behaviour I noticed.

[matthijsmelissen]

I think this might be not that easy to solve (and perhaps not even a bug), because SQL results are not typed as far as I know. So in a [field1 < field2] selector, there is in general no way to know whether string comparison or integer/float comparison should be done. Assuming float comparison as default might be the best solution.

[gravitystorm]

SQL results are typed - see e.g. https://github.com/mapnik/mapnik/blob/v2.2.0/plugins/input/postgis/postgis_featureset.cpp#L145 where the postgis input plugin handles the different attribute types returned by a query.

I've used nik2img to confirm that it's a carto bug and that mapnik handle both cases fine - also, carto produces XML containing integers not strings so while the carto output is correct, something is failing in the middle.

[matthijsmelissen]

SQL results are typed

Thanks, didn't know that.

[matthijsmelissen]

This bug is still blocking several important improvements in openstreetmap-carto, such as gravitystorm/openstreetmap-carto#1028. With the risk of stating the obvious, it would be great if somebody could try to fix this issue!

[nebulon42]

@math1985 I tried to reproduce it (current master), but was not able to. I created a test.mss file and fed it directly into carto. I guess whether there is a MML file should make no difference. Output was the following:

<Style name="style" filter-mode="first">
  <Rule>
    <Filter>([int] &lt; 22222) and ([int] &gt; 2223)</Filter>
    <TextSymbolizer size="13" face-name="DejaVu Sans Book" ><![CDATA[[name]]]></TextSymbolizer>
  </Rule>
  <Rule>
    <Filter>([int] &lt; 22222)</Filter>
    <TextSymbolizer face-name="DejaVu Sans Book" size="12" ><![CDATA[[name]]]></TextSymbolizer>
  </Rule>
</Style>

Could you please verify if it still fails for you?

[matthijsmelissen]

Confirmed that this is fixed by ad39e0b.