Backport #1586 to 1.3 branch (#1617) (#1621)

Signed-off-by: Peter Fitzgibbons <peter.fitzgibbons@gmail.com> (cherry picked from commit 0c2ba70) Co-authored-by: Peter Fitzgibbons <peter.fitzgibbons@gmail.com>
opensearch-project · May 10, 2023 · edc91b9 · edc91b9
1 parent dd8deff
commit edc91b9
Show file tree

Hide file tree

Showing 10 changed files with 32 additions and 47 deletions.
diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/AggregationIT.java
@@ -22,6 +22,7 @@
 import static org.opensearch.sql.util.MatcherUtils.verifySchema;
 
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -1029,7 +1030,7 @@ public void minOnNestedField() throws Exception {
         TEST_INDEX_NESTED_TYPE);
     JSONObject result = executeQuery(query);
     JSONObject aggregation = getAggregation(result, "message.dayOfWeek@NESTED");
-    Assert.assertEquals(1.0, (double) aggregation.query("/minDays/value"), 0.0001);
+    Assert.assertEquals(1.0, ((BigDecimal) aggregation.query("/minDays/value")).doubleValue(), 0.0001);
   }
 
   @Test
@@ -1039,7 +1040,7 @@ public void sumOnNestedField() throws Exception {
         TEST_INDEX_NESTED_TYPE);
     JSONObject result = executeQuery(query);
     JSONObject aggregation = getAggregation(result, "message.dayOfWeek@NESTED");
-    Assert.assertEquals(19.0, (double) aggregation.query("/sumDays/value"), 0.0001);
+    Assert.assertEquals(19.0, ((BigDecimal) aggregation.query("/sumDays/value")).doubleValue(), 0.0001);
   }
 
   @Test

diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/NestedFieldQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/NestedFieldQueryIT.java
@@ -16,6 +16,7 @@
 import static org.opensearch.sql.util.MatcherUtils.kvString;
 
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.function.Function;
 import org.hamcrest.BaseMatcher;
@@ -316,7 +317,7 @@ public void aggregationWithoutGroupBy() throws IOException {
     JSONObject result = executeQuery(sql);
     JSONObject aggregation = getAggregation(result, "message.dayOfWeek@NESTED");
 
-    Assert.assertThat((Double) aggregation.query("/avgDay/value"), closeTo(3.166666666, 0.01));
+    Assert.assertThat(((BigDecimal) aggregation.query("/avgDay/value")).doubleValue(), closeTo(3.166666666, 0.01));
   }
 
   @Test
@@ -350,10 +351,10 @@ public void groupByRegularFieldAndSum() throws IOException {
     Assert.assertNotNull(msgInfoBuckets);
     Assert.assertThat(msgInfoBuckets.length(), equalTo(2));
     Assert.assertThat(msgInfoBuckets.query("/0/key"), equalTo("a"));
-    Assert.assertThat((Double) msgInfoBuckets.query("/0/message.dayOfWeek@NESTED/sumDay/value"),
+    Assert.assertThat(((BigDecimal) msgInfoBuckets.query("/0/message.dayOfWeek@NESTED/sumDay/value")).doubleValue(),
         closeTo(9.0, 0.01));
     Assert.assertThat(msgInfoBuckets.query("/1/key"), equalTo("b"));
-    Assert.assertThat((Double) msgInfoBuckets.query("/1/message.dayOfWeek@NESTED/sumDay/value"),
+    Assert.assertThat(((BigDecimal) msgInfoBuckets.query("/1/message.dayOfWeek@NESTED/sumDay/value")).doubleValue(),
         closeTo(10.0, 0.01));
   }
 
@@ -593,12 +594,12 @@ public void maxAggOnNestedInnerFieldWithoutWhere() throws IOException {
     Assert.assertThat(bucket.length(), equalTo(2));
     Assert.assertThat(bucket.query("/0/key"), equalTo("Bob Smith"));
     Assert.assertThat(
-        bucket.query("/0/projects.started_year@NESTED/projects.started_year@FILTER/max/value"),
-        equalTo(2015.0));
+        ((BigDecimal) bucket.query("/0/projects.started_year@NESTED/projects.started_year@FILTER/max/value")).doubleValue(),
+        closeTo(2015.0, 0.01));
     Assert.assertThat(bucket.query("/1/key"), equalTo("Jane Smith"));
     Assert.assertThat(
-        bucket.query("/1/projects.started_year@NESTED/projects.started_year@FILTER/max/value"),
-        equalTo(2015.0));
+        ((BigDecimal) bucket.query("/1/projects.started_year@NESTED/projects.started_year@FILTER/max/value")).doubleValue(),
+        closeTo(2015.0, 0.01));
   }
 
   @Test
@@ -780,12 +781,12 @@ public void havingMaxAggOnNestedInnerFieldWithoutWhere() throws IOException {
     Assert.assertThat(bucket.length(), equalTo(2));
     Assert.assertThat(bucket.query("/0/key"), equalTo("Bob Smith"));
     Assert.assertThat(
-        bucket.query("/0/projects.started_year@NESTED/projects.started_year@FILTER/max_0/value"),
-        equalTo(2015.0));
+        ((BigDecimal) bucket.query("/0/projects.started_year@NESTED/projects.started_year@FILTER/max_0/value")).doubleValue(),
+        closeTo(2015.0, 0.01));
     Assert.assertThat(bucket.query("/1/key"), equalTo("Jane Smith"));
     Assert.assertThat(
-        bucket.query("/1/projects.started_year@NESTED/projects.started_year@FILTER/max_0/value"),
-        equalTo(2015.0));
+        ((BigDecimal) bucket.query("/1/projects.started_year@NESTED/projects.started_year@FILTER/max_0/value")).doubleValue(),
+        closeTo(2015.0, 0.01));
   }
 
   /***********************************************************

diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLFunctionsIT.java
@@ -501,7 +501,7 @@ public void concat_ws_field_and_string() throws Exception {
   }
 
   /**
-   * Ignore this test case because painless doesn't whitelist String.split function.
+   * Ignore this test case because painless doesn't allowlist String.split function.
    *
    * @see <a href="https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html">https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html</a>
    */
@@ -517,7 +517,7 @@ public void whereConditionLeftFunctionRightVariableEqualTest() throws Exception
   }
 
   /**
-   * Ignore this test case because painless doesn't whitelist String.split function.
+   * Ignore this test case because painless doesn't allowlist String.split function.
    *
    * @see <a href="https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html">https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html</a>
    */
@@ -810,7 +810,7 @@ public void isnullWithMathExpr() throws IOException {
   }
 
   /**
-   * Ignore this test case because painless doesn't whitelist String.split function.
+   * Ignore this test case because painless doesn't allowlist String.split function.
    *
    * @see <a href="https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html">https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html</a>
    */

diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SubqueryIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SubqueryIT.java
@@ -7,6 +7,7 @@
 package org.opensearch.sql.legacy;
 
 import static org.hamcrest.Matchers.both;
+import static org.hamcrest.Matchers.closeTo;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.core.Is.is;
 import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT;
@@ -22,6 +23,7 @@
 
 import com.google.common.collect.Ordering;
 import java.io.IOException;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
@@ -345,10 +347,10 @@ public void selectFromSubqueryCountAndSum() throws IOException {
             TEST_INDEX_ACCOUNT));
 
     assertThat(result.query("/aggregations/count/value"), equalTo(1000));
-    assertThat(result.query("/aggregations/balance/value"), equalTo(25714837.0));
+    assertThat(((BigDecimal) result.query("/aggregations/balance/value")).doubleValue(),
+        closeTo(25714837.0, 0.01));
   }
 
-  @Ignore("Skip to avoid breaking test due to inconsistency in JDBC schema")
   @Test
   public void selectFromSubqueryWithoutAliasShouldPass() throws IOException {
     JSONObject response = executeJdbcRequest(

diff --git a/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java b/integ-test/src/test/java/org/opensearch/sql/util/MatcherUtils.java
@@ -21,6 +21,7 @@
 
 import com.google.common.base.Strings;
 import com.google.gson.JsonParser;
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -121,7 +122,7 @@ public static Matcher<JSONObject> kvString(String key, Matcher<String> matcher)
   }
 
   public static Matcher<JSONObject> kvDouble(String key, Matcher<Double> matcher) {
-    return featureValueOf("Json Match", matcher, actual -> (Double) actual.query(key));
+    return featureValueOf("Json Match", matcher, actual -> ((BigDecimal) actual.query(key)).doubleValue());
   }
 
   public static Matcher<JSONObject> kvInt(String key, Matcher<Integer> matcher) {
@@ -231,30 +232,7 @@ public void describeTo(Description description) {
 
       @Override
       protected boolean matchesSafely(JSONArray array) {
-        if (array.length() != expectedObjects.length) {
-          return false;
-        }
-
-        for (int i = 0; i < expectedObjects.length; i++) {
-          Object expected = expectedObjects[i];
-          boolean isEqual;
-
-          // Use similar() because JSONObject/JSONArray.equals() only check if same reference
-          if (expected instanceof JSONObject) {
-            isEqual = ((JSONObject) expected).similar(array.get(i));
-          } else if (expected instanceof JSONArray) {
-            isEqual = ((JSONArray) expected).similar(array.get(i));
-          } else if (null == expected) {
-            isEqual = JSONObject.NULL == array.get(i);
-          } else {
-            isEqual = expected.equals(array.get(i));
-          }
-
-          if (!isEqual) {
-            return false;
-          }
-        }
-        return true;
+        return array.similar(new JSONArray(expectedObjects));
       }
     };
   }

diff --git a/legacy/build.gradle b/legacy/build.gradle
@@ -89,7 +89,7 @@ dependencies {
         }
     }
     implementation group: 'com.google.guava', name: 'guava', version: '31.0.1-jre'
-    compile group: 'org.json', name: 'json', version:'20180813'
+    compile group: 'org.json', name: 'json', version:'20230227'
     compile group: 'org.apache.commons', name: 'commons-lang3', version: '3.10'
     compile group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}"
     compile project(':sql')

diff --git a/legacy/src/main/java/org/opensearch/sql/legacy/expression/model/ExprValueFactory.java b/legacy/src/main/java/org/opensearch/sql/legacy/expression/model/ExprValueFactory.java
@@ -6,6 +6,7 @@
 
 package org.opensearch.sql.legacy.expression.model;
 
+import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -61,6 +62,8 @@ public static ExprValue from(Object o) {
             return booleanValue((Boolean) o);
         } else if (o instanceof Double) {
             return doubleValue((Double) o);
+        } else if (o instanceof BigDecimal) {
+            return doubleValue(((BigDecimal) o).doubleValue());
         } else if (o instanceof String) {
             return stringValue((String) o);
         } else {

diff --git a/opensearch/build.gradle b/opensearch/build.gradle
@@ -35,7 +35,7 @@ dependencies {
     compile group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: "${versions.jackson}"
     compile group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: "${versions.jackson_databind}"
     compile group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: "${versions.jackson}"
-    compile group: 'org.json', name: 'json', version:'20180813'
+    compile group: 'org.json', name: 'json', version:'20230227'
     compileOnly group: 'org.opensearch.client', name: 'opensearch-rest-high-level-client', version: "${opensearch_version}"
     compile group: 'org.opensearch', name:'opensearch-ml-client', version: '1.3.4.0-SNAPSHOT'
 

diff --git a/ppl/build.gradle b/ppl/build.gradle
@@ -47,7 +47,7 @@ dependencies {
     compile "org.antlr:antlr4-runtime:4.7.1"
     compile group: 'com.google.guava', name: 'guava', version: '31.0.1-jre'
     compile group: 'org.opensearch', name: 'opensearch-x-content', version: "${opensearch_version}"
-    compile group: 'org.json', name: 'json', version: '20180813'
+    compile group: 'org.json', name: 'json', version: '20230227'
     compile group: 'org.springframework', name: 'spring-context', version: "${spring_version}"
     compile group: 'org.springframework', name: 'spring-beans', version: "${spring_version}"
     compile group: 'org.apache.logging.log4j', name: 'log4j-core', version:'2.17.1'

diff --git a/sql/build.gradle b/sql/build.gradle
@@ -46,7 +46,7 @@ dependencies {
 
     compile "org.antlr:antlr4-runtime:4.7.1"
     implementation group: 'com.google.guava', name: 'guava', version: '31.0.1-jre'
-    compile group: 'org.json', name: 'json', version:'20180813'
+    compile group: 'org.json', name: 'json', version:'20230227'
     compile group: 'org.springframework', name: 'spring-context', version: "${spring_version}"
     compile group: 'org.springframework', name: 'spring-beans', version: "${spring_version}"
     compile project(':common')