Skip to content

Commit 2879bc5

Browse files
committed
fix IT
Signed-off-by: Lantao Jin <ltjin@amazon.com>
1 parent 91309af commit 2879bc5

4 files changed

Lines changed: 39 additions & 21 deletions

File tree

core/src/main/java/org/opensearch/sql/executor/OpenSearchTypeSystem.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,33 @@ private OpenSearchTypeSystem() {}
1818

1919
@Override
2020
public RelDataType deriveAvgAggType(RelDataTypeFactory typeFactory, RelDataType argumentType) {
21-
if (SqlTypeName.INT_TYPES.contains(argumentType.getSqlTypeName())) {
21+
if (SqlTypeName.DECIMAL == argumentType.getSqlTypeName()) {
22+
return typeFactory.createTypeWithNullability(highPrecision(typeFactory, argumentType), false);
23+
} else if (SqlTypeName.INT_TYPES.contains(argumentType.getSqlTypeName())) {
2224
return typeFactory.createTypeWithNullability(
2325
typeFactory.createSqlType(SqlTypeName.DOUBLE), false);
2426
} else {
2527
return argumentType;
2628
}
2729
}
30+
31+
/**
32+
* Compute a higher precision version of a type.
33+
*
34+
* @return If type is a DECIMAL type, return a type with the precision and scale +4, to align the
35+
* behaviour with Apache Spark.
36+
*/
37+
public static RelDataType highPrecision(
38+
final RelDataTypeFactory typeFactory, final RelDataType type) {
39+
if (type.getSqlTypeName() == SqlTypeName.DECIMAL) {
40+
return typeFactory.createSqlType(
41+
type.getSqlTypeName(),
42+
Math.min(
43+
type.getPrecision() + 4,
44+
typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.DECIMAL)),
45+
Math.min(
46+
type.getScale() + 4, typeFactory.getTypeSystem().getMaxScale(SqlTypeName.DECIMAL)));
47+
}
48+
return type;
49+
}
2850
}

core/src/main/java/org/opensearch/sql/expression/function/udf/math/DivideFunction.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313
import org.apache.calcite.adapter.enumerable.RexToLixTranslator;
1414
import org.apache.calcite.linq4j.tree.Expression;
1515
import org.apache.calcite.linq4j.tree.Expressions;
16+
import org.apache.calcite.rel.type.RelDataTypeSystem;
1617
import org.apache.calcite.rex.RexCall;
1718
import org.apache.calcite.sql.type.ReturnTypes;
1819
import org.apache.calcite.sql.type.SqlReturnTypeInference;
20+
import org.apache.calcite.sql.type.SqlTypeName;
1921
import org.opensearch.sql.calcite.utils.MathUtils;
2022
import org.opensearch.sql.expression.function.ImplementorUDF;
2123

@@ -26,7 +28,12 @@
2628
* PPL. Therefore, we implement our versions
2729
*/
2830
public class DivideFunction extends ImplementorUDF {
29-
public static final int MAX_SCALE = 38;
31+
/**
32+
* The maximum scale of numeric which is aligned with {@link
33+
* RelDataTypeSystem#getMaxScale(SqlTypeName)}. TODO The max scale in Spark is 38, but the default
34+
* max scale in Calcite is 19.
35+
*/
36+
public static final int MAX_NUMERIC_SCALE = 19;
3037

3138
public DivideFunction() {
3239
super(new DivideImplementor(), NullPolicy.ANY);
@@ -59,10 +66,13 @@ public static Number divide(Number dividend, Number divisor) {
5966
return MathUtils.coerceToWidestIntegralType(dividend, divisor, result);
6067
} else if (MathUtils.isDecimal(dividend) && MathUtils.isIntegral(divisor)) {
6168
return ((BigDecimal) dividend)
62-
.divide(BigDecimal.valueOf(divisor.longValue()), MAX_SCALE + 1, RoundingMode.HALF_UP);
69+
.divide(
70+
BigDecimal.valueOf(divisor.longValue()),
71+
MAX_NUMERIC_SCALE + 1,
72+
RoundingMode.HALF_UP);
6373
} else if (MathUtils.isIntegral(dividend) && MathUtils.isDecimal(divisor)) {
6474
return (BigDecimal.valueOf(dividend.longValue()))
65-
.divide((BigDecimal) divisor, MAX_SCALE + 1, RoundingMode.HALF_UP);
75+
.divide((BigDecimal) divisor, MAX_NUMERIC_SCALE + 1, RoundingMode.HALF_UP);
6676
}
6777
double result = dividend.doubleValue() / divisor.doubleValue();
6878
return MathUtils.coerceToWidestFloatingType(dividend, divisor, result);

integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteStatsCommandIT.java

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,6 @@
55

66
package org.opensearch.sql.calcite.remote;
77

8-
import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT;
9-
import static org.opensearch.sql.util.MatcherUtils.rows;
10-
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
11-
12-
import java.io.IOException;
13-
import org.json.JSONObject;
14-
import org.junit.jupiter.api.Test;
158
import org.opensearch.sql.ppl.StatsCommandIT;
169

1710
public class CalciteStatsCommandIT extends StatsCommandIT {
@@ -20,13 +13,6 @@ public void init() throws Exception {
2013
super.init();
2114
enableCalcite();
2215
disallowCalciteFallback();
23-
}
24-
25-
@Test
26-
public void testStatsNestedDoubleValue2() throws IOException {
27-
JSONObject response =
28-
executeQuery(
29-
String.format("source=%s | eval t = typeof(avg(abs(age) * 2.0))", TEST_INDEX_ACCOUNT));
30-
verifyDataRows(response, rows("60.342"));
16+
setQuerySizeLimit(2000);
3117
}
3218
}

integ-test/src/test/java/org/opensearch/sql/calcite/standalone/CalcitePPLIntegTestCase.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ private Settings defaultSettings() {
109109
return new Settings() {
110110
private final Map<Key, Object> defaultSettings =
111111
new ImmutableMap.Builder<Key, Object>()
112-
.put(Key.QUERY_SIZE_LIMIT, 200)
112+
.put(Key.QUERY_SIZE_LIMIT, 2000)
113113
.put(Key.FIELD_TYPE_TOLERANCE, true)
114114
.put(Key.CALCITE_ENGINE_ENABLED, true)
115115
.put(Key.CALCITE_FALLBACK_ALLOWED, false)
@@ -135,7 +135,7 @@ protected Settings enablePushdown() {
135135
return new Settings() {
136136
private final Map<Key, Object> defaultSettings =
137137
new ImmutableMap.Builder<Key, Object>()
138-
.put(Key.QUERY_SIZE_LIMIT, 200)
138+
.put(Key.QUERY_SIZE_LIMIT, 2000)
139139
.put(Key.FIELD_TYPE_TOLERANCE, true)
140140
.put(Key.CALCITE_ENGINE_ENABLED, true)
141141
.put(Key.CALCITE_FALLBACK_ALLOWED, false)

0 commit comments

Comments
 (0)