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.

Sign up for GitHub

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

Jump to bottom

EQL: implement case sensitivity for indexOf and endsWith string functions #57707

Merged
merged 3 commits into from
Jun 9, 2020
Merged

EQL: implement case sensitivity for indexOf and endsWith string functions #57707

merged 3 commits into from
Jun 9, 2020

Conversation

aleksmaus
Copy link
Member

Addresses: #56750

Thanks @astefan for paving the way!

@aleksmaus aleksmaus added the :Analytics/EQL EQL querying label Jun 5, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Jun 5, 2020
astefan
astefan approved these changes Jun 9, 2020
View reviewed changes
Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

rw-access
rw-access reviewed Jun 9, 2020
View reviewed changes
...ql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/EndsWith.java Outdated
.build(), dataType());
ParamsBuilder params = paramsBuilder();

String template = formatTemplate("{eql}.endsWith(" + sourceScript.template() + "," + patternScript.template() + ",{})");
Copy link
Contributor

Choose a reason for hiding this comment

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

We're not totally consistent in these. Here's something I did for stringContains

elasticsearch/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContains.java

Lines 87 to 98 in 8fb5b56

protected ScriptTemplate asScriptFrom(ScriptTemplate stringScript, ScriptTemplate substringScript) {
return new ScriptTemplate(format(Locale.ROOT, formatTemplate("{eql}.%s(%s,%s,%s)"),
"stringContains",
stringScript.template(),
substringScript.template(),
"{}"),
paramsBuilder()
.script(stringScript.params())
.script(substringScript.params())
.variable(isCaseSensitive())
.build(), dataType());
}

Copy link
Member Author

@aleksmaus aleksmaus Jun 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Followed Andrei's implementation for consistency, can change back. Let me know.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated PR

@aleksmaus aleksmaus merged commit e808026 into elastic:master Jun 9, 2020
aleksmaus added a commit to aleksmaus/elasticsearch that referenced this pull request Jun 10, 2020
@aleksmaus
EQL: implement case sensitivity for indexOf and endsWith string funct…
85a29e5 
…ions (elastic#57707)

* EQL: implement case sensitivity for indexOf and endsWith string functions
@aleksmaus aleksmaus mentioned this pull request Jun 10, 2020
Merged
aleksmaus added a commit that referenced this pull request Jun 10, 2020
@aleksmaus
EQL: implement case sensitivity for indexOf and endsWith string funct…
ec60335 
…ions (#57707) (#57908)

* EQL: implement case sensitivity for indexOf and endsWith string functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rw-access rw-access rw-access approved these changes

@astefan astefan astefan approved these changes

@costin costin Awaiting requested review from costin

@imotov imotov Awaiting requested review from imotov

Assignees
No one assigned
Labels
:Analytics/EQL EQL querying Team:QL (Deprecated) Meta label for query languages team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aleksmaus @elasticmachine @astefan @rw-access