Implemented "greatest" and "least" functions in old engine

opendistro-for-elasticsearch · Aug 22, 2021 · 83107e0 · 83107e0
1 parent 953ad26
commit 83107e0
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 37 deletions.
diff --git a/...g-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLFunctionsIT.java b/...g-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/legacy/SQLFunctionsIT.java
@@ -884,13 +884,13 @@ public void literalMultiField() throws Exception {
   public void greatestWithAliasAndStrings() throws Exception {
     JSONObject response =
             executeJdbcRequest("SELECT greatest(firstname, lastname) AS max " +
-            "FROM " + TEST_INDEX_ACCOUNT + " LIMIT 3");
+                    "FROM " + TEST_INDEX_ACCOUNT + " LIMIT 3");
 
     verifyDataRows(response,
             rows("duke"),
             rows("hattie"),
             rows("nanette"));
-  }
+    }
 
   private SearchHits query(String query) throws IOException {
     final String rsp = executeQueryWithStringOutput(query);

diff --git a/...cy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/utils/SQLFunctions.java b/...cy/src/main/java/com/amazon/opendistroforelasticsearch/sql/legacy/utils/SQLFunctions.java
@@ -606,51 +606,52 @@ private Tuple<String, String> binaryOpertator(String methodName, String operator
     private Tuple<String, String> greatest(SQLExpr a, SQLExpr b) {
         String name = nextId("greatest");
 
-        String name_a = extractName(a);
-        String name_b = extractName(b);
-
-        String value_a = getPropertyOrStringValue(a);
-        String value_b = getPropertyOrStringValue(b);
-
-        String nameString_a = nextId("string_a");
-        String nameString_b = nextId("string_b");
-
-        return new Tuple<>(name,
-                scriptDeclare(a) + scriptDeclare(b)
-                + "def " + name + ";"
-                + String.format("if (%s instanceof Number) ", name_a)
-                + String.format("{%s = (%s >= %s ? %s : %s);} ", name, name_a, name_b, name_a, name_b)
-                + "else {"
-                + def(nameString_a, convertToString(value_a)) + ";"
-                + def(nameString_b, convertToString(value_b)) + ";"
-                + String.format("%s = (%s.compareTo(%s) >= 0 ? %s : %s);}",
-                        name, nameString_a, nameString_b, value_a, value_b)
-                + name + " = " + name);
+        return comparisonFunctionTemplate(a, b, name, ">");
     }
 
     private Tuple<String, String> least(SQLExpr a, SQLExpr b) {
         String name = nextId("least");
 
-        String name_a = extractName(a);
-        String name_b = extractName(b);
+        return comparisonFunctionTemplate(a, b, name, "<");
+    }
 
-        String value_a = getPropertyOrStringValue(a);
-        String value_b = getPropertyOrStringValue(b);
+    private Tuple<String, String> comparisonFunctionTemplate(SQLExpr a, SQLExpr b,
+                                                             String name, String comparisonOperator) {
+        String value_a = (a instanceof SQLTextLiteralExpr
+                ? getPropertyOrStringValue(a) : getPropertyOrValue(a));
+        String value_b = (b instanceof SQLTextLiteralExpr
+                ? getPropertyOrStringValue(b) : getPropertyOrValue(b));
 
         String nameString_a = nextId("string_a");
         String nameString_b = nextId("string_b");
 
-        return new Tuple<>(name,
-                scriptDeclare(a) + scriptDeclare(b)
-                        + "def " + name + ";"
-                        + String.format("if (%s instanceof Number) ", name_a)
-                        + String.format("{%s = (%s <= %s ? %s : %s);} ", name, name_a, name_b, name_a, name_b)
-                        + "else {"
-                        + def(nameString_a, convertToString(value_a)) + ";"
-                        + def(nameString_b, convertToString(value_b)) + ";"
-                        + String.format("%s = (%s.compareTo(%s) <= 0 ? %s : %s);}",
-                        name, nameString_a, nameString_b, value_a, value_b)
-                        + name + " = " + name);
+        String numberComparison = String.format("%s = (%s %s %s ? %s : %s);",
+                name, value_a, comparisonOperator, value_b, value_a, value_b);
+        String stringOrDateComparison = def(nameString_a, convertToString(value_a)) + ";"
+                + def(nameString_b, convertToString(value_b)) + ";"
+                + String.format("%s = (%s.compareTo(%s) %s 0 ? %s : %s);",
+                name, nameString_a, nameString_b, comparisonOperator, value_a, value_b);
+
+        String definition = "def " + name + ";"
+                + String.format("if (%s) {%s = %s;} ", checkIfNull(a), name, value_b)
+                + String.format("else if (%s) {%s = %s;} ", checkIfNull(b), name, value_a)
+                + "else {";
+
+        if (isProperty(a) && isProperty(b)) {
+            definition += String.format("if (%s instanceof Number) {", value_a)
+                    + numberComparison
+                    + "} else {"
+                    + stringOrDateComparison
+                    + "} ";
+        } else if (a instanceof SQLNumericLiteralExpr || b instanceof SQLNumericLiteralExpr) {
+            definition += numberComparison;
+        } else {
+            definition += stringOrDateComparison;
+        }
+
+        definition += String.format("} %s = %s", name, name);
+
+        return new Tuple<>(name, definition);
     }
 
     private static boolean isProperty(SQLExpr expr) {
@@ -661,6 +662,8 @@ private static boolean isProperty(SQLExpr expr) {
     private static String getPropertyOrValue(SQLExpr expr) {
         if (isProperty(expr)) {
             return doc(expr) + ".value";
+        } else if (expr instanceof SQLNullExpr) {
+            return "null";
         } else {
             return exprString(expr);
         }
@@ -724,6 +727,17 @@ private static String convertType(SQLExpr script) {
 
     }
 
+    private static String checkIfNull(SQLExpr expr) {
+        if (isProperty(expr)) {
+            return doc(expr) + ".size() == 0 || " + getPropertyOrValue(expr) + " == null";
+        } else if (expr instanceof SQLNullExpr) {
+            return "0 == 0";
+        }
+        else {
+            return "0 == 1";
+        }
+    }
+
     private static String convertToString(String name) {
         return String.format("(%s instanceof String ? (String) %s : %s.toString())",
                 name, name, name);