add fix to handle functions applied to literal (#913)

* add functions with literal

* address PR comments

* remove unnecessary check. anyMatch returns false for empty list.

* remove isNonLiteral()

Co-authored-by: Rupal Mahajan <>

integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java

@@ -60,6 +60,15 @@ public void testDateInGroupBy() throws IOException{
             rows("2018-08-11"));
   }
 
+  @Test
+  public void testDateWithHavingClauseOnly() throws IOException {
+    JSONObject result =
+            executeQuery(String.format("SELECT (TO_DAYS(DATE('2050-01-01')) - 693961) FROM %s HAVING (COUNT(1) > 0)",TEST_INDEX_BANK) );
+    verifySchema(result,
+            schema("(TO_DAYS(DATE('2050-01-01')) - 693961)", null, "long"));
+    verifyDataRows(result, rows(54787));
+  }
+
   @Test
   public void testAddDate() throws IOException {
     JSONObject result =

integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/MathematicalFunctionIT.java

@@ -15,6 +15,7 @@
 
 package com.amazon.opendistroforelasticsearch.sql.sql;
 
+import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK;
 import static com.amazon.opendistroforelasticsearch.sql.legacy.plugin.RestSqlAction.QUERY_API_ENDPOINT;
 import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.rows;
 import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.schema;
@@ -38,6 +39,16 @@ public class MathematicalFunctionIT extends SQLIntegTestCase {
   public void init() throws Exception {
     super.init();
     TestUtils.enableNewQueryEngine(client());
+    loadIndex(Index.BANK);
+  }
+
+  @Test
+  public void testPI() throws IOException {
+    JSONObject result =
+            executeQuery(String.format("SELECT PI() FROM %s HAVING (COUNT(1) > 0)",TEST_INDEX_BANK) );
+    verifySchema(result,
+            schema("PI()", null, "double"));
+    verifyDataRows(result, rows(3.141592653589793));
   }
 
   @Test

integ-test/src/test/resources/correctness/bugfixes/899.txt

@@ -0,0 +1,4 @@
+SELECT 'hello' FROM kibana_sample_data_flights HAVING (COUNT(1) > 0)
+SELECT UPPER('hello') AS `literal` FROM kibana_sample_data_flights HAVING (COUNT(1) > 0)
+SELECT UPPER(UPPER('hello')) AS `literal` FROM kibana_sample_data_flights HAVING (COUNT(1) > 0)
+SELECT SUBSTRING(CONCAT(UPPER('hello'), 'world'), 1, 6) AS `literal` FROM kibana_sample_data_flights HAVING (COUNT(1) > 0)

sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstAggregationBuilder.java

@@ -21,6 +21,7 @@
 import com.amazon.opendistroforelasticsearch.sql.ast.Node;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.AggregateFunction;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Alias;
+import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.Literal;
 import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression;
 import com.amazon.opendistroforelasticsearch.sql.ast.tree.Aggregation;
@@ -124,17 +125,26 @@ private List<UnresolvedExpression> replaceGroupByItemIfAliasOrOrdinal() {
    */
   private Optional<UnresolvedExpression> findNonAggregatedItemInSelect() {
     return querySpec.getSelectItems().stream()
-                                     .filter(this::isNonLiteral)
                                      .filter(this::isNonAggregatedExpression)
+                                     .filter(this::isNonLiteralFunction)
                                      .findFirst();
   }
 
   private boolean isAggregatorNotFoundAnywhere() {
     return querySpec.getAggregators().isEmpty();
   }
 
-  private boolean isNonLiteral(UnresolvedExpression expr) {
-    return !(expr instanceof Literal);
+  private boolean isNonLiteralFunction(UnresolvedExpression expr) {
+    // The base case for recursion
+    if (expr instanceof Literal) {
+      return false;
+    }
+    if (expr instanceof Function) {
+      List<? extends Node> children = expr.getChild();
+      return children.stream().anyMatch(child ->
+              isNonLiteralFunction((UnresolvedExpression) child));
+    }
+    return true;
   }
 
   private boolean isNonAggregatedExpression(UnresolvedExpression expr) {

sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstAggregationBuilderTest.java

@@ -112,6 +112,27 @@ void can_build_implicit_group_by_for_aggregator_in_having_clause() {
             hasGroupByItems(),
             hasAggregators(
                 alias("AVG(age)", aggregate("AVG", qualifiedName("age"))))));
+
+    assertThat(
+            buildAggregation("SELECT PI() FROM test HAVING AVG(age) > 30"),
+            allOf(
+                    hasGroupByItems(),
+                    hasAggregators(
+                            alias("AVG(age)", aggregate("AVG", qualifiedName("age"))))));
+
+    assertThat(
+            buildAggregation("SELECT ABS(1.5) FROM test HAVING AVG(age) > 30"),
+            allOf(
+                    hasGroupByItems(),
+                    hasAggregators(
+                            alias("AVG(age)", aggregate("AVG", qualifiedName("age"))))));
+
+    assertThat(
+            buildAggregation("SELECT ABS(ABS(1.5)) FROM test HAVING AVG(age) > 30"),
+            allOf(
+                    hasGroupByItems(),
+                    hasAggregators(
+                            alias("AVG(age)", aggregate("AVG", qualifiedName("age"))))));
   }
 
   @Test