Skip to content

Commit

Permalink
SQL: fix LIKE function equality by considering its pattern as well (#…
Browse files Browse the repository at this point in the history
…40260)

* Define a equals method for Like function so that the pattern used
is considered in the equality check. Whenever the functions are resolved
this check should be used.

(cherry picked from commit 4e5d5af)
  • Loading branch information
astefan committed Mar 21, 2019
1 parent 9ba9b17 commit 67a0116
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 5 deletions.
3 changes: 3 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 @@ -64,6 +64,9 @@ SELECT last_name l FROM "test_emp" WHERE NOT (emp_no < 10003 AND last_name NOT L
whereFieldOnMatchWithAndAndOr
SELECT last_name l FROM "test_emp" WHERE emp_no < 10003 AND (gender = 'M' AND NOT FALSE OR last_name LIKE 'K%') ORDER BY emp_no;

whereFieldWithLikeAndNotLike
SELECT COUNT(*), last_name AS f FROM test_emp WHERE last_name LIKE '%o%' AND last_name NOT LIKE '%f%' GROUP BY f HAVING COUNT(*) > 1;

// TODO: (NOT) RLIKE in particular and more NOT queries in general

whereIsNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.elasticsearch.xpack.sql.expression.function.aggregate.Count;
import org.elasticsearch.xpack.sql.expression.function.scalar.Cast;
import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.ArithmeticOperation;
import org.elasticsearch.xpack.sql.expression.predicate.regex.Like;
import org.elasticsearch.xpack.sql.plan.TableIdentifier;
import org.elasticsearch.xpack.sql.plan.logical.Aggregate;
import org.elasticsearch.xpack.sql.plan.logical.EsRelation;
Expand Down Expand Up @@ -848,9 +849,11 @@ private Expression collectResolvedAndReplace(Expression e, Map<String, List<Func
List<Function> list = getList(seen, fName);
for (Function seenFunction : list) {
if (seenFunction != f && f.arguments().equals(seenFunction.arguments())) {
// TODO: we should move to always compare the functions directly
// Special check for COUNT: an already seen COUNT function will be returned only if its DISTINCT property
// matches the one from the unresolved function to be checked.
if (seenFunction instanceof Count) {
// Same for LIKE: the equals function also compares the pattern of LIKE
if (seenFunction instanceof Count || seenFunction instanceof Like) {
if (seenFunction.equals(f)){
return seenFunction;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
package org.elasticsearch.xpack.sql.expression.predicate.regex;

import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.tree.Source;

import java.util.Objects;

public class Like extends RegexMatch {

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

@Override
public boolean equals(Object obj) {
return super.equals(obj) && Objects.equals(((Like) obj).pattern(), pattern());
}

@Override
public int hashCode() {
return Objects.hash(super.hashCode(), pattern());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ public QueryBuilder asBuilder() {
return boolQuery;
}

boolean isAnd() {
public boolean isAnd() {
return isAnd;
}

Query left() {
public Query left() {
return left;
}

Query right() {
public Query right() {
return right;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.xpack.sql.planner.QueryTranslator.QueryTranslation;
import org.elasticsearch.xpack.sql.querydsl.agg.AggFilter;
import org.elasticsearch.xpack.sql.querydsl.agg.GroupByDateHistogram;
import org.elasticsearch.xpack.sql.querydsl.query.BoolQuery;
import org.elasticsearch.xpack.sql.querydsl.query.ExistsQuery;
import org.elasticsearch.xpack.sql.querydsl.query.NotQuery;
import org.elasticsearch.xpack.sql.querydsl.query.Query;
Expand Down Expand Up @@ -198,6 +199,33 @@ public void testLikeConstructsNotSupported() {
SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false));
assertEquals("Scalar function (LTRIM(keyword)) not allowed (yet) as arguments for LIKE", ex.getMessage());
}

public void testDifferentLikeAndNotLikePatterns() {
LogicalPlan p = plan("SELECT keyword k FROM test WHERE k LIKE 'X%' AND k NOT LIKE 'Y%'");
assertTrue(p instanceof Project);
p = ((Project) p).child();
assertTrue(p instanceof Filter);

Expression condition = ((Filter) p).condition();
QueryTranslation qt = QueryTranslator.toQuery(condition, false);
assertEquals(BoolQuery.class, qt.query.getClass());
BoolQuery bq = ((BoolQuery) qt.query);
assertTrue(bq.isAnd());
assertTrue(bq.left() instanceof QueryStringQuery);
assertTrue(bq.right() instanceof NotQuery);

NotQuery nq = (NotQuery) bq.right();
assertTrue(nq.child() instanceof QueryStringQuery);
QueryStringQuery lqsq = (QueryStringQuery) bq.left();
QueryStringQuery rqsq = (QueryStringQuery) nq.child();

assertEquals("X*", lqsq.query());
assertEquals(1, lqsq.fields().size());
assertEquals("keyword", lqsq.fields().keySet().iterator().next());
assertEquals("Y*", rqsq.query());
assertEquals(1, rqsq.fields().size());
assertEquals("keyword", rqsq.fields().keySet().iterator().next());
}

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

0 comments on commit 67a0116

Please sign in to comment.