Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Unable to use in expression as layer filter #16262

Closed
1ec5 opened this issue Mar 3, 2020 · 3 comments · Fixed by #16272
Closed

Unable to use in expression as layer filter #16262

1ec5 opened this issue Mar 3, 2020 · 3 comments · Fixed by #16272
Assignees
Labels
bug Core The cross-platform C++ core, aka mbgl expressions

Comments

@1ec5
Copy link
Contributor

1ec5 commented Mar 3, 2020

The in expression operator added in #16162 can only be used with paint and layout properties, not with layer filters. To reproduce, load this style and observe that nothing is visible:

colors.json
{
  "version": 8,
  "zoom": 0,
  "center": [0, 0],
  "sources": {
    "quadrants": {
      "type": "geojson",
      "data": {
        "type": "FeatureCollection",
        "features": [
          {
            "type": "Feature",
            "properties": {
              "name": "ABC",
              "color": "red"
            },
            "geometry": {
              "type": "Polygon",
              "coordinates": [[
                [-180, -90],
                [-180, 0],
                [0, 0],
                [0, -90],
                [-180, -90]
              ]]
            }
          },
          {
            "type": "Feature",
            "properties": {
              "name": "DEF",
              "color": "#00ff00"
            },
            "geometry": {
              "type": "Polygon",
              "coordinates": [[
                [-180, 90],
                [-180, 0],
                [0, 0],
                [0, 90],
                [-180, 90]
              ]]
            }
          },
          {
            "type": "Feature",
            "properties": {
              "name": "GHI",
              "color": "#0000ff"
            },
            "geometry": {
              "type": "Polygon",
              "coordinates": [[
                [180, -90],
                [180, 0],
                [0, 0],
                [0, -90],
                [180, -90]
              ]]
            }
          },
          {
            "type": "Feature",
            "properties": {
              "name": "GHI",
              "color": "#ffff00"
            },
            "geometry": {
              "type": "Polygon",
              "coordinates": [[
                [180, 90],
                [180, 0],
                [0, 0],
                [0, 90],
                [180, 90]
              ]]
            }
          }
        ]
      }
    }
  },
  "glyphs": "local://glyphs/{fontstack}/{range}.pbf",
  "layers": [
    {
      "id": "colors",
      "type": "fill",
      "source": "quadrants",
      "filter": ["in", "#", ["get", "color"]],
      "paint": {
        "fill-color": ["get", "color"]
      }
    }
  ]
}
GL JS v1.8.1 Expected GL Native v1.3.0
gl-js expected actual

mbgl::style::conversion::isExpression() always returns false for the filter ["in", "#", ["get", "color"]], because in is also a legacy filter operator:

} else if (*op == "in" || *op == "!in" || *op == "!has" || *op == "none") {
return false;

/ref mapbox/mapbox-gl-native-ios#183 (comment)
/cc @jmkiley @Chaoba @chloekraw

@1ec5
Copy link
Contributor Author

1ec5 commented Mar 4, 2020

mbgl::style::conversion::isExpression() does have some code to distinguish some expression operators from identically named filters, but in with two parameters might not be possible to resolve through static type checking. For example, ["in", "a", "abc"] has two equally valid interpretations:

  • Filter: Is the a property equal to “abc”?
  • Expression: Is the string “a” a substring of “abc”?

To avoid breaking backwards compatibility with existing styles, we’d have to interpret it as a filter, but that would prevent us from fixing this issue and fully supporting mapbox/mapbox-gl-js#4113.

This is a contrived case, to be sure: it’s more likely for the filter to be written ["==", "a", "abc"], and there’s no practical reason for someone to test whether a literal string is in a literal string. But nothing would’ve stopped someone from using in instead of == back before Studio launched expression support. And isExpression() relies on static type checking, which means it doesn’t know whether a is a string literal or a reference to a feature property.

@1ec5
Copy link
Contributor Author

1ec5 commented Mar 4, 2020

GL JS has code to avoid the original issue above, but it doesn’t quite address the ambiguity: mapbox/mapbox-gl-js#9373.

@kkaefer
Copy link
Member

kkaefer commented Mar 4, 2020

JS PR is in mapbox/mapbox-gl-js#9374

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl expressions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants