From bfa4038ac49a256f90f2b5a9074731e16cc7c435 Mon Sep 17 00:00:00 2001 From: Peter Fitzgibbons Date: Tue, 9 May 2023 09:13:20 -0700 Subject: [PATCH] Backport #1586 to 1.3 branch (#1617) Signed-off-by: Peter Fitzgibbons (cherry picked from commit 0c2ba70d4de9b3eadb35a6aed83351f0b7354e67) --- .../opensearch/sql/legacy/AggregationIT.java | 5 ++-- .../sql/legacy/NestedFieldQueryIT.java | 23 +++++++-------- .../opensearch/sql/legacy/SQLFunctionsIT.java | 6 ++-- .../org/opensearch/sql/legacy/SubqueryIT.java | 6 ++-- .../org/opensearch/sql/util/MatcherUtils.java | 28 ++----------------- legacy/build.gradle | 2 +- .../expression/model/ExprValueFactory.java | 3 ++ opensearch/build.gradle | 2 +- ppl/build.gradle | 2 +- sql/build.gradle | 2 +- 10 files changed, 32 insertions(+), 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 index 82260b9f07..a3b1301a69 100644 --- 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 index cd83a09a03..80ca06fd5d 100644 --- 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 index 99f3c83cdc..a435787478 100644 --- 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 https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html */ @@ -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 https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html */ @@ -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 https://www.elastic.co/guide/en/elasticsearch/painless/7.0/painless-api-reference.html */ 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 index de73118a54..0d1091d7ec 100644 --- 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 index 8c5be3d761..836f9a1690 100644 --- 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 kvString(String key, Matcher matcher) } public static Matcher kvDouble(String key, Matcher 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 kvInt(String key, Matcher 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 index 06c694f25d..849e21b694 100644 --- 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 index bc7cb40c31..5dc2b5b50a 100644 --- 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 index e871224004..412f9e5472 100644 --- 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 index 8b96d302d8..1feb1322ef 100644 --- 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 index 8572be7f3f..6aad866550 100644 --- 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')