Skip to content

SQL: fix LIKE function equality by considering its pattern as well #40260

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 5 commits into from
Mar 21, 2019
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
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