SQL: Introduce NotEquals node to simplify expressions (#35234)

Add NotEquals node in parser to simplify expressions so that <value1> != <value2> is no longer translated internally to NOT(<value1> = <value2>) Closes: #35210 Fixes: #35233
elastic · Nov 5, 2018 · 1c7c6de · 1c7c6de
1 parent 7db159c
commit 1c7c6de
Show file tree

Hide file tree

Showing 17 changed files with 169 additions and 44 deletions.
diff --git a/x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec b/x-pack/plugin/sql/qa/src/main/resources/agg.sql-spec
@@ -129,6 +129,10 @@ aggCountAndHaving
 SELECT gender g, COUNT(*) c FROM "test_emp" GROUP BY g HAVING COUNT(*) > 10 ORDER BY gender;
 aggCountAndHavingEquality
 SELECT gender g, COUNT(*) c FROM "test_emp" GROUP BY g HAVING COUNT(*) = 10 ORDER BY gender;
+aggCountAndHavingNotEquals
+SELECT gender g, COUNT(*) c FROM "test_emp" GROUP BY g HAVING COUNT(*) != 10 ORDER BY gender;
+aggCountAndHavingNegateEquality
+SELECT gender g, COUNT(*) c FROM "test_emp" GROUP BY g HAVING NOT COUNT(*) = 10 ORDER BY gender;
 aggCountOnColumnAndHaving
 SELECT gender g, COUNT(gender) c FROM "test_emp" GROUP BY g HAVING COUNT(gender) > 10 ORDER BY gender;
 aggCountOnColumnAndWildcardAndHaving

diff --git a/x-pack/plugin/sql/qa/src/main/resources/functions.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/functions.csv-spec
@@ -232,21 +232,13 @@ SELECT POSITION('x',LCASE("first_name")) pos, "first_name" FROM "test_emp" WHERE
       pos:i    |  first_name:s
 ---------------+---------------
 4              |Guoxiang       
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-null           |null           
-1              |Xinglin  
+1              |Xinglin
 ;
 
 selectPositionWithLcaseAndConditionWithGroupByAndOrderBy
-SELECT POSITION('m',LCASE("first_name")), COUNT(*) pos FROM "test_emp" WHERE POSITION('m',LCASE("first_name")) != 0 GROUP BY POSITION('m',LCASE("first_name")) ORDER BY POSITION('m',LCASE("first_name")) DESC;
+SELECT POSITION('m',LCASE("first_name")), COUNT(*) pos FROM "test_emp"
+  WHERE POSITION('m',LCASE("first_name")) != 0
+  GROUP BY POSITION('m',LCASE("first_name")) ORDER BY POSITION('m',LCASE("first_name")) DESC;
 
 POSITION(m,LCASE(first_name)):i|      pos:l
 -------------------------------+---------------
@@ -256,7 +248,6 @@ POSITION(m,LCASE(first_name)):i|      pos:l
 3                              |6              
 2                              |1              
 1                              |9              
-null                           |10    
 ;
 
 selectInsertWithPositionAndCondition

diff --git a/x-pack/plugin/sql/qa/src/main/resources/select.csv-spec b/x-pack/plugin/sql/qa/src/main/resources/select.csv-spec
@@ -1,4 +1,48 @@
+//
+// SELECT with = and !=
+//
+equalsSelectClause
+SELECT CAST(4 = 4 AS STRING), CAST(NOT 4 = 4 AS STRING), CAST(3 = 4 AS STRING), CAST(NOT 3 = 4 AS STRING), CAST(1 = null AS STRING), CAST(NOT null = 1 AS STRING);
+
+  CAST(4 == 4 AS VARCHAR):s |  CAST(NOT(4 == 4) AS VARCHAR):s |  CAST(3 == 4 AS VARCHAR):s |  CAST(NOT(3 == 4) AS VARCHAR):s |  CAST(1 == null AS VARCHAR):s |  CAST(NOT(null == 1) AS VARCHAR):s
+----------------------------+---------------------------------+----------------------------+---------------------------------+-------------------------------+-----------------------------------
+true                        |false                            |false                       |true                             |null                           |null
+;
+
+notEqualsSelectClause
+SELECT CAST(4 != 4 AS STRING), CAST(NOT 4 != 4 AS STRING), CAST(3 != 4 AS STRING), CAST(NOT 3 != 4 AS STRING), CAST(1 != null AS STRING), CAST(NOT 1 != null AS STRING);
+
+  CAST(4 != 4 AS VARCHAR):s |  CAST(NOT(4 != 4) AS VARCHAR):s |  CAST(3 != 4 AS VARCHAR):s |  CAST(NOT(3 != 4) AS VARCHAR):s |  CAST(1 != null AS VARCHAR):s |  CAST(NOT(1 != null) AS VARCHAR):s
+----------------------------+---------------------------------+----------------------------+---------------------------------+-------------------------------+-----------------------------------
+false                       |true                             |true                        |false                            |null                           |null
+;
+
+equalSelectClauseWithTableColumns
+SELECT CAST(languages = 2 AS STRING), CAST(NOT languages = 2 AS STRING), CAST(languages = null AS STRING), CAST(NOT languages = null AS STRING)
+FROM "test_emp" WHERE emp_no IN(10018, 10019, 10020) ORDER BY emp_no;
+
+  CAST((languages) == 2 AS VARCHAR):s |  CAST(NOT((languages) == 2) AS VARCHAR):s |  CAST((languages) == null AS VARCHAR):s |  CAST(NOT((languages) == null) AS VARCHAR):s
+--------------------------------------+-------------------------------------------+-----------------------------------------+---------------------------------------------
+true                                  |false                                      |null                                     |null
+false                                 |true                                       |null                                     |null
+null                                  |null                                       |null                                     |null
+;
+
+notEqualsAndNotEqualsSelectClauseWithTableColumns
+SELECT CAST(languages != 2 AS STRING), CAST(NOT languages != 2 AS STRING), CAST(languages != null AS STRING), CAST(NOT languages != null AS STRING)
+FROM "test_emp" WHERE emp_no IN(10018, 10019, 10020) ORDER BY emp_no;
+
+  CAST((languages) != 2 AS VARCHAR):s |  CAST(NOT((languages) != 2) AS VARCHAR):s |  CAST((languages) != null AS VARCHAR):s |  CAST(NOT((languages) != null) AS VARCHAR):s
+--------------------------------------+-------------------------------------------+-----------------------------------------+---------------------------------------------
+false                                 |true                                       |null                                     |null
+true                                  |false                                      |null                                     |null
+null                                  |null                                       |null                                     |null
+;
+
+
+//
 // SELECT with IN
+//
 inWithLiterals
 SELECT 1 IN (1, 2, 3), 1 IN (2, 3);
 

diff --git a/.../elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java b/.../elasticsearch/xpack/sql/expression/function/scalar/whitelist/InternalSqlScriptUtils.java
@@ -40,6 +40,7 @@
  * Acts as a registry of the various static methods used <b>internally</b> by the scalar functions
  * (to simplify the whitelist definition).
  */
+@SuppressWarnings("unused")
 public final class InternalSqlScriptUtils {
 
     private InternalSqlScriptUtils() {}
@@ -52,7 +53,7 @@ private InternalSqlScriptUtils() {}
     public static <T> Object docValue(Map<String, ScriptDocValues<T>> doc, String fieldName) {
         if (doc.containsKey(fieldName)) {
             ScriptDocValues<T> docValues = doc.get(fieldName);
-            if (docValues.size() > 0) {
+            if (!docValues.isEmpty()) {
                 return docValues.get(0);
             }
         }
@@ -83,6 +84,10 @@ public static Boolean eq(Object left, Object right) {
         return BinaryComparisonOperation.EQ.apply(left, right);
     }
 
+    public static Boolean neq(Object left, Object right) {
+        return BinaryComparisonOperation.NEQ.apply(left, right);
+    }
+
     public static Boolean lt(Object left, Object right) {
         return BinaryComparisonOperation.LT.apply(left, right);
     }

diff --git a/...csearch/xpack/sql/expression/predicate/operator/comparison/BinaryComparisonProcessor.java b/...csearch/xpack/sql/expression/predicate/operator/comparison/BinaryComparisonProcessor.java
@@ -19,6 +19,7 @@ public class BinaryComparisonProcessor extends FunctionalBinaryProcessor<Object,
     public enum BinaryComparisonOperation implements PredicateBiFunction<Object, Object, Boolean> {
 
         EQ(Comparisons::eq, "=="),
+        NEQ(Comparisons::neq, "!="),
         GT(Comparisons::gt, ">"),
         GTE(Comparisons::gte, ">="),
         LT(Comparisons::lt, "<"),
@@ -62,4 +63,4 @@ public BinaryComparisonProcessor(StreamInput in) throws IOException {
     public String getWriteableName() {
         return NAME;
     }
-}
+}
diff --git a/...ava/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/Comparisons.java b/...ava/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/Comparisons.java
@@ -19,6 +19,11 @@ public static Boolean eq(Object l, Object r) {
         return i == null ? null : i.intValue() == 0;
     }
 
+    static Boolean neq(Object l, Object r) {
+        Integer i = compare(l, r);
+        return i == null ? null : i.intValue() != 0;
+    }
+
     static Boolean lt(Object l, Object r) {
         Integer i = compare(l, r);
         return i == null ? null : i.intValue() < 0;
@@ -50,6 +55,9 @@ static Boolean in(Object l, Set<Object> r) {
      */
     @SuppressWarnings({ "rawtypes", "unchecked" })
     static Integer compare(Object l, Object r) {
+        if (l == null || r == null) {
+            return null;
+        }
         // typical number comparison
         if (l instanceof Number && r instanceof Number) {
             return compare((Number) l, (Number) r);

diff --git a/...ain/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/Equals.java b/...ain/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/Equals.java
@@ -6,11 +6,12 @@
 package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison;
 
 import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparisonProcessor.BinaryComparisonOperation;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 
-public class Equals extends BinaryComparison {
+public class Equals extends BinaryComparison implements BinaryOperator.Negateable {
 
     public Equals(Location location, Expression left, Expression right) {
         super(location, left, right, BinaryComparisonOperation.EQ);
@@ -30,4 +31,9 @@ protected Equals replaceChildren(Expression newLeft, Expression newRight) {
     public Equals swapLeftAndRight() {
         return new Equals(location(), right(), left());
     }
+
+    @Override
+    public BinaryOperator<?, ?, ?, ?> negate() {
+        return new NotEquals(location(), left(), right());
+    }
 }
diff --git a/.../java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/NotEquals.java b/.../java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/NotEquals.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison;
+
+import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator;
+import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparisonProcessor.BinaryComparisonOperation;
+import org.elasticsearch.xpack.sql.tree.Location;
+import org.elasticsearch.xpack.sql.tree.NodeInfo;
+
+public class NotEquals extends BinaryComparison implements BinaryOperator.Negateable {
+
+    public NotEquals(Location location, Expression left, Expression right) {
+        super(location, left, right, BinaryComparisonOperation.NEQ);
+    }
+
+    @Override
+    protected NodeInfo<NotEquals> info() {
+        return NodeInfo.create(this, NotEquals::new, left(), right());
+    }
+
+    @Override
+    protected NotEquals replaceChildren(Expression newLeft, Expression newRight) {
+        return new NotEquals(location(), newLeft, newRight);
+    }
+
+    @Override
+    public NotEquals swapLeftAndRight() {
+        return new NotEquals(location(), right(), left());
+    }
+
+    @Override
+    public BinaryOperator<?, ?, ?, ?> negate() {
+        return new Equals(location(), left(), right());
+    }
+}
diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java
@@ -52,6 +52,7 @@
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThan;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThanOrEqual;
+import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.NotEquals;
 import org.elasticsearch.xpack.sql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.sql.plan.logical.EsRelation;
 import org.elasticsearch.xpack.sql.plan.logical.Filter;
@@ -1313,7 +1314,7 @@ private Expression simplify(BinaryComparison bc) {
             }
 
             // false for equality
-            if (bc instanceof GreaterThan || bc instanceof LessThan) {
+            if (bc instanceof NotEquals || bc instanceof GreaterThan || bc instanceof LessThan) {
                 if (!l.nullable() && !r.nullable() && l.semanticEquals(r)) {
                     return FALSE;
                 }

diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java
@@ -44,6 +44,7 @@
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThanOrEqual;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThan;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThanOrEqual;
+import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.NotEquals;
 import org.elasticsearch.xpack.sql.expression.predicate.regex.Like;
 import org.elasticsearch.xpack.sql.expression.predicate.regex.LikePattern;
 import org.elasticsearch.xpack.sql.expression.predicate.regex.RLike;
@@ -165,7 +166,7 @@ public Expression visitComparison(ComparisonContext ctx) {
             case SqlBaseParser.EQ:
                 return new Equals(loc, left, right);
             case SqlBaseParser.NEQ:
-                return new Not(loc, new Equals(loc, left, right));
+                return new NotEquals(loc, left, right);
             case SqlBaseParser.LT:
                 return new LessThan(loc, left, right);
             case SqlBaseParser.LTE:

diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java
@@ -31,7 +31,6 @@
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFunction;
 import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeHistogramFunction;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
-import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In;
 import org.elasticsearch.xpack.sql.expression.predicate.IsNotNull;
 import org.elasticsearch.xpack.sql.expression.predicate.Range;
 import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate;
@@ -44,8 +43,10 @@
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThan;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThanOrEqual;
+import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThan;
 import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.LessThanOrEqual;
+import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.NotEquals;
 import org.elasticsearch.xpack.sql.expression.predicate.regex.Like;
 import org.elasticsearch.xpack.sql.expression.predicate.regex.LikePattern;
 import org.elasticsearch.xpack.sql.expression.predicate.regex.RLike;
@@ -536,16 +537,15 @@ protected QueryTranslation asQuery(BinaryComparison bc, boolean onAggs) {
                 //
                 // Agg context means HAVING -> PipelineAggs
                 //
-                ScriptTemplate script = bc.asScript();
                 if (onAggs) {
-                    aggFilter = new AggFilter(at.id().toString(), script);
+                    aggFilter = new AggFilter(at.id().toString(), bc.asScript());
                 }
                 else {
                     // query directly on the field
                     if (at instanceof FieldAttribute) {
                         query = wrapIfNested(translateQuery(bc), ne);
                     } else {
-                        query = new ScriptQuery(at.location(), script);
+                        query = new ScriptQuery(at.location(), bc.asScript());
                     }
                 }
                 return new QueryTranslation(query, aggFilter);
@@ -576,7 +576,7 @@ private static Query translateQuery(BinaryComparison bc) {
             if (bc instanceof LessThanOrEqual) {
                 return new RangeQuery(loc, name, null, false, value, true, format);
             }
-            if (bc instanceof Equals) {
+            if (bc instanceof Equals || bc instanceof NotEquals) {
                 if (bc.left() instanceof FieldAttribute) {
                     FieldAttribute fa = (FieldAttribute) bc.left();
                     // equality should always be against an exact match
@@ -585,7 +585,11 @@ private static Query translateQuery(BinaryComparison bc) {
                         name = fa.exactAttribute().name();
                     }
                 }
-                return new TermQuery(loc, name, value);
+                Query query = new TermQuery(loc, name, value);
+                if (bc instanceof NotEquals) {
+                    query = new NotQuery(loc, query);
+                }
+                return query;
             }
 
             throw new SqlIllegalArgumentException("Don't know how to translate binary comparison [{}] in [{}]", bc.right().nodeString(),
@@ -655,11 +659,10 @@ protected QueryTranslation asQuery(Range r, boolean onAggs) {
                 //
                 // Agg context means HAVING -> PipelineAggs
                 //
-                ScriptTemplate script = r.asScript();
                 Attribute at = ((NamedExpression) e).toAttribute();
 
                 if (onAggs) {
-                    aggFilter = new AggFilter(at.id().toString(), script);
+                    aggFilter = new AggFilter(at.id().toString(), r.asScript());
                 } else {
                     // typical range; no scripting involved
                     if (at instanceof FieldAttribute) {
@@ -669,7 +672,7 @@ protected QueryTranslation asQuery(Range r, boolean onAggs) {
                     }
                     // scripted query
                     else {
-                        query = new ScriptQuery(at.location(), script);
+                        query = new ScriptQuery(at.location(), r.asScript());
                     }
                 }
                 return new QueryTranslation(query, aggFilter);

diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/query/NotQuery.java
@@ -9,10 +9,10 @@
 import org.elasticsearch.search.sort.NestedSortBuilder;
 import org.elasticsearch.xpack.sql.tree.Location;
 
-import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
-
 import java.util.Objects;
 
+import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
+
 public class NotQuery extends Query {
     private final Query child;
 

diff --git a/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt b/x-pack/plugin/sql/src/main/resources/org/elasticsearch/xpack/sql/plugin/sql_whitelist.txt
@@ -20,6 +20,7 @@ class org.elasticsearch.xpack.sql.expression.function.scalar.whitelist.InternalS
 # Comparison
 #
   Boolean eq(Object, Object)
+  Boolean neq(Object, Object)
   Boolean lt(Object, Object)
   Boolean lte(Object, Object)
   Boolean gt(Object, Object)