Skip to content

SQL: Fix translation of LIKE/RLIKE keywords #36672

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

Merged
merged 3 commits into from
Dec 17, 2018
Merged
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
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