EQL: startsWith function implementation#54400
Conversation
|
Pinging @elastic/es-ql (:Query Languages/EQL) |
|
@elasticmachine run elasticsearch-ci/bwc |
| if (source instanceof String == false && source instanceof Character == false) { | ||
| throw new EqlIllegalArgumentException("A string/char is required; received [{}]", source); | ||
| } | ||
| if (pattern == null) { |
There was a problem hiding this comment.
maybe open for discussion: return null only if he source is null and exception for the rest of params that should not be null?
There was a problem hiding this comment.
Sound good. Looks like another pattern that could be extracted/reused depending on which way we want to proceed with null handling for consistency between all functions.
| return null; | ||
| } | ||
| if (pattern instanceof String == false && pattern instanceof Character == false) { | ||
| throw new EqlIllegalArgumentException("A string/char is required; received [{}]", pattern); |
There was a problem hiding this comment.
nit: just a suggestion maybe extract the function to avoid repetition, for example:
https://github.com/elastic/elasticsearch/pull/54380/files#diff-5ad1821091fb7e7473cbdfdded44fbcbR52
There was a problem hiding this comment.
I believe that, at one point after all these string functions are done, we need to refactor them and extract some common code in a base class, if possible of course.
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(source()); |
There was a problem hiding this comment.
should include the pattern in both equals and hashCode?
There was a problem hiding this comment.
Good catch. Will fix.
|
@elasticmachine update branch |
(cherry picked from commit 666719f)
Addresses #53855