Skip to content

Commit

Permalink
SQL: Fix translation of LIKE/RLIKE keywords (#36672)
Browse files Browse the repository at this point in the history
* SQL: Fix translation of LIKE/RLIKE keywords

Refactor Like/RLike functions to simplify internals and improve query
 translation when chained or within a script context.

Fix #36039
Fix #36584

(cherry picked from commit 6d9d5e3)
  • Loading branch information
costin committed Dec 17, 2018
1 parent 59d2525 commit de8e3c4
Show file tree
Hide file tree
Showing 14 changed files with 158 additions and 154 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ aggMaxWithAlias
SELECT gender g, MAX(emp_no) m FROM "test_emp" GROUP BY g ORDER BY gender;
aggMaxOnDate
SELECT gender, MAX(birth_date) m FROM "test_emp" GROUP BY gender ORDER BY gender;
aggAvgAndMaxWithLikeFilter
SELECT CAST(AVG(salary) AS LONG) AS avg, CAST(SUM(salary) AS LONG) AS s FROM "test_emp" WHERE first_name LIKE 'G%';

// Conditional MAX
aggMaxWithHaving
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugin/sql/qa/src/main/resources/datetime.sql-spec
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ SELECT DAY_OF_WEEK(birth_date) day, COUNT(*) c FROM test_emp WHERE DAY_OF_WEEK(b
currentTimestampYear
SELECT YEAR(CURRENT_TIMESTAMP()) AS result;

currentTimestampMonth
//
// H2 uses the local timezone instead of the specified one
//
currentTimestampMonth-Ignore
SELECT MONTH(CURRENT_TIMESTAMP()) AS result;

currentTimestampHour-Ignore
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugin/sql/qa/src/main/resources/filter.sql-spec
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ whereFieldWithNotEqualsOnString
SELECT last_name l FROM "test_emp" WHERE emp_no < 10003 AND gender <> 'M';
whereFieldWithLikeMatch
SELECT last_name l FROM "test_emp" WHERE emp_no < 10003 AND last_name LIKE 'K%';
whereFieldWithNotLikeMatch
SELECT last_name l FROM "test_emp" WHERE emp_no < 10020 AND first_name NOT LIKE 'Ma%';

whereFieldWithOrderNot
SELECT last_name l FROM "test_emp" WHERE NOT emp_no < 10003 ORDER BY emp_no LIMIT 5;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ public static Object nullif(Object left, Object right) {
// Regex
//
public static Boolean regex(String value, String pattern) {
// TODO: this needs to be improved to avoid creating the pattern on every call
return RegexOperation.match(value, pattern);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,24 @@

public class Like extends RegexMatch {

public Like(Location location, Expression left, LikePattern right) {
super(location, left, right);
}
private final LikePattern pattern;

@Override
protected NodeInfo<Like> info() {
return NodeInfo.create(this, Like::new, left(), pattern());
public Like(Location location, Expression left, LikePattern pattern) {
super(location, left, pattern.asJavaRegex());
this.pattern = pattern;
}

public LikePattern pattern() {
return (LikePattern) right();
return pattern;
}

@Override
protected Like replaceChildren(Expression newLeft, Expression newRight) {
return new Like(location(), newLeft, (LikePattern) newRight);
protected NodeInfo<Like> info() {
return NodeInfo.create(this, Like::new, field(), pattern);
}

@Override
protected String asString(Expression pattern) {
return ((LikePattern) pattern).asJavaRegex();
protected Like replaceChild(Expression newLeft) {
return new Like(location(), newLeft, pattern);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
*/
package org.elasticsearch.xpack.sql.expression.predicate.regex;

import org.elasticsearch.xpack.sql.expression.LeafExpression;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.util.StringUtils;

import java.util.Objects;
Expand All @@ -21,7 +17,7 @@
*
* To prevent conflicts with ES, the string and char must be validated to not contain '*'.
*/
public class LikePattern extends LeafExpression {
public class LikePattern {

private final String pattern;
private final char escape;
Expand All @@ -30,8 +26,7 @@ public class LikePattern extends LeafExpression {
private final String wildcard;
private final String indexNameWildcard;

public LikePattern(Location location, String pattern, char escape) {
super(location);
public LikePattern(String pattern, char escape) {
this.pattern = pattern;
this.escape = escape;
// early initialization to force string validation
Expand All @@ -40,11 +35,6 @@ public LikePattern(Location location, String pattern, char escape) {
this.indexNameWildcard = StringUtils.likeToIndexWildcard(pattern, escape);
}

@Override
protected NodeInfo<LikePattern> info() {
return NodeInfo.create(this, LikePattern::new, pattern, escape);
}

public String pattern() {
return pattern;
}
Expand Down Expand Up @@ -74,16 +64,6 @@ public String asIndexNameWildcard() {
return indexNameWildcard;
}

@Override
public boolean nullable() {
return false;
}

@Override
public DataType dataType() {
return DataType.KEYWORD;
}

@Override
public int hashCode() {
return Objects.hash(pattern, escape);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,29 @@
package org.elasticsearch.xpack.sql.expression.predicate.regex;

import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Literal;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.tree.NodeInfo;

public class RLike extends RegexMatch {

public RLike(Location location, Expression left, Literal right) {
super(location, left, right);
private final String pattern;

public RLike(Location location, Expression left, String pattern) {
super(location, left, pattern);
this.pattern = pattern;
}

@Override
protected NodeInfo<RLike> info() {
return NodeInfo.create(this, RLike::new, left(), (Literal) right());
public String pattern() {
return pattern;
}

@Override
protected RLike replaceChildren(Expression newLeft, Expression newRight) {
return new RLike(location(), newLeft, (Literal) newRight);
protected NodeInfo<RLike> info() {
return NodeInfo.create(this, RLike::new, field(), pattern);
}

@Override
protected String asString(Expression pattern) {
return pattern.fold().toString();
protected RLike replaceChild(Expression newChild) {
return new RLike(location(), newChild, pattern);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,45 @@
package org.elasticsearch.xpack.sql.expression.predicate.regex;

import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.predicate.BinaryPredicate;
import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction;
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
import org.elasticsearch.xpack.sql.expression.predicate.regex.RegexProcessor.RegexOperation;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.type.DataType;

public abstract class RegexMatch extends BinaryPredicate<String, String, Boolean, RegexOperation> {
public abstract class RegexMatch extends UnaryScalarFunction {

protected RegexMatch(Location location, Expression value, Expression pattern) {
super(location, value, pattern, RegexOperation.INSTANCE);
private final String pattern;

protected RegexMatch(Location location, Expression value, String pattern) {
super(location, value);
this.pattern = pattern;
}

@Override
public DataType dataType() {
return DataType.BOOLEAN;
}

@Override
public boolean nullable() {
return field().nullable() && pattern != null;
}

@Override
public boolean foldable() {
// right() is not directly foldable in any context but Like can fold it.
return left().foldable();
return field().foldable();
}

@Override
public Boolean fold() {
Object val = left().fold();
val = val != null ? val.toString() : val;
return function().apply((String) val, asString(right()));
Object val = field().fold();
return RegexOperation.match(val, pattern);
}

protected abstract String asString(Expression pattern);
@Override
protected Processor makeProcessor() {
return new RegexProcessor(pattern);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,79 +7,71 @@

import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.gen.processor.BinaryProcessor;
import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
import org.elasticsearch.xpack.sql.expression.predicate.PredicateBiFunction;

import java.io.IOException;
import java.util.Objects;
import java.util.regex.Pattern;

public class RegexProcessor extends BinaryProcessor {
public class RegexProcessor implements Processor {

public static class RegexOperation implements PredicateBiFunction<String, String, Boolean> {
public static class RegexOperation {

public static final RegexOperation INSTANCE = new RegexOperation();
public static Boolean match(Object value, Pattern pattern) {
if (pattern == null) {
return Boolean.TRUE;
}

@Override
public String name() {
return symbol();
}
if (value == null) {
return null;
}

@Override
public String symbol() {
return "REGEX";
return pattern.matcher(value.toString()).matches();
}

@Override
public Boolean doApply(String value, String pattern) {
return match(value, pattern);
}
public static Boolean match(Object value, String pattern) {
if (pattern == null) {
return Boolean.TRUE;
}

public static Boolean match(Object value, Object pattern) {
if (value == null || pattern == null) {
if (value == null) {
return null;
}

Pattern p = Pattern.compile(pattern.toString());
return p.matcher(value.toString()).matches();
return Pattern.compile(pattern).matcher(value.toString()).matches();
}
}

public static final String NAME = "rgx";

public RegexProcessor(Processor value, Processor pattern) {
super(value, pattern);
}
private Pattern pattern;

public RegexProcessor(StreamInput in) throws IOException {
super(in);
public RegexProcessor(String pattern) {
this.pattern = pattern != null ? Pattern.compile(pattern) : null;
}

@Override
protected Boolean doProcess(Object value, Object pattern) {
return RegexOperation.match(value, pattern);
public String getWriteableName() {
return NAME;
}

@Override
protected void checkParameter(Object param) {
if (!(param instanceof String || param instanceof Character)) {
throw new SqlIllegalArgumentException("A string/char is required; received [{}]", param);
}
public RegexProcessor(StreamInput in) throws IOException {
this(in.readOptionalString());
}

@Override
public String getWriteableName() {
return NAME;
public void writeTo(StreamOutput out) throws IOException {
out.writeOptionalString(pattern != null ? pattern.toString() : null);
}

@Override
protected void doWrite(StreamOutput out) throws IOException {}
public Object process(Object input) {
return RegexOperation.match(input, pattern);
}

@Override
public int hashCode() {
return Objects.hash(left(), right());
return Objects.hash(pattern);
}

@Override
Expand All @@ -93,6 +85,6 @@ public boolean equals(Object obj) {
}

RegexProcessor other = (RegexProcessor) obj;
return Objects.equals(left(), other.left()) && Objects.equals(right(), other.right());
return Objects.equals(pattern, other.pattern);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public Expression visitPredicated(PredicatedContext ctx) {
e = new Like(loc, exp, visitPattern(pCtx.pattern()));
break;
case SqlBaseParser.RLIKE:
e = new RLike(loc, exp, new Literal(source(pCtx.regex), string(pCtx.regex), DataType.KEYWORD));
e = new RLike(loc, exp, 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 Expand Up @@ -301,7 +301,7 @@ public LikePattern visitPattern(PatternContext ctx) {
}
}

return new LikePattern(source(ctx), pattern, escape);
return new LikePattern(pattern, escape);
}


Expand Down
Loading

0 comments on commit de8e3c4

Please sign in to comment.