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

EQL: Remove case_sensitive option #62255

Closed
costin opened this issue Sep 10, 2020 · 6 comments
Closed

EQL: Remove case_sensitive option #62255

costin opened this issue Sep 10, 2020 · 6 comments
Assignees
Labels
:Analytics/EQL EQL querying >bug >refactoring Team:QL (Deprecated) Meta label for query languages team

Comments

@costin
Copy link
Member

costin commented Sep 10, 2020

Currently in EQL, requests allow a case_sensitive option for specifying whether string equality is case sensitive or not, as well the behavior for some functions. Initially this was introduced to determine the existing behavior and because the actual implementation requires scripting which is not ideal performance wise.

As Elasticsearch now supports such queries, this setting should be removed and EQL be always insensitive, where applicable.
This means:
operators: == and !=. Currently >=, <=, < and > are not supported
functions: startsWith, endsWith, stringContains, between, match, wildcard (note some regex might not fully support this, requiring the user to change the regex. This is considering missing functionality and will be added asap once the missing bits land in ES).

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (:Query Languages/EQL)

@rw-access
Copy link
Contributor

rw-access commented Sep 16, 2020

I think we still need the toggle for the detection engine, since I would expect both case-sensitive and case-insensitive queries to be widely used. @paulewing or @tsg can chime in more

@tsg
Copy link

tsg commented Sep 17, 2020

Yes, from what I understood from @MikePaquette when this has come up before, we want equalities to be case-insensitive by default as that is most common, but there are also cases where we need case-sensitive equals. I'm not a fan of the global case-sensitive option because it introduces a "mode" that is external to the language and it doesn't let the rule author combine case-sensitive and case-insensitive comparisons in the same query.

Perhaps we can discuss this one live in the EQL meeting today.

@matriv
Copy link
Contributor

matriv commented Sep 21, 2020

I'm also against a global param that toggles case-sensitivity.

  • It removes the flexibility to combine case-sensitive and case-insensitive comparisons in the same query
  • Operators like >, <` that are case sensitive (exact) are not affected by this toggle, therefore removes the "global" effect of the toggle

So, I strongly prefer the per operator/function definition of the case-sensitivity.

@costin
Copy link
Member Author

costin commented Sep 22, 2020

A belated update to this ticket after discussing it further:

  1. the case-sensitive global flag is removed
  2. if there will be a need in the future to add case sensitive equality, there is the option of a function exactEquals or potentially a different set of operators such as === (though their semantics in PHP and JS are different)
  3. Equality operator == remains case insensitive, while the associated operators remains case sensitive.
  4. Functions that require case sensitivity, will have a flag to turn-off this behavior. By default it is false meaning the functions, just like == are case insensitive.

In other words, the intent and scope of this ticket remains the same.

@costin
Copy link
Member Author

costin commented Oct 7, 2020

Closed by #63218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/EQL EQL querying >bug >refactoring Team:QL (Deprecated) Meta label for query languages team
Projects
None yet
Development

No branches or pull requests

7 participants