Skip to content

Commit 7b9e6d9

Browse files
github-actions[bot]github-actions[bot]
committed
Support using decimal as span literals (#4717)
* Support decimal for span used in bin command Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Support decimal literal in span function Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Convert interval to double if it is BigDecimal for further calculation Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Make the error messages for decimal span length more meaningful Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Add mssing weeks to PLURAL_UNIT in ppl lexer Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> --------- Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> (cherry picked from commit a3c90a8) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 9bdd75c commit 7b9e6d9

8 files changed

Lines changed: 103 additions & 26 deletions

File tree

‎core/src/main/java/org/opensearch/sql/expression/function/udf/SpanFunction.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import org.apache.calcite.schema.impl.ScalarFunctionImpl;
2121
import org.apache.calcite.sql.type.CompositeOperandTypeChecker;
2222
import org.apache.calcite.sql.type.OperandTypes;
23-
import org.apache.calcite.sql.type.ReturnTypes;
2423
import org.apache.calcite.sql.type.SqlReturnTypeInference;
2524
import org.apache.calcite.sql.type.SqlTypeName;
2625
import org.apache.calcite.sql.type.SqlTypeFamily;
@@ -45,7 +44,17 @@ public SpanFunction() {
4544

4645
@Override
4746
public SqlReturnTypeInference getReturnTypeInference() {
48-
return ReturnTypes.ARG0;
47+
// Return arg0 type if it has a unit (i.e. time related span)
48+
return callBinding -> {
49+
if (SqlTypeUtil.isString(callBinding.getOperandType(2))) {
50+
return callBinding.getOperandType(0);
51+
}
52+
// Use the least restrictive type between the field type and the interval type if it's a
53+
// numeric span. E.g. span(int_field, double_literal) -> double
54+
return callBinding
55+
.getTypeFactory()
56+
.leastRestrictive(List.of(callBinding.getOperandType(0), callBinding.getOperandType(1)));
57+
};
4958
}
5059

5160
@Override
@@ -57,10 +66,9 @@ public UDFOperandMetadata getOperandMetadata() {
5766
.or(
5867
OperandTypes.family(
5968
SqlTypeFamily.DATETIME, SqlTypeFamily.NUMERIC, SqlTypeFamily.CHARACTER))
60-
// TODO: numeric span should support decimal as its interval
6169
.or(
6270
OperandTypes.family(
63-
SqlTypeFamily.NUMERIC, SqlTypeFamily.INTEGER, SqlTypeFamily.ANY)));
71+
SqlTypeFamily.NUMERIC, SqlTypeFamily.NUMERIC, SqlTypeFamily.ANY)));
6472
}
6573

6674
public static class SpanImplementor implements NotNullImplementor {
@@ -73,8 +81,12 @@ public Expression implement(
7381
Expression interval = translatedOperands.get(1);
7482

7583
RelDataType fieldType = call.getOperands().get(0).getType();
84+
RelDataType intervalType = call.getOperands().get(1).getType();
7685
RelDataType unitType = call.getOperands().get(2).getType();
7786

87+
if (SqlTypeUtil.isDecimal(intervalType)) {
88+
interval = Expressions.call(interval, "doubleValue");
89+
}
7890
if (SqlTypeUtil.isNull(unitType)) {
7991
SqlTypeName sqlTypeName = call.getType().getSqlTypeName();
8092
Expression result;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,4 +1068,12 @@ public void testBinWithEvalCreatedDottedFieldName() throws IOException {
10681068
rows(false, "go", "opentelemetry", 16, 1, "12-14"),
10691069
rows(true, "rust", "opentelemetry", 12, 1, "14-16"));
10701070
}
1071+
1072+
@Test
1073+
public void testBinWithDecimalSpan() throws IOException {
1074+
JSONObject result =
1075+
executeQuery("source=events_null | bin cpu_usage span=7.5 | stats count() by cpu_usage");
1076+
verifySchema(result, schema("count()", "bigint"), schema("cpu_usage", "string"));
1077+
verifyDataRows(result, rows(3, "37.5-45.0"), rows(2, "45.0-52.5"), rows(1, "52.5-60.0"));
1078+
}
10711079
}

‎integ-test/src/test/java/org/opensearch/sql/ppl/StatsCommandIT.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,4 +1217,21 @@ public void testStatsSortOnMeasureComplex() throws IOException {
12171217
resetQueryBucketSize();
12181218
}
12191219
}
1220+
1221+
@Test
1222+
public void testStatsByFractionalSpan() throws IOException {
1223+
JSONObject response1 =
1224+
executeQuery(
1225+
String.format(
1226+
"source=%s | stats count by span(balance, 4170.5)",
1227+
TEST_INDEX_BANK_WITH_NULL_VALUES));
1228+
verifySchema(response1, schema("count", "bigint"), schema("span(balance,4170.5)", "double"));
1229+
verifyDataRows(
1230+
response1,
1231+
rows(3, null),
1232+
rows(1, 4170.5),
1233+
rows(1, 29193.5),
1234+
rows(1, 37534.5),
1235+
rows(1, 45875.5));
1236+
}
12201237
}

‎ppl/src/main/antlr/OpenSearchPPLLexer.g4‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -521,40 +521,35 @@ ALIGNTIME: 'ALIGNTIME';
521521
// Must precede ID to avoid conflicts with identifier matching
522522
PERCENTILE_SHORTCUT: PERC(INTEGER_LITERAL | DECIMAL_LITERAL) | 'P'(INTEGER_LITERAL | DECIMAL_LITERAL);
523523

524-
SPANLENGTH: [0-9]+ (
525-
'US' |'CS'|'DS'
526-
|'MS'|'MILLISECOND'|'MILLISECONDS'
527-
|'S'|'SEC'|'SECS'|'SECOND'|'SECONDS'
528-
|'MIN'|'MINS'|'MINUTE'|'MINUTES'
529-
|'H'|'HR'|'HRS'|'HOUR'|'HOURS'
530-
|'H'|'HR'|'HRS'|'HOUR'|'HOURS'
531-
|'D'|'DAY'|'DAYS'
532-
|'W'|'WEEK'|'WEEKS'
533-
|'M'|'MON'|'MONTH'|'MONTHS'
534-
|'Q'|'QTR'|'QTRS'|'QUARTER'|'QUARTERS'
535-
|'Y'|'YR'|'YRS'|'YEAR'|'YEARS'
536-
);
524+
fragment DAY_OR_DOUBLE: 'D';
525+
fragment COMMON_TIME_UNIT: 'S'|'SEC'|'SECOND'
526+
|'M'|'MIN'|'MINUTE'
527+
|'H'|'HR'|'HOUR'
528+
|'DAY'|'W'|'WEEK'
529+
|'MON'|'MONTH'
530+
|'Q'|'QTR'|'QUARTER'
531+
|'Y'|'YR'|'YEAR';
532+
fragment PLURAL_UNIT: 'MILLISECONDS'|'SECS'|'SECONDS'|'MINS'|'MINUTES'|'HRS'|'HOURS'
533+
|'DAYS'|'WEEKS'|'MONTHS'|'QTRS'|'QUARTERS'|'YRS'|'YEARS';
534+
fragment SPANUNIT: COMMON_TIME_UNIT | PLURAL_UNIT
535+
|'US'|'CS'|'DS'
536+
|'MS'|'MILLISECOND';
537+
SPANLENGTH: DEC_DIGIT+ (SPANUNIT | DAY_OR_DOUBLE);
538+
DECIMAL_SPANLENGTH: (DEC_DIGIT+)? '.' DEC_DIGIT+ SPANUNIT;
537539

538540
NUMERIC_ID : DEC_DIGIT+ ID_LITERAL;
539541

540542
// LITERALS AND VALUES
541543
//STRING_LITERAL: DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING;
542544
fragment WEEK_SNAP_UNIT: 'W' [0-7];
543-
fragment TIME_SNAP_UNIT: 'S' | 'SEC' | 'SECOND'
544-
| 'M' | 'MIN' | 'MINUTE'
545-
| 'H' | 'HR' | 'HOUR' | 'HOURS'
546-
| 'D' | 'DAY'
547-
| 'W' | 'WEEK' | WEEK_SNAP_UNIT
548-
| 'MON' | 'MONTH'
549-
| 'Q' | 'QTR' | 'QUARTER'
550-
| 'Y' | 'YR' | 'YEAR';
545+
fragment TIME_SNAP_UNIT: COMMON_TIME_UNIT | WEEK_SNAP_UNIT | DAY_OR_DOUBLE;
551546
TIME_SNAP: AT TIME_SNAP_UNIT;
552547
ID: ID_LITERAL;
553548
CLUSTER: CLUSTER_PREFIX_LITERAL;
554549
INTEGER_LITERAL: DEC_DIGIT+;
555550
DECIMAL_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+;
556551
FLOAT_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ 'F';
557-
DOUBLE_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ 'D';
552+
DOUBLE_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ DAY_OR_DOUBLE;
558553

559554
fragment DATE_SUFFIX: ([\-.][*0-9]+)+;
560555
fragment CLUSTER_PREFIX_LITERAL: [*A-Z]+?[*A-Z_\-0-9]* COLON;

‎ppl/src/main/antlr/OpenSearchPPLParser.g4‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,10 @@ timechartParameter
336336

337337
spanLiteral
338338
: SPANLENGTH
339+
| DECIMAL_SPANLENGTH
340+
| DOUBLE_LITERAL // 1.5d can also represent decimal span length
339341
| INTEGER_LITERAL
342+
| DECIMAL_LITERAL
340343
;
341344

342345
evalCommand

‎ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,15 @@ private List<UnresolvedExpression> multiFieldRelevanceArguments(
824824
public UnresolvedExpression visitSpanLiteral(OpenSearchPPLParser.SpanLiteralContext ctx) {
825825
if (ctx.INTEGER_LITERAL() != null) {
826826
return AstDSL.intLiteral(Integer.parseInt(ctx.INTEGER_LITERAL().getText()));
827+
} else if (ctx.DECIMAL_LITERAL() != null) {
828+
return AstDSL.decimalLiteral(new BigDecimal(ctx.DECIMAL_LITERAL().getText()));
829+
} else if (ctx.DECIMAL_SPANLENGTH() != null || ctx.DOUBLE_LITERAL() != null) {
830+
throw new IllegalArgumentException(
831+
StringUtils.format(
832+
"Span length [%s] is invalid: floating-point time intervals are not supported.",
833+
ctx.DECIMAL_SPANLENGTH() != null
834+
? ctx.DECIMAL_SPANLENGTH().getText()
835+
: ctx.DOUBLE_LITERAL().getText()));
827836
} else {
828837
return AstDSL.stringLiteral(ctx.getText());
829838
}

‎ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,4 +1606,24 @@ public void testChartCommandWithBottomLimit() {
16061606
exprList(argument("limit", intLiteral(3)), argument("top", booleanLiteral(false))))
16071607
.build());
16081608
}
1609+
1610+
@Test
1611+
public void testTimeSpanWithDecimalShouldThrow() {
1612+
Throwable t1 =
1613+
assertThrows(
1614+
IllegalArgumentException.class, () -> plan("source=t | timechart span=1.5d count"));
1615+
assertTrue(
1616+
t1.getMessage()
1617+
.contains(
1618+
"Span length [1.5d] is invalid: floating-point time intervals are not supported."));
1619+
1620+
Throwable t2 =
1621+
assertThrows(
1622+
IllegalArgumentException.class,
1623+
() -> plan("source=t | stats count by span(@timestamp, 2.5y)"));
1624+
assertTrue(
1625+
t2.getMessage()
1626+
.contains(
1627+
"Span length [2.5y] is invalid: floating-point time intervals are not supported."));
1628+
}
16091629
}

‎ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,6 +1655,19 @@ public void testVisitSpanLiteral() {
16551655
exprList(
16561656
argument("limit", intLiteral(10)), argument("useother", booleanLiteral(true))))
16571657
.build());
1658+
1659+
// Test span literal with decimal value
1660+
assertEqual(
1661+
"source=events_null | bin cpu_usage span=7.5 | stats count() by cpu_usage",
1662+
agg(
1663+
bin(
1664+
relation("events_null"),
1665+
field("cpu_usage"),
1666+
argument("span", decimalLiteral(new java.math.BigDecimal("7.5")))),
1667+
exprList(alias("count()", aggregate("count", allFields()))),
1668+
emptyList(),
1669+
exprList(alias("cpu_usage", field("cpu_usage"))),
1670+
defaultStatsArgs()));
16581671
}
16591672

16601673
@Test

0 commit comments

Comments
 (0)