From 32746fdaf15b71d088aeeb9da0d68c87ea641040 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Fri, 6 Jul 2018 12:46:57 +0300 Subject: [PATCH] SQL: Fix incorrect HAVING equality (#31820) Fix bug that causes `HAVING a = b` to be translated ad-litteram in Painless which uses `==` for equality checks not `=`. Close #31796 (cherry picked from commit 89cb0872cfcee7a715bc243b4b83ffc6f3d67239) --- .../elasticsearch/xpack/sql/expression/predicate/Equals.java | 3 ++- .../elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java | 2 +- x-pack/qa/sql/src/main/resources/agg.sql-spec | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Equals.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Equals.java index 90bffebecd067..a5b3272d7cc63 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Equals.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Equals.java @@ -27,6 +27,7 @@ protected Equals replaceChildren(Expression newLeft, Expression newRight) { return new Equals(location(), newLeft, newRight); } + @Override public Object fold() { return Objects.equals(left().fold(), right().fold()); } @@ -38,6 +39,6 @@ public Equals swapLeftAndRight() { @Override public String symbol() { - return "="; + return "=="; } } diff --git a/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java b/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java index 63831c2d4dec0..cbf6d0d476e57 100644 --- a/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java +++ b/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java @@ -65,7 +65,7 @@ public void testExplainWithWhere() throws IOException { assertThat(readLine(), startsWith("----------")); assertThat(readLine(), startsWith("With[{}]")); assertThat(readLine(), startsWith("\\_Project[[?*]]")); - assertThat(readLine(), startsWith(" \\_Filter[?i = 2]")); + assertThat(readLine(), startsWith(" \\_Filter[?i == 2]")); assertThat(readLine(), startsWith(" \\_UnresolvedRelation[[][index=test],null,Unknown index [test]]")); assertEquals("", readLine()); diff --git a/x-pack/qa/sql/src/main/resources/agg.sql-spec b/x-pack/qa/sql/src/main/resources/agg.sql-spec index f778458dfe2bf..3e1c38c337dad 100644 --- a/x-pack/qa/sql/src/main/resources/agg.sql-spec +++ b/x-pack/qa/sql/src/main/resources/agg.sql-spec @@ -80,6 +80,8 @@ SELECT COUNT(DISTINCT hire_date) AS count FROM test_emp; // Conditional COUNT 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; aggCountOnColumnAndHaving SELECT gender g, COUNT(gender) c FROM "test_emp" GROUP BY g HAVING COUNT(gender) > 10 ORDER BY gender; // NOT supported yet since Having introduces a new agg @@ -91,6 +93,8 @@ aggCountOnColumnAndHavingOnAlias SELECT gender g, COUNT(gender) c FROM "test_emp" GROUP BY g HAVING c > 10 ORDER BY gender; aggCountOnColumnAndMultipleHaving SELECT gender g, COUNT(gender) c FROM "test_emp" GROUP BY g HAVING c > 10 AND c < 70 ORDER BY gender ; +aggCountOnColumnAndMultipleHavingEquals +SELECT gender g, COUNT(gender) c FROM "test_emp" GROUP BY g HAVING c > 10 AND c = 63 ORDER BY gender ; aggCountOnColumnAndMultipleHavingWithLimit SELECT gender g, COUNT(gender) c FROM "test_emp" GROUP BY g HAVING c > 10 AND c < 70 ORDER BY gender LIMIT 1; aggCountOnColumnAndHavingBetween @@ -145,6 +149,7 @@ SELECT gender g, MIN(emp_no) m FROM "test_emp" GROUP BY g HAVING m BETWEEN 10 AN aggMinWithMultipleHavingOnAliasAndFunction SELECT gender g, MIN(emp_no) m FROM "test_emp" GROUP BY g HAVING m > 10 AND MIN(emp_no) < 99999 ORDER BY gender; + // MAX aggMaxImplicit // tag::max