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

Complex filters #407

Closed
mourner opened this issue Jun 3, 2014 · 14 comments
Closed

Complex filters #407

mourner opened this issue Jun 3, 2014 · 14 comments
Assignees
Milestone

Comments

@mourner
Copy link
Member

mourner commented Jun 3, 2014

Example from v0 style for native:

"source": "outdoors",
"layer": "poi_label",
"filter": [
  "and",
  { "field": "scalerank", "value": 4 },
  { "field": "localrank", "operator": "<=", "value": 16 },
  { "field": "localrank", "operator": ">", "value": 4 }
]

For the new style format, I suggest implementing the following syntax:

"filter": { // "and" at the top level
  "source": "outdoors",
  "layer": "poi_label",
  "scalerank": 4,
  ">": {"localrank": 4},
  "<=": {"localrank": 16}
}

Nested filters would look like this:

"filter": {
  "$or": {
    "maki": "park",
    "$and": {
        "foo": "bar",
        "$not": {"maki": "airport"}
    },
    "$match": {"maki": ".*"}
  },
  "bar": 5
}

Or with &&, ||, !, =~ instead of $and, $or, $not, $match.

It looks pretty clear, easy to read, and very compact for most cases. Consider that we're optimizing for common filters that are usually used in a style, not super-complex tree-like filters, because they are uncommon (but should still be possible).

@ansis @kkaefer @edenh @nickidlugash @incanus thoughts? Related discussions: #356

@mourner mourner self-assigned this Jun 3, 2014
@ansis
Copy link
Contributor

ansis commented Jun 3, 2014

Looks good! I'm seeing a couple possible issues

or

With the suggested $or: { ... } you can't do an or on multiple values for the same key. For example, checking if class is either motorway or main. These are very common in current styles.

Instead we could do:

{
    "$or": [
        { "class": "motorway" },
        { "class": "motorway_link" } ] }

and possibly have a $in operator to make this shorter

{
    "class": { "$in": [ "motorway", "motorway_link" ] }
}

magic keywords

The magic keywords source, layer and type aren't clearly special, and reserving them make it impossible for these words to be used as feature keys. type is currently used as a key in mapbox streets, and seems like something that would also be commonly used in custom data

We could prefix them: $source, $layer, $type. I'm not a huge fan of having $$$$$ everywhere, but it does nicely distinguish them and I can't think of anything better.


The suggested filter format, especially with these changes, is very similar to mongodb's comparison and logical operators. Should we follow @tmcw's suggestion and just full adopt those?

guide and list of comparison and logical operators

The remaining differences are mostly renames.

@mourner
Copy link
Member Author

mourner commented Jun 3, 2014

@ansis it would be an extension of the current syntax, so one field with multiple values is specified simply like this:

"class": ["motorway", "motorway_link"]

So if value is an array, it's an "or" relation.

We could fully adopt the Mongo syntax, but it wouldn't look as compact I think, and we would be more limited in how we can customize it specifically for GL styles.

@mourner
Copy link
Member Author

mourner commented Jun 3, 2014

We could also add some more syntactic sugar and allow multiple operators per field like this:

"localrank": {">": 4, "<=": 16}

@kkaefer
Copy link
Member

kkaefer commented Jun 4, 2014

"filter": { // "and" at the top level
  "source": "outdoors",
  "layer": "poi_label",
  "scalerank": 4,
  ">": {"localrank": 4},
  "<=": {"localrank": 16}
}

is problematic because it'd make it impossible to select the attributes source and layers because they are, magic words; agreeing with @ansis here.

I think we should go with MongoDB's query syntax; it looks pretty concise. Instead of having the source and layer properties in the filter object, we should just revert to having them at the top level object. What was the reason to move them to the filter object?

A modified MongoDB syntax would look like this:

{
    "scalerank": 4,
    "localrank": { "<=": 16, ">": 4 }
}

and the original MongoDB syntax is

{
    "scalerank": 4,
    "localrank": { "$lte": 16, "$gt": 4 }
}

@mourner
Copy link
Member Author

mourner commented Jun 4, 2014

I agree we can go with MongoDB-style, but with some more differences. E.g. I'm convinced we need to keep the current "field": ["foo", "bar"] logic (OR) instead of having MongoDB-like exact matching of an array.

Instead of having the source and layer properties in the filter object, we should just revert to having them at the top level object. What was the reason to move them to the filter object?

Because it is a part of a filter semantically. Having it at the top of the layer object is confusing for users.

@tmcw
Copy link
Contributor

tmcw commented Jun 4, 2014

E.g. I'm convinced we need to keep the current "field": ["foo", "bar"] logic (OR) instead of having MongoDB-like exact matching of an array.

I think what you're looking for is the $in query

// currently
{ "field": ["foo", "bar"] }
// mongo
{ "field": { "$in": ["foo", "bar"] } }

@kkaefer
Copy link
Member

kkaefer commented Jun 4, 2014

I think @mourner is proposing making $in the default and instead using something like $and to designate the mongodb semantic.

@mourner
Copy link
Member Author

mourner commented Jun 4, 2014

Yes. I think we don't need exact array matching at all (although we can have a special operator for it), just checking that a field contains each of the multiple fields, e.g.:

{"food": {"$has": ["restaurant", "chinese"]}}

@mourner mourner added this to the v1.0.0 milestone Jun 5, 2014
@mourner
Copy link
Member Author

mourner commented Jun 5, 2014

In #430

@mourner mourner closed this as completed Jun 5, 2014
@mourner
Copy link
Member Author

mourner commented Jun 5, 2014

Instead of having the source and layer properties in the filter object, we should just revert to having them at the top level object. What was the reason to move them to the filter object?

Forgot to mention one more reason: filter properties are inherited, so you can make styles more compact by combining layers with shared source, layer or feature_type into groups and specifying the property once for the whole group.

@kkaefer
Copy link
Member

kkaefer commented Jun 6, 2014

Wait, we support nested sources that inherit from their parents? Is there an example for this?

@mourner
Copy link
Member Author

mourner commented Jun 6, 2014

No example currently since migration scripts don't group layers — you have to do that manually if you want, but basically, lets say we have:

{
  "id": "landcover_snow",
  "filter": {"source": "outdoors", "layer": "landcover", "class": "snow"}
}, {
  "id": "landcover_crop",
  "filter": {"source": "outdoors", "layer": "landcover", "class": "crop"}
}, {
  "id": "landcover_grass",
  "filter": {"source": "outdoors", "layer": "landcover", "class": "grass"}
}, {
  "id": "landcover_scrub",
  "filter": {"source": "outdoors", "layer": "landcover", "class": "scrub"}
}, {
  "id": "landcover_wood",
  "filter": {"source": "outdoors", "layer": "landcover", "class": "wood"}
}

We can group them like this:

{
  "id": "landcover",
  "filter": {"source": "outdoors", "layer": "landcover"},
  "layers": [{
    "id": "landcover_snow",
    "filter": {"class": "snow"}
  }, {
    "id": "landcover_crop",
    "filter": {"class": "crop"}
  }, {
    "id": "landcover_grass",
    "filter": {"class": "grass"}
  }, {
    "id": "landcover_scrub",
    "filter": {"class": "scrub"}
  }, {
    "id": "landcover_wood",
    "filter": {"class": "wood"}
  }]
}

When preprocessing, the group filter gets propagated to children layers.

@kkaefer
Copy link
Member

kkaefer commented Jun 6, 2014

How are we going to represent nested layers then? As I understand it, all of the landcover_* layers are still at the top level?

@mourner
Copy link
Member Author

mourner commented Jun 6, 2014

Whenever styles indicate that the group will be composited, e.g. with opacity, prerender, comp-ops etc. Do you see any issues with this approach?

bensleveritt pushed a commit to bensleveritt/mapbox-gl-js that referenced this issue Oct 24, 2016
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

No branches or pull requests

4 participants