From 7b9e6d92b36ffdebbae770ef7ef5e5440a1aa450 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 13 Nov 2025 06:55:57 +0000 Subject: [PATCH] Support using decimal as span literals (#4717) * Support decimal for span used in bin command Signed-off-by: Yuanchun Shen * Support decimal literal in span function Signed-off-by: Yuanchun Shen * Convert interval to double if it is BigDecimal for further calculation Signed-off-by: Yuanchun Shen * Make the error messages for decimal span length more meaningful Signed-off-by: Yuanchun Shen * Add mssing weeks to PLURAL_UNIT in ppl lexer Signed-off-by: Yuanchun Shen --------- Signed-off-by: Yuanchun Shen (cherry picked from commit a3c90a84730c6398428513e573495c75edc09297) Signed-off-by: github-actions[bot] --- .../expression/function/udf/SpanFunction.java | 20 ++++++++-- .../calcite/remote/CalciteBinCommandIT.java | 8 ++++ .../opensearch/sql/ppl/StatsCommandIT.java | 17 ++++++++ ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 39 ++++++++----------- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 3 ++ .../sql/ppl/parser/AstExpressionBuilder.java | 9 +++++ .../sql/ppl/parser/AstBuilderTest.java | 20 ++++++++++ .../ppl/parser/AstExpressionBuilderTest.java | 13 +++++++ 8 files changed, 103 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/SpanFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/SpanFunction.java index 190e3a54f46..1e7a355e5e2 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/SpanFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/SpanFunction.java @@ -20,7 +20,6 @@ import org.apache.calcite.schema.impl.ScalarFunctionImpl; import org.apache.calcite.sql.type.CompositeOperandTypeChecker; import org.apache.calcite.sql.type.OperandTypes; -import org.apache.calcite.sql.type.ReturnTypes; import org.apache.calcite.sql.type.SqlReturnTypeInference; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeFamily; @@ -45,7 +44,17 @@ public SpanFunction() { @Override public SqlReturnTypeInference getReturnTypeInference() { - return ReturnTypes.ARG0; + // Return arg0 type if it has a unit (i.e. time related span) + return callBinding -> { + if (SqlTypeUtil.isString(callBinding.getOperandType(2))) { + return callBinding.getOperandType(0); + } + // Use the least restrictive type between the field type and the interval type if it's a + // numeric span. E.g. span(int_field, double_literal) -> double + return callBinding + .getTypeFactory() + .leastRestrictive(List.of(callBinding.getOperandType(0), callBinding.getOperandType(1))); + }; } @Override @@ -57,10 +66,9 @@ public UDFOperandMetadata getOperandMetadata() { .or( OperandTypes.family( SqlTypeFamily.DATETIME, SqlTypeFamily.NUMERIC, SqlTypeFamily.CHARACTER)) - // TODO: numeric span should support decimal as its interval .or( OperandTypes.family( - SqlTypeFamily.NUMERIC, SqlTypeFamily.INTEGER, SqlTypeFamily.ANY))); + SqlTypeFamily.NUMERIC, SqlTypeFamily.NUMERIC, SqlTypeFamily.ANY))); } public static class SpanImplementor implements NotNullImplementor { @@ -73,8 +81,12 @@ public Expression implement( Expression interval = translatedOperands.get(1); RelDataType fieldType = call.getOperands().get(0).getType(); + RelDataType intervalType = call.getOperands().get(1).getType(); RelDataType unitType = call.getOperands().get(2).getType(); + if (SqlTypeUtil.isDecimal(intervalType)) { + interval = Expressions.call(interval, "doubleValue"); + } if (SqlTypeUtil.isNull(unitType)) { SqlTypeName sqlTypeName = call.getType().getSqlTypeName(); Expression result; diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java index bb326dc39a7..f2589720967 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteBinCommandIT.java @@ -1068,4 +1068,12 @@ public void testBinWithEvalCreatedDottedFieldName() throws IOException { rows(false, "go", "opentelemetry", 16, 1, "12-14"), rows(true, "rust", "opentelemetry", 12, 1, "14-16")); } + + @Test + public void testBinWithDecimalSpan() throws IOException { + JSONObject result = + executeQuery("source=events_null | bin cpu_usage span=7.5 | stats count() by cpu_usage"); + verifySchema(result, schema("count()", "bigint"), schema("cpu_usage", "string")); + verifyDataRows(result, rows(3, "37.5-45.0"), rows(2, "45.0-52.5"), rows(1, "52.5-60.0")); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/StatsCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/StatsCommandIT.java index da857179be7..b5b775c4ebe 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/StatsCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/StatsCommandIT.java @@ -1217,4 +1217,21 @@ public void testStatsSortOnMeasureComplex() throws IOException { resetQueryBucketSize(); } } + + @Test + public void testStatsByFractionalSpan() throws IOException { + JSONObject response1 = + executeQuery( + String.format( + "source=%s | stats count by span(balance, 4170.5)", + TEST_INDEX_BANK_WITH_NULL_VALUES)); + verifySchema(response1, schema("count", "bigint"), schema("span(balance,4170.5)", "double")); + verifyDataRows( + response1, + rows(3, null), + rows(1, 4170.5), + rows(1, 29193.5), + rows(1, 37534.5), + rows(1, 45875.5)); + } } diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 94ce9eb90f0..6a57e951594 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -521,40 +521,35 @@ ALIGNTIME: 'ALIGNTIME'; // Must precede ID to avoid conflicts with identifier matching PERCENTILE_SHORTCUT: PERC(INTEGER_LITERAL | DECIMAL_LITERAL) | 'P'(INTEGER_LITERAL | DECIMAL_LITERAL); -SPANLENGTH: [0-9]+ ( - 'US' |'CS'|'DS' - |'MS'|'MILLISECOND'|'MILLISECONDS' - |'S'|'SEC'|'SECS'|'SECOND'|'SECONDS' - |'MIN'|'MINS'|'MINUTE'|'MINUTES' - |'H'|'HR'|'HRS'|'HOUR'|'HOURS' - |'H'|'HR'|'HRS'|'HOUR'|'HOURS' - |'D'|'DAY'|'DAYS' - |'W'|'WEEK'|'WEEKS' - |'M'|'MON'|'MONTH'|'MONTHS' - |'Q'|'QTR'|'QTRS'|'QUARTER'|'QUARTERS' - |'Y'|'YR'|'YRS'|'YEAR'|'YEARS' -); +fragment DAY_OR_DOUBLE: 'D'; +fragment COMMON_TIME_UNIT: 'S'|'SEC'|'SECOND' + |'M'|'MIN'|'MINUTE' + |'H'|'HR'|'HOUR' + |'DAY'|'W'|'WEEK' + |'MON'|'MONTH' + |'Q'|'QTR'|'QUARTER' + |'Y'|'YR'|'YEAR'; +fragment PLURAL_UNIT: 'MILLISECONDS'|'SECS'|'SECONDS'|'MINS'|'MINUTES'|'HRS'|'HOURS' + |'DAYS'|'WEEKS'|'MONTHS'|'QTRS'|'QUARTERS'|'YRS'|'YEARS'; +fragment SPANUNIT: COMMON_TIME_UNIT | PLURAL_UNIT + |'US'|'CS'|'DS' + |'MS'|'MILLISECOND'; +SPANLENGTH: DEC_DIGIT+ (SPANUNIT | DAY_OR_DOUBLE); +DECIMAL_SPANLENGTH: (DEC_DIGIT+)? '.' DEC_DIGIT+ SPANUNIT; NUMERIC_ID : DEC_DIGIT+ ID_LITERAL; // LITERALS AND VALUES //STRING_LITERAL: DQUOTA_STRING | SQUOTA_STRING | BQUOTA_STRING; fragment WEEK_SNAP_UNIT: 'W' [0-7]; -fragment TIME_SNAP_UNIT: 'S' | 'SEC' | 'SECOND' - | 'M' | 'MIN' | 'MINUTE' - | 'H' | 'HR' | 'HOUR' | 'HOURS' - | 'D' | 'DAY' - | 'W' | 'WEEK' | WEEK_SNAP_UNIT - | 'MON' | 'MONTH' - | 'Q' | 'QTR' | 'QUARTER' - | 'Y' | 'YR' | 'YEAR'; +fragment TIME_SNAP_UNIT: COMMON_TIME_UNIT | WEEK_SNAP_UNIT | DAY_OR_DOUBLE; TIME_SNAP: AT TIME_SNAP_UNIT; ID: ID_LITERAL; CLUSTER: CLUSTER_PREFIX_LITERAL; INTEGER_LITERAL: DEC_DIGIT+; DECIMAL_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+; FLOAT_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ 'F'; -DOUBLE_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ 'D'; +DOUBLE_LITERAL: (DEC_DIGIT+)? '.' DEC_DIGIT+ DAY_OR_DOUBLE; fragment DATE_SUFFIX: ([\-.][*0-9]+)+; fragment CLUSTER_PREFIX_LITERAL: [*A-Z]+?[*A-Z_\-0-9]* COLON; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 86f4ef78f1b..efbf60552c8 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -336,7 +336,10 @@ timechartParameter spanLiteral : SPANLENGTH + | DECIMAL_SPANLENGTH + | DOUBLE_LITERAL // 1.5d can also represent decimal span length | INTEGER_LITERAL + | DECIMAL_LITERAL ; evalCommand diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index a46b35cc4b0..29d0e1cf873 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -824,6 +824,15 @@ private List multiFieldRelevanceArguments( public UnresolvedExpression visitSpanLiteral(OpenSearchPPLParser.SpanLiteralContext ctx) { if (ctx.INTEGER_LITERAL() != null) { return AstDSL.intLiteral(Integer.parseInt(ctx.INTEGER_LITERAL().getText())); + } else if (ctx.DECIMAL_LITERAL() != null) { + return AstDSL.decimalLiteral(new BigDecimal(ctx.DECIMAL_LITERAL().getText())); + } else if (ctx.DECIMAL_SPANLENGTH() != null || ctx.DOUBLE_LITERAL() != null) { + throw new IllegalArgumentException( + StringUtils.format( + "Span length [%s] is invalid: floating-point time intervals are not supported.", + ctx.DECIMAL_SPANLENGTH() != null + ? ctx.DECIMAL_SPANLENGTH().getText() + : ctx.DOUBLE_LITERAL().getText())); } else { return AstDSL.stringLiteral(ctx.getText()); } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java index 110b4bd817f..8ef097b43fc 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstBuilderTest.java @@ -1606,4 +1606,24 @@ public void testChartCommandWithBottomLimit() { exprList(argument("limit", intLiteral(3)), argument("top", booleanLiteral(false)))) .build()); } + + @Test + public void testTimeSpanWithDecimalShouldThrow() { + Throwable t1 = + assertThrows( + IllegalArgumentException.class, () -> plan("source=t | timechart span=1.5d count")); + assertTrue( + t1.getMessage() + .contains( + "Span length [1.5d] is invalid: floating-point time intervals are not supported.")); + + Throwable t2 = + assertThrows( + IllegalArgumentException.class, + () -> plan("source=t | stats count by span(@timestamp, 2.5y)")); + assertTrue( + t2.getMessage() + .contains( + "Span length [2.5y] is invalid: floating-point time intervals are not supported.")); + } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java index bbf1d986015..56151e8d3a9 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java @@ -1655,6 +1655,19 @@ public void testVisitSpanLiteral() { exprList( argument("limit", intLiteral(10)), argument("useother", booleanLiteral(true)))) .build()); + + // Test span literal with decimal value + assertEqual( + "source=events_null | bin cpu_usage span=7.5 | stats count() by cpu_usage", + agg( + bin( + relation("events_null"), + field("cpu_usage"), + argument("span", decimalLiteral(new java.math.BigDecimal("7.5")))), + exprList(alias("count()", aggregate("count", allFields()))), + emptyList(), + exprList(alias("cpu_usage", field("cpu_usage"))), + defaultStatsArgs())); } @Test