From e7f8777641ce5e0828992533211f347855c8dd20 Mon Sep 17 00:00:00 2001
From: chloe-zh <fizhang@amazon>
Date: Wed, 20 Nov 2019 14:50:20 -0800
Subject: [PATCH] Made a fix to log10, added test

---
 .../sql/utils/SQLFunctions.java                  | 16 +++++++++++++---
 .../sql/esintgtest/MathFunctionsIT.java          |  7 +++++++
 .../sql/unittest/MathFunctionsTest.java          | 12 ++++++++++++
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java
index 5131b35725..fc7c8f54fc 100644
--- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java
+++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java
@@ -325,17 +325,17 @@ public Tuple<String, String> function(String methodName, List<KVValue> paramers,
                 functionStr = log(SQLUtils.toSQLExpr("2"), (SQLExpr) paramers.get(0).value, name);
                 break;
             case "log10":
-                functionStr = log(SQLUtils.toSQLExpr("10"), (SQLExpr) paramers.get(0).value, name);
+                functionStr = log10((SQLExpr) paramers.get(0).value);
                 break;
             case "log":
                 if (paramers.size() > 1) {
                     functionStr = log((SQLExpr) paramers.get(0).value, (SQLExpr) paramers.get(1).value, name);
                 } else {
-                    functionStr = log((SQLUtils.toSQLExpr("Math.E")), (SQLExpr) paramers.get(0).value, name);
+                    functionStr = ln((SQLExpr) paramers.get(0).value);
                 }
                 break;
             case "ln":
-                functionStr = log(SQLUtils.toSQLExpr("Math.E"), (SQLExpr) paramers.get(0).value, name);
+                functionStr = ln((SQLExpr) paramers.get(0).value);
                 break;
             case "assign":
                 functionStr = assign((SQLExpr) paramers.get(0).value);
@@ -653,6 +653,16 @@ public Tuple<String, String> log(SQLExpr base, SQLExpr field, String valueName)
         return new Tuple<>(name, result);
     }
 
+    public Tuple<String, String> log10(SQLExpr field) {
+        String name = nextId("log10");
+        return new Tuple<>(name, def(name, StringUtils.format("Math.log10(%s)", getPropertyOrValue(field))));
+    }
+
+    public Tuple<String, String> ln(SQLExpr field) {
+        String name = nextId("ln");
+        return new Tuple<>(name, def(name, StringUtils.format("Math.log(%s)", getPropertyOrValue(field))));
+    }
+
     public Tuple<String, String> trim(SQLExpr field, String valueName) {
         return strSingleValueTemplate("trim", field, valueName);
     }
diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MathFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MathFunctionsIT.java
index fc88bf4df5..34ecf4447e 100644
--- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MathFunctionsIT.java
+++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/MathFunctionsIT.java
@@ -210,6 +210,13 @@ public void logInAggregationShouldPass() {
         );
     }
 
+    @Test
+    public void log10Test() throws IOException{
+        SearchHit[] hits = query("SELECT log10(1000) AS log10");
+        double log10 = (double) getField(hits[0], "log10");
+        assertThat(log10, equalTo(3.0));
+    }
+
     @Test
     public void ln() throws IOException {
         SearchHit[] hits = query("SELECT LN(5) AS ln");
diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/MathFunctionsTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/MathFunctionsTest.java
index c8f17bffe0..dbb605627e 100644
--- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/MathFunctionsTest.java
+++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/MathFunctionsTest.java
@@ -408,6 +408,18 @@ public void logWithTwoParams() {
                         "Math.log(doc['age'].value)/Math.log(3)"));
     }
 
+    @Test
+    public void log10Test() {
+        String query = "SELECT LOG10(age) FROM accounts";
+        ScriptField scriptField = CheckScriptContents.getScriptFieldFromQuery(query);
+        assertTrue(
+                CheckScriptContents.scriptContainsString(
+                        scriptField,
+                        "Math.log10(doc['age'].value)"
+                )
+        );
+    }
+
     @Test
     public void lnTest() {
         String query = "SELECT LN(age) FROM age WHERE LN(age) = 5.0";