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: Update between() case handling and tests #59738

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1065,31 +1065,3 @@ sequence
'''
expected_event_ids = [1, 2,
2, 3]

# TODO: update toggles for this function
[[queries]]
name = "betweenCaseSensitive"
case_sensitive = true
expected_event_ids = [1, 2, 42]
query = '''
process where between(process_name, "s", "e", false) == "t"
'''

# TODO: add toggles to this function so it's not always insensitive
[[queries]]
name = "lengthCaseSensitive"
case_sensitive = true
expected_event_ids = [7, 14, 29, 44]
query = '''
process where length(between(process_name, 'g', 'e')) > 0
'''

# TODO: add toggles to this function so it's not always insensitive
#[[queries]]
#name = ""
#case_insensitive = true
#expected_event_ids = [7, 14, 22, 29, 44]
#query = '''
#process where length(between(process_name, 'g', 'e')) > 0
#'''

Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private static FunctionDefinition[][] functions() {
// Scalar functions
// String
new FunctionDefinition[] {
def(Between.class, Between::new, 2, "between"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the number of optional parameters is not needed anymore, maybe we should remove it from FunctionRegistry.

def(Between.class, Between::new, "between"),
def(CIDRMatch.class, CIDRMatch::new, "cidrmatch"),
def(Concat.class, Concat::new, "concat"),
def(EndsWith.class, EndsWith::new, "endswith"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@

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.Expressions;
import org.elasticsearch.xpack.ql.expression.FieldAttribute;
import org.elasticsearch.xpack.ql.expression.Literal;
import org.elasticsearch.xpack.ql.expression.function.OptionalArgument;
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.script.ScriptTemplate;
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.Source;
import org.elasticsearch.xpack.ql.type.DataType;
Expand All @@ -32,20 +34,24 @@

/**
* EQL specific between function.
* between(source, left, right[, greedy=false, case_sensitive=false])
* between(source, left, right[, greedy=false])
* Extracts a substring from source that’s between left and right substrings
*/
public class Between extends ScalarFunction implements OptionalArgument {
public class Between extends CaseSensitiveScalarFunction implements OptionalArgument {

private final Expression input, left, right, greedy, caseSensitive;
private final Expression input, left, right, greedy;

public Between(Source source, Expression input, Expression left, Expression right, Expression greedy, Expression caseSensitive) {
super(source, Arrays.asList(input, left, right, toDefault(greedy), toDefault(caseSensitive)));
public Between(Source source, Expression input, Expression left, Expression right, Expression greedy, Configuration configuration) {
super(source, Arrays.asList(input, left, right, toDefault(greedy)), configuration);
this.input = input;
this.left = left;
this.right = right;
this.greedy = arguments().get(3);
this.caseSensitive = arguments().get(4);
}

@Override
public boolean isCaseSensitive() {
return ((EqlConfiguration) configuration()).isCaseSensitive();
}

private static Expression toDefault(Expression exp) {
Expand Down Expand Up @@ -73,34 +79,29 @@ protected TypeResolution resolveType() {
return resolution;
}

resolution = isBoolean(greedy, sourceText(), Expressions.ParamOrdinal.FOURTH);
if (resolution.unresolved()) {
return resolution;
}

return isBoolean(caseSensitive, sourceText(), Expressions.ParamOrdinal.FIFTH);
return isBoolean(greedy, sourceText(), Expressions.ParamOrdinal.FOURTH);
}

@Override
protected Pipe makePipe() {
return new BetweenFunctionPipe(source(), this, Expressions.pipe(input),
Expressions.pipe(left), Expressions.pipe(right),
Expressions.pipe(greedy), Expressions.pipe(caseSensitive));
Expressions.pipe(greedy), isCaseSensitive());
}

@Override
public boolean foldable() {
return input.foldable() && left.foldable() && right.foldable() && greedy.foldable() && caseSensitive.foldable();
return input.foldable() && left.foldable() && right.foldable() && greedy.foldable();
}

@Override
public Object fold() {
return doProcess(input.fold(), left.fold(), right.fold(), greedy.fold(), caseSensitive.fold());
return doProcess(input.fold(), left.fold(), right.fold(), greedy.fold(), isCaseSensitive());
}

@Override
protected NodeInfo<? extends Expression> info() {
return NodeInfo.create(this, Between::new, input, left, right, greedy, caseSensitive);
return NodeInfo.create(this, Between::new, input, left, right, greedy, configuration());
}

@Override
Expand All @@ -109,26 +110,25 @@ public ScriptTemplate asScript() {
ScriptTemplate leftScript = asScript(left);
ScriptTemplate rightScript = asScript(right);
ScriptTemplate greedyScript = asScript(greedy);
ScriptTemplate caseSensitiveScript = asScript(caseSensitive);

return asScriptFrom(inputScript, leftScript, rightScript, greedyScript, caseSensitiveScript);
return asScriptFrom(inputScript, leftScript, rightScript, greedyScript);
}

protected ScriptTemplate asScriptFrom(ScriptTemplate inputScript, ScriptTemplate leftScript,
ScriptTemplate rightScript, ScriptTemplate greedyScript, ScriptTemplate caseSensitiveScript) {
ScriptTemplate rightScript, ScriptTemplate greedyScript) {
return new ScriptTemplate(format(Locale.ROOT, formatTemplate("{eql}.%s(%s,%s,%s,%s,%s)"),
"between",
inputScript.template(),
leftScript.template(),
rightScript.template(),
greedyScript.template(),
caseSensitiveScript.template()),
"{}"),
paramsBuilder()
.script(inputScript.params())
.script(leftScript.params())
.script(rightScript.params())
.script(greedyScript.params())
.script(caseSensitiveScript.params())
.variable(isCaseSensitive())
.build(), dataType());
}

Expand All @@ -146,10 +146,10 @@ public DataType dataType() {

@Override
public Expression replaceChildren(List<Expression> newChildren) {
if (newChildren.size() != 5) {
throw new IllegalArgumentException("expected [5] children but received [" + newChildren.size() + "]");
if (newChildren.size() != 4) {
throw new IllegalArgumentException("expected [4] children but received [" + newChildren.size() + "]");
}

return new Between(source(), newChildren.get(0), newChildren.get(1), newChildren.get(2), newChildren.get(3), newChildren.get(4));
return new Between(source(), newChildren.get(0), newChildren.get(1), newChildren.get(2), newChildren.get(3), configuration());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,25 @@

public class BetweenFunctionPipe extends Pipe {

private final Pipe input, left, right, greedy, caseSensitive;
private final Pipe input, left, right, greedy;
private final boolean isCaseSensitive;

public BetweenFunctionPipe(Source source, Expression expression, Pipe input, Pipe left, Pipe right, Pipe greedy, Pipe caseSensitive) {
super(source, expression, Arrays.asList(input, left, right, greedy, caseSensitive));
public BetweenFunctionPipe(Source source, Expression expression, Pipe input, Pipe left, Pipe right, Pipe greedy,
boolean isCaseSensitive) {
super(source, expression, Arrays.asList(input, left, right, greedy));
this.input = input;
this.left = left;
this.right = right;
this.greedy = greedy;
this.caseSensitive = caseSensitive;
this.isCaseSensitive = isCaseSensitive;
}

@Override
public final Pipe replaceChildren(List<Pipe> newChildren) {
if (newChildren.size() != 5) {
throw new IllegalArgumentException("expected [5] children but received [" + newChildren.size() + "]");
if (newChildren.size() != 4) {
throw new IllegalArgumentException("expected [4] children but received [" + newChildren.size() + "]");
}
return replaceChildren(newChildren.get(0), newChildren.get(1), newChildren.get(2), newChildren.get(3), newChildren.get(4));
return replaceChildren(newChildren.get(0), newChildren.get(1), newChildren.get(2), newChildren.get(3));
}

@Override
Expand All @@ -42,26 +44,26 @@ public final Pipe resolveAttributes(AttributeResolver resolver) {
Pipe newLeft = left.resolveAttributes(resolver);
Pipe newRight = right.resolveAttributes(resolver);
Pipe newGreedy = greedy.resolveAttributes(resolver);
Pipe newCaseSensitive = caseSensitive.resolveAttributes(resolver);
if (newInput == input && newLeft == left && newRight == right && newGreedy == greedy && newCaseSensitive == caseSensitive) {

if (newInput == input && newLeft == left && newRight == right && newGreedy == greedy) {
return this;
}
return replaceChildren(newInput, newLeft, newRight, newGreedy, newCaseSensitive);
return replaceChildren(newInput, newLeft, newRight, newGreedy);
}

@Override
public boolean supportedByAggsOnlyQuery() {
return input.supportedByAggsOnlyQuery() && left.supportedByAggsOnlyQuery() && right.supportedByAggsOnlyQuery()
&& greedy.supportedByAggsOnlyQuery() && caseSensitive.supportedByAggsOnlyQuery();
&& greedy.supportedByAggsOnlyQuery();
}

@Override
public boolean resolved() {
return input.resolved() && left.resolved() && right.resolved() && greedy.resolved() && caseSensitive.resolved();
return input.resolved() && left.resolved() && right.resolved() && greedy.resolved();
}

protected Pipe replaceChildren(Pipe newInput, Pipe newLeft, Pipe newRight, Pipe newGreedy, Pipe newCaseSensitive) {
return new BetweenFunctionPipe(source(), expression(), newInput, newLeft, newRight, newGreedy, newCaseSensitive);
protected Pipe replaceChildren(Pipe newInput, Pipe newLeft, Pipe newRight, Pipe newGreedy) {
return new BetweenFunctionPipe(source(), expression(), newInput, newLeft, newRight, newGreedy, isCaseSensitive);
}

@Override
Expand All @@ -70,18 +72,17 @@ public final void collectFields(QlSourceBuilder sourceBuilder) {
left.collectFields(sourceBuilder);
right.collectFields(sourceBuilder);
greedy.collectFields(sourceBuilder);
caseSensitive.collectFields(sourceBuilder);
}

@Override
protected NodeInfo<BetweenFunctionPipe> info() {
return NodeInfo.create(this, BetweenFunctionPipe::new, expression(), input, left, right, greedy, caseSensitive);
return NodeInfo.create(this, BetweenFunctionPipe::new, expression(), input, left, right, greedy, isCaseSensitive);
}

@Override
public BetweenFunctionProcessor asProcessor() {
return new BetweenFunctionProcessor(input.asProcessor(), left.asProcessor(), right.asProcessor(),
greedy.asProcessor(), caseSensitive.asProcessor());
greedy.asProcessor(), isCaseSensitive);
}

public Pipe input() {
Expand All @@ -100,13 +101,13 @@ public Pipe greedy() {
return greedy;
}

public Pipe caseSensitive() {
return caseSensitive;
protected boolean isCaseSensitive() {
return isCaseSensitive;
}

@Override
public int hashCode() {
return Objects.hash(input(), left(), right(), greedy(), caseSensitive());
return Objects.hash(input(), left(), right(), greedy(), isCaseSensitive);
}

@Override
Expand All @@ -124,6 +125,6 @@ public boolean equals(Object obj) {
&& Objects.equals(left(), other.left())
&& Objects.equals(right(), other.right())
&& Objects.equals(greedy(), other.greedy())
&& Objects.equals(caseSensitive(), other.caseSensitive());
&& Objects.equals(isCaseSensitive(), other.isCaseSensitive());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,23 @@ public class BetweenFunctionProcessor implements Processor {

public static final String NAME = "sbtw";

private final Processor input, left, right, greedy, caseSensitive;
private final Processor input, left, right, greedy;
private final boolean isCaseSensitive;

public BetweenFunctionProcessor(Processor input, Processor left, Processor right, Processor greedy, Processor caseSensitive) {
public BetweenFunctionProcessor(Processor input, Processor left, Processor right, Processor greedy, boolean isCaseSensitive) {
this.input = input;
this.left = left;
this.right = right;
this.greedy = greedy;
this.caseSensitive = caseSensitive;
this.isCaseSensitive = isCaseSensitive;
}

public BetweenFunctionProcessor(StreamInput in) throws IOException {
input = in.readNamedWriteable(Processor.class);
left = in.readNamedWriteable(Processor.class);
right = in.readNamedWriteable(Processor.class);
greedy = in.readNamedWriteable(Processor.class);
caseSensitive = in.readNamedWriteable(Processor.class);
isCaseSensitive = in.readBoolean();
}

@Override
Expand All @@ -41,7 +42,7 @@ public final void writeTo(StreamOutput out) throws IOException {
out.writeNamedWriteable(left);
out.writeNamedWriteable(right);
out.writeNamedWriteable(greedy);
out.writeNamedWriteable(caseSensitive);
out.writeBoolean(isCaseSensitive);
}

@Override
Expand All @@ -51,27 +52,24 @@ public String getWriteableName() {

@Override
public Object process(Object o) {
return doProcess(input.process(o), left.process(o), right.process(o), greedy.process(o), caseSensitive.process(o));
return doProcess(input.process(o), left.process(o), right.process(o), greedy.process(o), isCaseSensitive());
}

public static Object doProcess(Object input, Object left, Object right, Object greedy, Object caseSensitive) {
if (input == null) {
public static Object doProcess(Object input, Object left, Object right, Object greedy, boolean isCaseSensitive) {
if (input == null || left == null || right == null || greedy == null) {
return null;
}

Check.isString(input);
Check.isString(left);
Check.isString(right);

Check.isBoolean(greedy);
Check.isBoolean(caseSensitive);

String str = input.toString();
String strRight = right.toString();
String strLeft = left.toString();
boolean bGreedy = ((Boolean) greedy).booleanValue();
boolean bCaseSensitive = ((Boolean) caseSensitive).booleanValue();
return StringUtils.between(str, strLeft, strRight, bGreedy, bCaseSensitive);
boolean bGreedy = (Boolean) greedy;
return StringUtils.between(str, strLeft, strRight, bGreedy, isCaseSensitive);
}

protected Processor input() {
Expand All @@ -90,13 +88,13 @@ public Processor greedy() {
return greedy;
}

public Processor caseSensitive() {
return caseSensitive;
protected boolean isCaseSensitive() {
return isCaseSensitive;
}

@Override
public int hashCode() {
return Objects.hash(input(), left(), right(), greedy(), caseSensitive());
return Objects.hash(input(), left(), right(), greedy(), isCaseSensitive);
}

@Override
Expand All @@ -114,6 +112,6 @@ public boolean equals(Object obj) {
&& Objects.equals(left(), other.left())
&& Objects.equals(right(), other.right())
&& Objects.equals(greedy(), other.greedy())
&& Objects.equals(caseSensitive(), other.caseSensitive());
&& Objects.equals(isCaseSensitive(), other.isCaseSensitive());
}
}
Loading