[ios, macos] Support array values in match expressions. #11866
Conversation
| NSPredicate *predicate = [NSPredicate predicateWithFormat:@"$featureIdentifier IN { 6, 5, 4, 3}"]; | ||
| XCTAssertEqualObjects(predicate.mgl_jsonExpressionObject, expected); | ||
| NSPredicate *predicateAfter = [NSPredicate predicateWithFormat:@"MGL_MATCH(CAST($featureIdentifier, 'NSNumber'), 3, YES, 4, YES, 5, YES, 6, YES, NO) == YES"]; | ||
| NSPredicate *predicateAfter = [NSPredicate predicateWithFormat:@"MGL_MATCH(CAST($featureIdentifier, 'NSNumber'), { 6, 5, 4, 3}, YES, NO) == YES"]; |
There was a problem hiding this comment.
I’m worried this PR will eventually make it impossible to put an array-typed constant value (for MGLLineStyleLayer.lineDashPattern) into a match expression. As I mentioned in #11539 (comment), we currently conflate aggregate expressions with array constant values. I think this behavior is desirable; do we need to change it to accommodate this shortcut?
There was a problem hiding this comment.
Do we agree that the match operator has the following syntax:
"match", "variable", "valueArrays | keyPath | expression", "trueValue", "defaultValue"
The change I am making is that since is possible to have valueArrays, the predicate workaround for IN/CONTAINS inlines the array as pairs of value,boolean, but that is unnecessary because match already supports comparing to arrays. This change for NSPredicate only "standardizes" the syntax between NSExpression and NSPredicate.
What kind of expression are you thinking for MGLLineStyleLayer.lineDashPattern?
There was a problem hiding this comment.
👍 You’re right, I kept conflating input types with output types. Since feature attributes can’t currently be set to arrays (mapbox/mapbox-gl-js#2434), the only obvious use case for an array as a match input would be matching return values of a to-rgba expression. But even then, I agree that it is understandable that a match expression would only match against atomic values. The only confusion might come from Swift developers used to pattern matching on tuples in switch statements.
1ec5
added
iOS
fabian-guerra
force-pushed
the
fabian-match-aggregate-11539
branch
3 times, most recently
from
934b541
to
65f62bd
Compare
|
This PR is currently scheduled for iOS map SDK v4.1.0, so the branch needs to be rebased atop master. |
fabian-guerra
force-pushed
the
fabian-match-aggregate-11539
branch
from
65f62bd
to
4fe5742
Compare
fabian-guerra
force-pushed
the
fabian-match-aggregate-11539
branch
from
4fe5742
to
14f67e2
Compare
|
This PR has been retargeted, but it also needs to be rebased atop master. |
fabian-guerra
force-pushed
the
fabian-match-aggregate-11539
branch
from
14f67e2
to
5dd9f4f
Compare
| @@ -536,6 +536,7 @@ expression that contains references to those variables. | |||
| An input expression, then any number of argument pairs, followed by a default | |||
| expression. Each argument pair consists of a constant value followed by an | |||
| expression to produce as a result of matching that constant value. | |||
| An input value can be either a constant or aggregate expression. | |||
There was a problem hiding this comment.
If the input value is an aggregate expression, then any of the constant values within that aggregate expression result in the following argument. This is shorthand for specifying an argument pair for each of the constant values within that aggregate expression. It is not possible to match the aggregate expression itself.
Fixes #11539
Previously aggregate expressions were inlined as pairs of
value/boolean. This PR allows a constant value get tested against an aggregate expression.It does not support aggregate expressions as constant value.