EQL: Add case handling to stringContains (#58762) (#58813)

Co-authored-by: Ross Wolf <31489089+rw-access@users.noreply.github.com>
(cherry picked from commit 1a58776d3aa563beb364b067a1db46497122306f)
This commit is contained in:
Andrei Stefan 2020-07-01 13:51:45 +03:00 committed by GitHub
parent 470bcee5bf
commit b904a60275
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 135 additions and 52 deletions

View File

@ -6,13 +6,15 @@
package org.elasticsearch.xpack.eql.expression.function.scalar.string; package org.elasticsearch.xpack.eql.expression.function.scalar.string;
import org.elasticsearch.xpack.eql.session.EqlConfiguration;
import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.Expressions; import org.elasticsearch.xpack.ql.expression.Expressions;
import org.elasticsearch.xpack.ql.expression.FieldAttribute; import org.elasticsearch.xpack.ql.expression.FieldAttribute;
import org.elasticsearch.xpack.ql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.ql.expression.function.scalar.string.CaseSensitiveScalarFunction;
import org.elasticsearch.xpack.ql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.ql.expression.gen.pipeline.Pipe;
import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate;
import org.elasticsearch.xpack.ql.expression.gen.script.Scripts; import org.elasticsearch.xpack.ql.expression.gen.script.Scripts;
import org.elasticsearch.xpack.ql.session.Configuration;
import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.NodeInfo;
import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType; import org.elasticsearch.xpack.ql.type.DataType;
@ -32,12 +34,12 @@ import static org.elasticsearch.xpack.ql.expression.gen.script.ParamsBuilder.par
* stringContains(a, b) * stringContains(a, b)
* Returns true if b is a substring of a * Returns true if b is a substring of a
*/ */
public class StringContains extends ScalarFunction { public class StringContains extends CaseSensitiveScalarFunction {
private final Expression string, substring; private final Expression string, substring;
public StringContains(Source source, Expression string, Expression substring) { public StringContains(Source source, Expression string, Expression substring, Configuration configuration) {
super(source, Arrays.asList(string, substring)); super(source, Arrays.asList(string, substring), configuration);
this.string = string; this.string = string;
this.substring = substring; this.substring = substring;
} }
@ -59,7 +61,7 @@ public class StringContains extends ScalarFunction {
@Override @Override
protected Pipe makePipe() { protected Pipe makePipe() {
return new StringContainsFunctionPipe(source(), this, return new StringContainsFunctionPipe(source(), this,
Expressions.pipe(string), Expressions.pipe(substring)); Expressions.pipe(string), Expressions.pipe(substring), isCaseSensitive());
} }
@Override @Override
@ -69,12 +71,12 @@ public class StringContains extends ScalarFunction {
@Override @Override
public Object fold() { public Object fold() {
return doProcess(string.fold(), substring.fold()); return doProcess(string.fold(), substring.fold(), isCaseSensitive());
} }
@Override @Override
protected NodeInfo<? extends Expression> info() { protected NodeInfo<? extends Expression> info() {
return NodeInfo.create(this, StringContains::new, string, substring); return NodeInfo.create(this, StringContains::new, string, substring, eqlConfiguration());
} }
@Override @Override
@ -83,13 +85,15 @@ public class StringContains extends ScalarFunction {
} }
protected ScriptTemplate asScriptFrom(ScriptTemplate stringScript, ScriptTemplate substringScript) { protected ScriptTemplate asScriptFrom(ScriptTemplate stringScript, ScriptTemplate substringScript) {
return new ScriptTemplate(format(Locale.ROOT, formatTemplate("{eql}.%s(%s,%s)"), return new ScriptTemplate(format(Locale.ROOT, formatTemplate("{eql}.%s(%s,%s,%s)"),
"stringContains", "stringContains",
stringScript.template(), stringScript.template(),
substringScript.template()), substringScript.template(),
"{}"),
paramsBuilder() paramsBuilder()
.script(stringScript.params()) .script(stringScript.params())
.script(substringScript.params()) .script(substringScript.params())
.variable(isCaseSensitive())
.build(), dataType()); .build(), dataType());
} }
@ -111,6 +115,15 @@ public class StringContains extends ScalarFunction {
throw new IllegalArgumentException("expected [2] children but received [" + newChildren.size() + "]"); throw new IllegalArgumentException("expected [2] children but received [" + newChildren.size() + "]");
} }
return new StringContains(source(), newChildren.get(0), newChildren.get(1)); return new StringContains(source(), newChildren.get(0), newChildren.get(1), eqlConfiguration());
}
public EqlConfiguration eqlConfiguration() {
return (EqlConfiguration) configuration();
}
@Override
public boolean isCaseSensitive() {
return eqlConfiguration().isCaseSensitive();
} }
} }

View File

@ -19,11 +19,13 @@ import java.util.Objects;
public class StringContainsFunctionPipe extends Pipe { public class StringContainsFunctionPipe extends Pipe {
private final Pipe string, substring; private final Pipe string, substring;
private final boolean isCaseSensitive;
public StringContainsFunctionPipe(Source source, Expression expression, Pipe string, Pipe substring) { public StringContainsFunctionPipe(Source source, Expression expression, Pipe string, Pipe substring, boolean isCaseSensitive) {
super(source, expression, Arrays.asList(string, substring)); super(source, expression, Arrays.asList(string, substring));
this.string = string; this.string = string;
this.substring = substring; this.substring = substring;
this.isCaseSensitive = isCaseSensitive;
} }
@Override @Override
@ -55,7 +57,7 @@ public class StringContainsFunctionPipe extends Pipe {
} }
protected StringContainsFunctionPipe replaceChildren(Pipe string, Pipe substring) { protected StringContainsFunctionPipe replaceChildren(Pipe string, Pipe substring) {
return new StringContainsFunctionPipe(source(), expression(), string, substring); return new StringContainsFunctionPipe(source(), expression(), string, substring, isCaseSensitive);
} }
@Override @Override
@ -66,12 +68,12 @@ public class StringContainsFunctionPipe extends Pipe {
@Override @Override
protected NodeInfo<StringContainsFunctionPipe> info() { protected NodeInfo<StringContainsFunctionPipe> info() {
return NodeInfo.create(this, StringContainsFunctionPipe::new, expression(), string, substring); return NodeInfo.create(this, StringContainsFunctionPipe::new, expression(), string, substring, isCaseSensitive);
} }
@Override @Override
public StringContainsFunctionProcessor asProcessor() { public StringContainsFunctionProcessor asProcessor() {
return new StringContainsFunctionProcessor(string.asProcessor(), substring.asProcessor()); return new StringContainsFunctionProcessor(string.asProcessor(), substring.asProcessor(), isCaseSensitive);
} }
public Pipe string() { public Pipe string() {
@ -82,6 +84,9 @@ public class StringContainsFunctionPipe extends Pipe {
return substring; return substring;
} }
protected boolean isCaseSensitive() {
return isCaseSensitive;
}
@Override @Override
public int hashCode() { public int hashCode() {

View File

@ -19,29 +19,33 @@ public class StringContainsFunctionProcessor implements Processor {
public static final String NAME = "sstc"; public static final String NAME = "sstc";
private final Processor string, substring; private final Processor string, substring;
private final boolean isCaseSensitive;
public StringContainsFunctionProcessor(Processor string, Processor substring) { public StringContainsFunctionProcessor(Processor string, Processor substring, boolean isCaseSensitive) {
this.string = string; this.string = string;
this.substring = substring; this.substring = substring;
this.isCaseSensitive = isCaseSensitive;
} }
public StringContainsFunctionProcessor(StreamInput in) throws IOException { public StringContainsFunctionProcessor(StreamInput in) throws IOException {
string = in.readNamedWriteable(Processor.class); string = in.readNamedWriteable(Processor.class);
substring = in.readNamedWriteable(Processor.class); substring = in.readNamedWriteable(Processor.class);
isCaseSensitive = in.readBoolean();
} }
@Override @Override
public final void writeTo(StreamOutput out) throws IOException { public final void writeTo(StreamOutput out) throws IOException {
out.writeNamedWriteable(string); out.writeNamedWriteable(string);
out.writeNamedWriteable(substring); out.writeNamedWriteable(substring);
out.writeBoolean(isCaseSensitive);
} }
@Override @Override
public Object process(Object input) { public Object process(Object input) {
return doProcess(string.process(input), substring.process(input)); return doProcess(string.process(input), substring.process(input), isCaseSensitive);
} }
public static Object doProcess(Object string, Object substring) { public static Object doProcess(Object string, Object substring, boolean isCaseSensitive) {
if (string == null) { if (string == null) {
return null; return null;
} }
@ -51,7 +55,8 @@ public class StringContainsFunctionProcessor implements Processor {
String strString = string.toString(); String strString = string.toString();
String strSubstring = substring.toString(); String strSubstring = substring.toString();
return StringUtils.stringContains(strString, strSubstring);
return StringUtils.stringContains(strString, strSubstring, isCaseSensitive);
} }
private static void throwIfNotString(Object obj) { private static void throwIfNotString(Object obj) {

View File

@ -65,15 +65,19 @@ final class StringUtils {
* *
* @param string string to search through. * @param string string to search through.
* @param substring string to search for. * @param substring string to search for.
* @param isCaseSensitive toggle for case sensitivity.
* @return {@code true} if {@code string} string contains {@code substring} string. * @return {@code true} if {@code string} string contains {@code substring} string.
*/ */
static boolean stringContains(String string, String substring) { static boolean stringContains(String string, String substring, boolean isCaseSensitive) {
if (hasLength(string) == false || hasLength(substring) == false) { if (hasLength(string) == false || hasLength(substring) == false) {
return false; return false;
} }
if (isCaseSensitive == false) {
string = string.toLowerCase(Locale.ROOT); string = string.toLowerCase(Locale.ROOT);
substring = substring.toLowerCase(Locale.ROOT); substring = substring.toLowerCase(Locale.ROOT);
}
return string.contains(substring); return string.contains(substring);
} }

View File

@ -57,8 +57,8 @@ public class InternalEqlScriptUtils extends InternalQlScriptUtils {
return (String) ToStringFunctionProcessor.doProcess(s); return (String) ToStringFunctionProcessor.doProcess(s);
} }
public static Boolean stringContains(String string, String substring) { public static Boolean stringContains(String string, String substring, Boolean isCaseSensitive) {
return (Boolean) StringContainsFunctionProcessor.doProcess(string, substring); return (Boolean) StringContainsFunctionProcessor.doProcess(string, substring, isCaseSensitive);
} }
public static Number number(String source, Number base) { public static Number number(String source, Number base) {

View File

@ -73,6 +73,6 @@ class org.elasticsearch.xpack.eql.expression.function.scalar.whitelist.InternalE
Integer length(String) Integer length(String)
Number number(String, Number) Number number(String, Number)
String string(Object) String string(Object)
Boolean stringContains(String, String) Boolean stringContains(String, String, Boolean)
String substring(String, Number, Number) String substring(String, Number, Number)
} }

View File

@ -6,6 +6,7 @@
package org.elasticsearch.xpack.eql.expression.function.scalar.string; package org.elasticsearch.xpack.eql.expression.function.scalar.string;
import org.elasticsearch.xpack.eql.EqlTestUtils;
import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.ql.expression.gen.pipeline.Pipe;
import org.elasticsearch.xpack.ql.tree.AbstractNodeTestCase; import org.elasticsearch.xpack.ql.tree.AbstractNodeTestCase;
@ -32,7 +33,11 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase<String
} }
public static StringContainsFunctionPipe randomStringContainsFunctionPipe() { public static StringContainsFunctionPipe randomStringContainsFunctionPipe() {
return (StringContainsFunctionPipe) (new StringContains(randomSource(), randomStringLiteral(), randomStringLiteral()).makePipe()); return (StringContainsFunctionPipe) (new StringContains(randomSource(),
randomStringLiteral(),
randomStringLiteral(),
EqlTestUtils.randomConfiguration())
.makePipe());
} }
@Override @Override
@ -45,7 +50,8 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase<String
b1.source(), b1.source(),
newExpression, newExpression,
b1.string(), b1.string(),
b1.substring()); b1.substring(),
b1.isCaseSensitive());
assertEquals(newB, b1.transformPropertiesOnly(v -> Objects.equals(v, b1.expression()) ? newExpression : v, Expression.class)); assertEquals(newB, b1.transformPropertiesOnly(v -> Objects.equals(v, b1.expression()) ? newExpression : v, Expression.class));
StringContainsFunctionPipe b2 = randomInstance(); StringContainsFunctionPipe b2 = randomInstance();
@ -54,7 +60,8 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase<String
newLoc, newLoc,
b2.expression(), b2.expression(),
b2.string(), b2.string(),
b2.substring()); b2.substring(),
b2.isCaseSensitive());
assertEquals(newB, assertEquals(newB,
b2.transformPropertiesOnly(v -> Objects.equals(v, b2.source()) ? newLoc : v, Source.class)); b2.transformPropertiesOnly(v -> Objects.equals(v, b2.source()) ? newLoc : v, Source.class));
} }
@ -64,8 +71,9 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase<String
StringContainsFunctionPipe b = randomInstance(); StringContainsFunctionPipe b = randomInstance();
Pipe newString = pipe(((Expression) randomValueOtherThan(b.string(), () -> randomStringLiteral()))); Pipe newString = pipe(((Expression) randomValueOtherThan(b.string(), () -> randomStringLiteral())));
Pipe newSubstring = pipe(((Expression) randomValueOtherThan(b.substring(), () -> randomStringLiteral()))); Pipe newSubstring = pipe(((Expression) randomValueOtherThan(b.substring(), () -> randomStringLiteral())));
boolean newCaseSensitive = randomValueOtherThan(b.isCaseSensitive(), () -> randomBoolean());
StringContainsFunctionPipe newB = StringContainsFunctionPipe newB =
new StringContainsFunctionPipe(b.source(), b.expression(), b.string(), b.substring()); new StringContainsFunctionPipe(b.source(), b.expression(), b.string(), b.substring(), newCaseSensitive);
StringContainsFunctionPipe transformed = newB.replaceChildren(newString, b.substring()); StringContainsFunctionPipe transformed = newB.replaceChildren(newString, b.substring());
assertEquals(transformed.string(), newString); assertEquals(transformed.string(), newString);
@ -92,15 +100,18 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase<String
randoms.add(f -> new StringContainsFunctionPipe(f.source(), randoms.add(f -> new StringContainsFunctionPipe(f.source(),
f.expression(), f.expression(),
pipe(((Expression) randomValueOtherThan(f.string(), () -> randomStringLiteral()))), pipe(((Expression) randomValueOtherThan(f.string(), () -> randomStringLiteral()))),
f.substring())); f.substring(),
randomValueOtherThan(f.isCaseSensitive(), () -> randomBoolean())));
randoms.add(f -> new StringContainsFunctionPipe(f.source(), randoms.add(f -> new StringContainsFunctionPipe(f.source(),
f.expression(), f.expression(),
f.string(), f.string(),
pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))))); pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))),
randomValueOtherThan(f.isCaseSensitive(), () -> randomBoolean())));
randoms.add(f -> new StringContainsFunctionPipe(f.source(), randoms.add(f -> new StringContainsFunctionPipe(f.source(),
f.expression(), f.expression(),
pipe(((Expression) randomValueOtherThan(f.string(), () -> randomStringLiteral()))), pipe(((Expression) randomValueOtherThan(f.string(), () -> randomStringLiteral()))),
pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))))); pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))),
randomValueOtherThan(f.isCaseSensitive(), () -> randomBoolean())));
return randomFrom(randoms).apply(instance); return randomFrom(randoms).apply(instance);
} }
@ -110,6 +121,7 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase<String
return new StringContainsFunctionPipe(instance.source(), return new StringContainsFunctionPipe(instance.source(),
instance.expression(), instance.expression(),
instance.string(), instance.string(),
instance.substring()); instance.substring(),
instance.isCaseSensitive());
} }
} }

View File

@ -8,10 +8,11 @@ package org.elasticsearch.xpack.eql.expression.function.scalar.string;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.eql.EqlIllegalArgumentException; import org.elasticsearch.xpack.eql.EqlIllegalArgumentException;
import org.elasticsearch.xpack.eql.expression.function.scalar.string.StringContainsFunctionProcessor;
import java.util.Locale;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringContainsFunctionProcessor.doProcess;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
public class StringContainsFunctionProcessorTests extends ESTestCase { public class StringContainsFunctionProcessorTests extends ESTestCase {
@ -24,26 +25,36 @@ public class StringContainsFunctionProcessorTests extends ESTestCase {
} }
} }
public void testNullOrEmptyParameters() throws Exception { public void testStringContains() throws Exception {
run(() -> { run(() -> {
String substring = randomBoolean() ? null : randomAlphaOfLength(10); String substring = randomBoolean() ? null : randomAlphaOfLength(10);
String str = randomBoolean() ? null : randomAlphaOfLength(10); String str = randomBoolean() ? null : randomValueOtherThan(substring, () -> randomAlphaOfLength(10));
boolean caseSensitive = randomBoolean();
if (str != null && substring != null) { if (str != null && substring != null) {
str += substring; str += substring;
str += randomAlphaOfLength(10); str += randomValueOtherThan(substring, () -> randomAlphaOfLength(10));
} }
final String string = str; final String string = str;
// The string parameter can be null. Expect exception if any of other parameters is null. // The string parameter can be null. Expect exception if any of other parameters is null.
if ((string != null) && (substring == null)) { if (string != null && substring == null) {
EqlIllegalArgumentException e = expectThrows(EqlIllegalArgumentException.class, EqlIllegalArgumentException e = expectThrows(EqlIllegalArgumentException.class,
() -> StringContainsFunctionProcessor.doProcess(string, substring)); () -> doProcess(string, substring, caseSensitive));
assertThat(e.getMessage(), equalTo("A string/char is required; received [null]")); assertThat(e.getMessage(), equalTo("A string/char is required; received [null]"));
} else { } else {
assertThat(StringContainsFunctionProcessor.doProcess(string, substring), assertThat(doProcess(string, substring, caseSensitive), equalTo(string == null ? null : true));
equalTo(string == null ? null : true));
// deliberately make the test return "false" by lowercasing or uppercasing the substring in a case-sensitive scenario
if (caseSensitive && substring != null) {
String subsChanged = randomBoolean() ? substring.toLowerCase(Locale.ROOT) : substring.toUpperCase(Locale.ROOT);
if (substring.equals(subsChanged) == false) {
assertThat(doProcess(string, subsChanged, caseSensitive), equalTo(string == null ? null : false));
} }
}
}
return null; return null;
}); });
} }
} }

View File

@ -8,6 +8,8 @@ package org.elasticsearch.xpack.eql.expression.function.scalar.string;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import java.util.Locale;
import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringUtils.stringContains; import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringUtils.stringContains;
import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringUtils.substringSlice; import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringUtils.substringSlice;
import static org.elasticsearch.xpack.ql.util.StringUtils.EMPTY; import static org.elasticsearch.xpack.ql.util.StringUtils.EMPTY;
@ -140,14 +142,36 @@ public class StringUtilsTests extends ESTestCase {
} }
public void testStringContainsWithNullOrEmpty() { public void testStringContainsWithNullOrEmpty() {
assertFalse(stringContains(null, null)); assertFalse(stringContains(null, null, true));
assertFalse(stringContains(null, "")); assertFalse(stringContains(null, "", true));
assertFalse(stringContains("", null)); assertFalse(stringContains("", null, true));
assertFalse(stringContains(null, null, false));
assertFalse(stringContains(null, "", false));
assertFalse(stringContains("", null, false));
} }
public void testStringContainsWithRandom() throws Exception { public void testStringContainsWithRandomCaseSensitive() throws Exception {
String substring = randomAlphaOfLength(10); String substring = randomAlphaOfLength(10);
String string = randomAlphaOfLength(10) + substring + randomAlphaOfLength(10); String string = randomValueOtherThan(substring, () -> randomAlphaOfLength(10))
assertTrue(stringContains(string, substring)); + substring
+ randomValueOtherThan(substring, () -> randomAlphaOfLength(10));
assertTrue(stringContains(string, substring, true));
}
public void testStringContainsWithRandomCaseInsensitive() throws Exception {
String substring = randomAlphaOfLength(10);
String subsChanged = substring.toUpperCase(Locale.ROOT);
String string = randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10))
+ subsChanged
+ randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10));
assertTrue(stringContains(string, substring, false));
substring = randomAlphaOfLength(10);
subsChanged = substring.toLowerCase(Locale.ROOT);
string = randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10))
+ subsChanged
+ randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10));
assertTrue(stringContains(string, substring, false));
} }
} }

View File

@ -189,14 +189,23 @@ process where startsWith(user_name, 'A') or startsWith(user_name, 'B')
{"prefix":{"user_name":{"value":"B","boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}}] {"prefix":{"user_name":{"value":"B","boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}}]
; ;
stringContains stringContains-caseSensitive
process where stringContains(process_name, "foo") process where stringContains(process_name, "foo")
; ;
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.stringContains( "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.stringContains(
InternalQlScriptUtils.docValue(doc,params.v0),params.v1))" InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2))"
"params":{"v0":"process_name","v1":"foo"} "params":{"v0":"process_name","v1":"foo","v2":true}
; ;
stringContains-caseInsensitive
process where stringContains(process_name, "foo")
;
"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.stringContains(
InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2))"
"params":{"v0":"process_name","v1":"foo","v2":false}
;
stringFunction stringFunction
process where string(pid) == "123" process where string(pid) == "123"
; ;