Skip to content

Commit

Permalink
SQL: Fix issue with LIKE/RLIKE as painless script (#53495)
Browse files Browse the repository at this point in the history
Add missing asScript() implementation for LIKE/RLIKE expressions.

When LIKE/RLIKE are used for example in GROUP BY or are wrapped with
scalar functions in a WHERE clause, the translation must produce a
painless script which will be executed to implement the correct
behaviour and previously this was completely missing, and as a
consquence wrong results were silently (no error) returned.

Fixes: #53486
  • Loading branch information
matriv authored Mar 16, 2020
1 parent 01eee1a commit eaa8ead
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
package org.elasticsearch.xpack.ql.expression.predicate.regex;

import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.gen.processor.Processor;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RegexProcessor.RegexOperation;
import org.elasticsearch.xpack.ql.tree.NodeInfo;
import org.elasticsearch.xpack.ql.tree.Source;

Expand All @@ -26,15 +24,4 @@ protected NodeInfo<Like> info() {
protected Like replaceChild(Expression newLeft) {
return new Like(source(), newLeft, pattern());
}

@Override
public Boolean fold() {
Object val = field().fold();
return RegexOperation.match(val, pattern().asJavaRegex());
}

@Override
protected Processor makeProcessor() {
return new RegexProcessor(pattern().asJavaRegex());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*
* To prevent conflicts with ES, the string and char must be validated to not contain '*'.
*/
public class LikePattern {
public class LikePattern implements StringPattern {

private final String pattern;
private final char escape;
Expand All @@ -43,9 +43,7 @@ public char escape() {
return escape;
}

/**
* Returns the pattern in (Java) regex format.
*/
@Override
public String asJavaRegex() {
return regex;
}
Expand Down Expand Up @@ -83,4 +81,4 @@ public boolean equals(Object obj) {
return Objects.equals(pattern, other.pattern)
&& escape == other.escape;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@
package org.elasticsearch.xpack.ql.expression.predicate.regex;

import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.gen.processor.Processor;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RegexProcessor.RegexOperation;
import org.elasticsearch.xpack.ql.tree.NodeInfo;
import org.elasticsearch.xpack.ql.tree.Source;

public class RLike extends RegexMatch<String> {
public class RLike extends RegexMatch<RLikePattern> {

public RLike(Source source, Expression value, String pattern) {
public RLike(Source source, Expression value, RLikePattern pattern) {
super(source, value, pattern);
}

Expand All @@ -26,15 +24,4 @@ protected NodeInfo<RLike> info() {
protected RLike replaceChild(Expression newChild) {
return new RLike(source(), newChild, pattern());
}

@Override
public Boolean fold() {
Object val = field().fold();
return RegexOperation.match(val, pattern());
}

@Override
protected Processor makeProcessor() {
return new RegexProcessor(pattern());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.ql.expression.predicate.regex;

public class RLikePattern implements StringPattern {

private final String regexpPattern;

public RLikePattern(String regexpPattern) {
this.regexpPattern = regexpPattern;
}

@Override
public String asJavaRegex() {
return regexpPattern;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,19 @@
import org.elasticsearch.xpack.ql.expression.Expressions;
import org.elasticsearch.xpack.ql.expression.Nullability;
import org.elasticsearch.xpack.ql.expression.function.scalar.UnaryScalarFunction;
import org.elasticsearch.xpack.ql.expression.gen.processor.Processor;
import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;

import java.util.Objects;

import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isStringAndExact;
import static org.elasticsearch.xpack.ql.expression.gen.script.ParamsBuilder.paramsBuilder;

public abstract class RegexMatch<T> extends UnaryScalarFunction {
public abstract class RegexMatch<T extends StringPattern> extends UnaryScalarFunction {

private final T pattern;

Expand Down Expand Up @@ -54,8 +58,30 @@ public boolean foldable() {
// right() is not directly foldable in any context but Like can fold it.
return field().foldable();
}


@Override
public Boolean fold() {
Object val = field().fold();
return RegexProcessor.RegexOperation.match(val, pattern().asJavaRegex());
}

@Override
protected Processor makeProcessor() {
return new RegexProcessor(pattern().asJavaRegex());
}

@Override
public ScriptTemplate asScript() {
ScriptTemplate fieldAsScript = asScript(field());
return new ScriptTemplate(
formatTemplate(format("{sql}.", "regex({},{})", fieldAsScript.template())),
paramsBuilder()
.script(fieldAsScript.params())
.variable(pattern.asJavaRegex())
.build(),
dataType());
}

public boolean equals(Object obj) {
return super.equals(obj) && Objects.equals(((RegexMatch<?>) obj).pattern(), pattern());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.ql.expression.predicate.regex;

interface StringPattern {
/**
* Returns the pattern in (Java) regex format.
*/
String asJavaRegex();
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public static Query doTranslate(RegexMatch e, TranslatorHandler handler) {
}

if (e instanceof RLike) {
String pattern = ((RLike) e).pattern();
String pattern = ((RLike) e).pattern().asJavaRegex();
q = new RegexQuery(e.source(), targetFieldName, pattern);
}

Expand Down Expand Up @@ -351,4 +351,4 @@ private static Query boolQuery(Source source, Query left, Query right, boolean i
}
return new BoolQuery(source, isAnd, left, right);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.elasticsearch.xpack.ql.expression.predicate.regex.Like;
import org.elasticsearch.xpack.ql.expression.predicate.regex.LikePattern;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLike;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLikePattern;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanLiteralsOnTheRight;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanSimplification;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.CombineBinaryComparisons;
Expand All @@ -48,13 +49,13 @@
import java.util.List;

import static java.util.Collections.emptyMap;
import static org.elasticsearch.xpack.ql.TestUtils.of;
import static org.elasticsearch.xpack.ql.expression.Literal.FALSE;
import static org.elasticsearch.xpack.ql.expression.Literal.NULL;
import static org.elasticsearch.xpack.ql.expression.Literal.TRUE;
import static org.elasticsearch.xpack.ql.tree.Source.EMPTY;
import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN;
import static org.elasticsearch.xpack.ql.type.DataTypes.INTEGER;
import static org.elasticsearch.xpack.ql.TestUtils.of;

public class OptimizerRulesTests extends ESTestCase {

Expand Down Expand Up @@ -189,7 +190,7 @@ public void testConstantFoldingLikes() {
new ConstantFolding().rule(new Like(EMPTY, of("test_emp"), new LikePattern("test%", (char) 0)))
.canonical());
assertEquals(TRUE,
new ConstantFolding().rule(new RLike(EMPTY, of("test_emp"), "test.emp")).canonical());
new ConstantFolding().rule(new RLike(EMPTY, of("test_emp"), new RLikePattern("test.emp"))).canonical());
}

public void testArithmeticFolding() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.elasticsearch.xpack.ql.expression.predicate.regex.Like;
import org.elasticsearch.xpack.ql.expression.predicate.regex.LikePattern;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLike;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLikePattern;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;
Expand Down Expand Up @@ -235,7 +236,7 @@ public Expression visitPredicated(PredicatedContext ctx) {
e = new Like(source, exp, visitPattern(pCtx.pattern()));
break;
case SqlBaseParser.RLIKE:
e = new RLike(source, exp, string(pCtx.regex));
e = new RLike(source, exp, new RLikePattern(string(pCtx.regex)));
break;
case SqlBaseParser.NULL:
// shortcut to avoid double negation later on (since there's no IsNull (missing in ES is a negated exists))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NotEquals;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.NullEquals;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLike;
import org.elasticsearch.xpack.ql.expression.predicate.regex.RLikePattern;
import org.elasticsearch.xpack.ql.index.EsIndex;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanLiteralsOnTheRight;
import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanSimplification;
Expand Down Expand Up @@ -369,7 +370,7 @@ public void testGenericNullableExpression() {
// comparison
assertNullLiteral(rule.rule(new GreaterThan(EMPTY, getFieldAttribute(), NULL)));
// regex
assertNullLiteral(rule.rule(new RLike(EMPTY, NULL, "123")));
assertNullLiteral(rule.rule(new RLike(EMPTY, NULL, new RLikePattern("123"))));
}

public void testNullFoldingOnCast() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,23 @@ private void assertDifferentRLikeAndNotRLikePatterns(String firstPattern, String
assertEquals("keyword", rqsq.field());
}

public void testLikeRLikeAsPainlessScripts() {
LogicalPlan p = plan("SELECT count(*), CASE WHEN keyword LIKE '%foo%' THEN 1 WHEN keyword RLIKE '.*bar.*' THEN 2 " +
"ELSE 3 END AS t FROM test GROUP BY t");
assertTrue(p instanceof Aggregate);
Expression condition = ((Aggregate) p).groupings().get(0);
assertFalse(condition.foldable());
GroupingContext groupingContext = QueryFolder.FoldAggregate.groupBy(((Aggregate) p).groupings());
assertNotNull(groupingContext);
ScriptTemplate scriptTemplate = groupingContext.tail.script();
assertEquals("InternalSqlScriptUtils.caseFunction([InternalSqlScriptUtils.regex(InternalSqlScriptUtils.docValue(" +
"doc,params.v0),params.v1),params.v2,InternalSqlScriptUtils.regex(InternalSqlScriptUtils.docValue(" +
"doc,params.v3),params.v4),params.v5,params.v6])",
scriptTemplate.toString());
assertEquals("[{v=keyword}, {v=^.*foo.*$}, {v=1}, {v=keyword}, {v=.*bar.*}, {v=2}, {v=3}]",
scriptTemplate.params().toString());
}

public void testTranslateNotExpression_WhereClause_Painless() {
LogicalPlan p = plan("SELECT * FROM test WHERE NOT(POSITION('x', keyword) = 0)");
assertTrue(p instanceof Project);
Expand Down

0 comments on commit eaa8ead

Please sign in to comment.