Skip to content

Commit a3c90a8

Browse files
yuancuyuancu
authored
Support using decimal as span literals (#4717)
* Support decimal for span used in bin command Signed-off-by: Yuanchun Shen <[email protected]> * Support decimal literal in span function Signed-off-by: Yuanchun Shen <[email protected]> * Convert interval to double if it is BigDecimal for further calculation Signed-off-by: Yuanchun Shen <[email protected]> * Make the error messages for decimal span length more meaningful Signed-off-by: Yuanchun Shen <[email protected]> * Add mssing weeks to PLURAL_UNIT in ppl lexer Signed-off-by: Yuanchun Shen <[email protected]> --------- Signed-off-by: Yuanchun Shen <[email protected]>
1 parent 66662ae commit a3c90a8

File tree

8 files changed

+103
-26
lines changed

8 files changed

+103
-26
lines changed

‎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.SqlTypeFamily;
2625
import org.apache.calcite.sql.type.SqlTypeUtil;
@@ -44,7 +43,17 @@ public SpanFunction() {
4443

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

5059
@Override
@@ -56,10 +65,9 @@ public UDFOperandMetadata getOperandMetadata() {
5665
.or(
5766
OperandTypes.family(
5867
SqlTypeFamily.DATETIME, SqlTypeFamily.NUMERIC, SqlTypeFamily.CHARACTER))
59-
// TODO: numeric span should support decimal as its interval
6068
.or(
6169
OperandTypes.family(
62-
SqlTypeFamily.NUMERIC, SqlTypeFamily.INTEGER, SqlTypeFamily.ANY)));
70+
SqlTypeFamily.NUMERIC, SqlTypeFamily.NUMERIC, SqlTypeFamily.ANY)));
6371
}
6472

6573
public static class SpanImplementor implements NotNullImplementor {
@@ -72,8 +80,12 @@ public Expression implement(
7280
Expression interval = translatedOperands.get(1);
7381

7482
RelDataType fieldType = call.getOperands().get(0).getType();
83+
RelDataType intervalType = call.getOperands().get(1).getType();
7584
RelDataType unitType = call.getOperands().get(2).getType();
7685

86+
if (SqlTypeUtil.isDecimal(intervalType)) {
87+
interval = Expressions.call(interval, "doubleValue");
88+
}
7789
if (SqlTypeUtil.isNull(unitType)) {
7890
return switch (call.getType().getSqlTypeName()) {
7991
case BIGINT, INTEGER, SMALLINT, TINYINT -> Expressions.multiply(

‎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
@@ -1225,4 +1225,21 @@ public void testStatsSortOnMeasureComplex() throws IOException {
12251225
resetQueryBucketSize();
12261226
}
12271227
}
1228+
1229+
@Test
1230+
public void testStatsByFractionalSpan() throws IOException {
1231+
JSONObject response1 =
1232+
executeQuery(
1233+
String.format(
1234+
"source=%s | stats count by span(balance, 4170.5)",
1235+
TEST_INDEX_BANK_WITH_NULL_VALUES));
1236+
verifySchema(response1, schema("count", "bigint"), schema("span(balance,4170.5)", "double"));
1237+
verifyDataRows(
1238+
response1,
1239+
rows(3, null),
1240+
rows(1, 4170.5),
1241+
rows(1, 29193.5),
1242+
rows(1, 37534.5),
1243+
rows(1, 45875.5));
1244+
}
12281245
}

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

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

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

535537
NUMERIC_ID : DEC_DIGIT+ ID_LITERAL;
536538

537539
// LITERALS AND VALUES
538540
//STRING_LITERAL: DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING;
539541
fragment WEEK_SNAP_UNIT: 'W' [0-7];
540-
fragment TIME_SNAP_UNIT: 'S' | 'SEC' | 'SECOND'
541-
| 'M' | 'MIN' | 'MINUTE'
542-
| 'H' | 'HR' | 'HOUR' | 'HOURS'
543-
| 'D' | 'DAY'
544-
| 'W' | 'WEEK' | WEEK_SNAP_UNIT
545-
| 'MON' | 'MONTH'
546-
| 'Q' | 'QTR' | 'QUARTER'
547-
| 'Y' | 'YR' | 'YEAR';
542+
fragment TIME_SNAP_UNIT: COMMON_TIME_UNIT | WEEK_SNAP_UNIT | DAY_OR_DOUBLE;
548543
TIME_SNAP: AT TIME_SNAP_UNIT;
549544
ID: ID_LITERAL;
550545
CLUSTER: CLUSTER_PREFIX_LITERAL;
551546
INTEGER_LITERAL: DEC_DIGIT+;
552547
DECIMAL_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+;
553548
FLOAT_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ 'F';
554-
DOUBLE_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ 'D';
549+
DOUBLE_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ DAY_OR_DOUBLE;
555550

556551
fragment DATE_SUFFIX: ([\-.][*0-9]+)+;
557552
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
@@ -332,7 +332,10 @@ timechartParameter
332332

333333
spanLiteral
334334
: SPANLENGTH
335+
| DECIMAL_SPANLENGTH
336+
| DOUBLE_LITERAL // 1.5d can also represent decimal span length
335337
| INTEGER_LITERAL
338+
| DECIMAL_LITERAL
336339
;
337340

338341
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
@@ -1656,6 +1656,19 @@ public void testVisitSpanLiteral() {
16561656
exprList(
16571657
argument("limit", intLiteral(10)), argument("useother", booleanLiteral(true))))
16581658
.build());
1659+
1660+
// Test span literal with decimal value
1661+
assertEqual(
1662+
"source=events_null | bin cpu_usage span=7.5 | stats count() by cpu_usage",
1663+
agg(
1664+
bin(
1665+
relation("events_null"),
1666+
field("cpu_usage"),
1667+
argument("span", decimalLiteral(new java.math.BigDecimal("7.5")))),
1668+
exprList(alias("count()", aggregate("count", allFields()))),
1669+
emptyList(),
1670+
exprList(alias("cpu_usage", field("cpu_usage"))),
1671+
defaultStatsArgs()));
16591672
}
16601673

16611674
@Test

0 commit comments

Comments
 (0)