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

8504 duplicate match exp #8542

Merged
merged 4 commits into from
Jul 25, 2019
Merged

8504 duplicate match exp #8542

merged 4 commits into from
Jul 25, 2019

Conversation

dereklieu
Copy link
Contributor

Closes #8504

The utility to convert legacy filters to expressions currently outputs a match expression as an optimization for certain legacy in, !in operators. This PR removes duplicate values from the result to ensure that the expression is valid.

return true;
}
return false;
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is O(n^2), so might get really slow when the filter has a lot of values (although not sure how likely we are to hit this in practice). An alternative would be to sort the values and then filter linearly, since it doesn't matter what order they are in the match expression:

const uniqueValues = values.sort().filter((v, i) => values[i - 1] !== v);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you can write this shorter without curly brackets, return and on one line.

return true;
}
return false;
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you can write this shorter without curly brackets, return and on one line.

@dereklieu dereklieu merged commit e309d47 into master Jul 25, 2019
@dereklieu dereklieu deleted the 8504-duplicate-match-exp branch July 25, 2019 22:07
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

Successfully merging this pull request may close these issues.

Style spec feature_filter should dedupe values for legacy in statements
3 participants