SQL: fix LIKE function equality by considering its pattern as well (#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.

Author: astefan

x-pack/plugin/sql/qa/src/main/resources/filter.sql-spec

@@ -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

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

@@ -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;
@@ -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;
                             }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/regex/Like.java

@@ -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 {
 
@@ -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());
+    }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/BoolQuery.java

@@ -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;
     }
 

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

@@ -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;
@@ -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)");