From 29d4bf8bacddf83b639efaedac22a8a2f305dde3 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 30 Sep 2025 09:40:00 -0700 Subject: [PATCH 01/26] Add grammar and AST for per_second function with UT and IT Signed-off-by: Chen Dai --- .../remote/CalciteTimechartCommandIT.java | 42 +++++++++++++ ppl/src/main/antlr/OpenSearchPPLParser.g4 | 5 ++ .../sql/ppl/parser/AstExpressionBuilder.java | 59 +++++++++++++++++++ .../sql/ppl/antlr/PPLSyntaxParserTest.java | 6 ++ .../sql/ppl/parser/AstBuilderTest.java | 23 ++++++++ 5 files changed, 135 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java index 3b4ca27dab5..02a9aa33250 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java @@ -483,6 +483,48 @@ private void createEventsManyHostsIndex() throws IOException { } } + @Test + public void testTimechartPerSecond() throws IOException { + JSONObject result = executeQuery("source=events | timechart span=1m per_second(cpu_usage)"); + + // Verify schema - per_second should return a double value + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + + // The test data should have some results, exact values depend on the test data + // but we can verify that results are returned and are numeric + assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); + + // Verify that per_second values are calculated correctly (should be sum/factor where factor = + // 60 for 1 minute) + // The exact verification of values would depend on the specific test data in events index + assertEquals(5, result.getInt("total")); + } + + @Test + public void testTimechartPerSecondWithHourSpan() throws IOException { + JSONObject result = executeQuery("source=events | timechart span=1h per_second(cpu_usage)"); + + // Verify schema + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + + // With 1 hour span, the factor should be 3600 seconds + assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); + } + + @Test + public void testTimechartPerSecondWithDaySpan() throws IOException { + JSONObject result = executeQuery("source=events | timechart span=1d per_second(cpu_usage)"); + + // Verify schema + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + + // With 1 day span, the factor should be 86400 seconds + assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); + } + private void createEventsNullIndex() throws IOException { String eventsMapping = "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"host\":{\"type\":\"text\"},\"cpu_usage\":{\"type\":\"double\"},\"region\":{\"type\":\"keyword\"}}}}"; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 87230eedd63..ffd7c7f8bbc 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -604,6 +604,7 @@ statsFunction | (DISTINCT_COUNT | DC | DISTINCT_COUNT_APPROX) LT_PRTHS valueExpression RT_PRTHS # distinctCountFunctionCall | takeAggFunction # takeAggFunctionCall | percentileApproxFunction # percentileApproxFunctionCall + | perFunction # perFunctionCall | statsFunctionName LT_PRTHS functionArgs RT_PRTHS # statsFunctionCall ; @@ -636,6 +637,10 @@ percentileApproxFunction COMMA percent = numericLiteral (COMMA compression = numericLiteral)? RT_PRTHS ; +perFunction + : PER_SECOND LT_PRTHS functionArg RT_PRTHS + ; + numericLiteral : integerLiteral | decimalLiteral 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 c055a95d55b..920033edc96 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 @@ -21,6 +21,7 @@ import java.util.stream.Stream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.RuleContext; +import org.antlr.v4.runtime.tree.ParseTree; import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Alias; @@ -56,8 +57,11 @@ import org.opensearch.sql.ast.expression.subquery.InSubquery; import org.opensearch.sql.ast.expression.subquery.ScalarSubquery; import org.opensearch.sql.ast.tree.Trendline; +import org.opensearch.sql.calcite.utils.binning.time.TimeUnitConfig; +import org.opensearch.sql.calcite.utils.binning.time.TimeUnitRegistry; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.utils.StringUtils; +import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BinaryArithmeticContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BooleanLiteralContext; @@ -791,4 +795,59 @@ public UnresolvedExpression visitNumericSpanValue( public UnresolvedExpression visitLogWithBaseSpan(OpenSearchPPLParser.LogWithBaseSpanContext ctx) { return org.opensearch.sql.ast.dsl.AstDSL.stringLiteral(ctx.getText()); } + + @Override + public UnresolvedExpression visitPerFunctionCall(OpenSearchPPLParser.PerFunctionCallContext ctx) { + // Step 1: Check immediate parent is Timechart command + ParseTree parent = ctx.getParent(); + if (!(parent instanceof OpenSearchPPLParser.TimechartCommandContext)) { + throw new SemanticCheckException("per_second() can only be used within timechart commands"); + } + + OpenSearchPPLParser.TimechartCommandContext timechartCtx = + (OpenSearchPPLParser.TimechartCommandContext) parent; + + // Step 2: Extract span information from timechart context + int spanInterval = 1; // default + String spanUnit = "m"; // default to minutes + + // Parse timechart parameters to find span + for (OpenSearchPPLParser.TimechartParameterContext paramCtx : + timechartCtx.timechartParameter()) { + if (paramCtx.spanClause() != null) { + // Extract from span clause (e.g., span(@timestamp, 1, "h")) + OpenSearchPPLParser.SpanClauseContext spanCtx = paramCtx.spanClause(); + if (spanCtx.value != null) { + spanInterval = Integer.parseInt(spanCtx.value.getText()); + } + if (spanCtx.unit != null) { + spanUnit = StringUtils.unquoteText(spanCtx.unit.getText()); + } + } else if (paramCtx.spanLiteral() != null) { + // Extract from span literal (e.g., span=1h) + OpenSearchPPLParser.SpanLiteralContext spanLiteralCtx = paramCtx.spanLiteral(); + if (spanLiteralCtx.integerLiteral() != null) { + spanInterval = Integer.parseInt(spanLiteralCtx.integerLiteral().getText()); + } + if (spanLiteralCtx.timespanUnit() != null) { + spanUnit = spanLiteralCtx.timespanUnit().getText(); + } + } + } + + // Step 3: Use TimeUnitRegistry to calculate total seconds as factor + TimeUnitConfig timeConfig = TimeUnitRegistry.getConfig(spanUnit); + if (timeConfig == null) { + throw new SemanticCheckException("Unsupported time unit: " + spanUnit); + } + + // Factor is the total seconds (not reciprocal, as internal_per_function will do + // sum(field)/factor) + long totalSeconds = timeConfig.toSeconds(spanInterval); + + // Step 4: Extract field argument and return function + UnresolvedExpression field = visit(ctx.perFunction().functionArg()); + return new Function( + "internal_per_function", Arrays.asList(field, AstDSL.doubleLiteral((double) totalSeconds))); + } } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index e4d7f912d89..d5e4f363550 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -99,6 +99,12 @@ public void testSearchFieldsCommandCrossClusterShouldPass() { assertNotEquals(null, tree); } + @Test + public void testPerSecondFunctionInTimechartShouldPass() { + ParseTree tree = new PPLSyntaxParser().parse("source=t | timechart per_second(a)"); + assertNotEquals(null, tree); + } + @Test public void testDynamicSourceClauseParseTreeStructure() { String query = "source=[myindex, logs, fieldIndex=\"test\", count=100]"; 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 a22ed58b222..7470445df35 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 @@ -74,6 +74,7 @@ import org.opensearch.sql.ast.tree.Kmeans; import org.opensearch.sql.ast.tree.ML; import org.opensearch.sql.ast.tree.RareTopN.CommandType; +import org.opensearch.sql.ast.tree.Timechart; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.common.setting.Settings.Key; @@ -1102,4 +1103,26 @@ public void testRexSedModeWithOffsetFieldThrowsException() { // Test that SED mode and offset_field cannot be used together (align with Splunk behavior) plan("source=test | rex field=email mode=sed offset_field=matchpos \"s/@.*/@company.com/\""); } + + @Test + public void testTimechartWithPerSecondFunction() { + assertEqual( + "source=t | timechart per_second(a)", + new Timechart( + relation("t"), + function("internal_per_function", field("a"), AstDSL.doubleLiteral(60.0))) + .span(AstDSL.span(AstDSL.field("@timestamp"), AstDSL.intLiteral(1), SpanUnit.of("m"))) + .by(null) + .limit(10) + .useOther(true)); + } + + @Test + public void testPerSecondWithStatsThrowsException() { + org.opensearch.sql.exception.SemanticCheckException exception = + assertThrows( + org.opensearch.sql.exception.SemanticCheckException.class, + () -> plan("source=t | stats per_second(a)")); + assertEquals("per_second() can only be used within timechart commands", exception.getMessage()); + } } From 9aff3423d50e2f3ae23fefaf84a3e8d55edfb209 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 30 Sep 2025 15:37:38 -0700 Subject: [PATCH 02/26] Translate per_second function call to eval + timechart Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 84 ++++++++++++++++++- .../sql/ppl/parser/AstExpressionBuilder.java | 70 +++------------- .../sql/ppl/parser/AstBuilderTest.java | 40 ++++----- 3 files changed, 112 insertions(+), 82 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 1a4c154209c..094301765d3 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -6,13 +6,25 @@ package org.opensearch.sql.ast.tree; import com.google.common.collect.ImmutableList; +import java.util.Arrays; import java.util.List; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.ToString; import org.opensearch.sql.ast.AbstractNodeVisitor; +import org.opensearch.sql.ast.dsl.AstDSL; +import org.opensearch.sql.ast.expression.AggregateFunction; +import org.opensearch.sql.ast.expression.DataType; +import org.opensearch.sql.ast.expression.Field; +import org.opensearch.sql.ast.expression.Function; +import org.opensearch.sql.ast.expression.Let; +import org.opensearch.sql.ast.expression.Literal; +import org.opensearch.sql.ast.expression.Span; +import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.expression.UnresolvedExpression; +import org.opensearch.sql.calcite.utils.binning.time.TimeUnitConfig; +import org.opensearch.sql.calcite.utils.binning.time.TimeUnitRegistry; /** AST node represent Timechart operation. */ @Getter @@ -49,8 +61,8 @@ public Timechart useOther(Boolean useOther) { } @Override - public Timechart attach(UnresolvedPlan child) { - return toBuilder().child(child).build(); + public UnresolvedPlan attach(UnresolvedPlan child) { + return toBuilder().child(child).build().transformPerSecondFunctions(); } @Override @@ -62,4 +74,72 @@ public List getChild() { public T accept(AbstractNodeVisitor nodeVisitor, C context) { return nodeVisitor.visitTimechart(this, context); } + + /** + * Transform per_second functions into sum + eval pattern. Simple approach for single aggregation + * function only. + */ + public UnresolvedPlan transformPerSecondFunctions() { + if (this.aggregateFunction == null || !(this.aggregateFunction instanceof AggregateFunction)) { + return this; + } + + AggregateFunction aggFunc = (AggregateFunction) this.aggregateFunction; + if (!"per_second".equals(aggFunc.getFuncName())) { + return this; // No per_second function found + } + + // Extract interval seconds from binExpression + long intervalSec = extractIntervalSeconds(); + + // Per-second unit = 1 (later add per_minute=60, per_hour=3600, etc.) + long unit = 1; + + // Get the field from per_second(field) + UnresolvedExpression field = aggFunc.getField(); + + // Create sum(field) to replace per_second(field) + AggregateFunction sumFunc = new AggregateFunction("sum", field); + + // Create eval: per_second(field) = sum(field) / (intervalSec / unit) + String fieldName = + field instanceof Field ? ((Field) field).getField().toString() : field.toString(); + String originalName = "per_second(" + fieldName + ")"; + String sumName = "sum(" + fieldName + ")"; + + Let evalTransform = + new Let( + new Field(AstDSL.qualifiedName(originalName)), + new Function( + "/", + Arrays.asList( + new Field(AstDSL.qualifiedName(sumName)), + new Literal((double) (intervalSec / unit), DataType.DOUBLE)))); + + this.aggregateFunction = sumFunc; + + // Create eval that wraps the new timechart + Eval evalNode = new Eval(Arrays.asList(evalTransform)); + Eval newEval = evalNode.attach(this); + return newEval; + } + + private long extractIntervalSeconds() { + // Simple extraction assuming span=literal format + if (!(this.binExpression instanceof Span)) { + return 60L; // Default 1 minute + } + + Span span = (Span) this.binExpression; + int intervalValue = + ((Literal) span.getValue()).getValue() instanceof Number + ? ((Number) ((Literal) span.getValue()).getValue()).intValue() + : 1; + + String unitString = SpanUnit.getName(span.getUnit()); + + // Use existing TimeUnitRegistry + TimeUnitConfig config = TimeUnitRegistry.getConfig(unitString); + return config != null ? config.toSeconds(intervalValue) : 60L; + } } 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 920033edc96..5a5ab1f80ac 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 @@ -57,11 +57,8 @@ import org.opensearch.sql.ast.expression.subquery.InSubquery; import org.opensearch.sql.ast.expression.subquery.ScalarSubquery; import org.opensearch.sql.ast.tree.Trendline; -import org.opensearch.sql.calcite.utils.binning.time.TimeUnitConfig; -import org.opensearch.sql.calcite.utils.binning.time.TimeUnitRegistry; import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.common.utils.StringUtils; -import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BinaryArithmeticContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.BooleanLiteralContext; @@ -90,6 +87,7 @@ import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.LogicalOrContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.LogicalXorContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.MultiFieldRelevanceFunctionContext; +import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.PerFunctionCallContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.RenameFieldExpressionContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SingleFieldRelevanceFunctionContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SortFieldContext; @@ -97,6 +95,7 @@ import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.StatsFunctionCallContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.StringLiteralContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.TableSourceContext; +import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.TimechartCommandContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.WcFieldExpressionContext; import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParserBaseVisitor; import org.opensearch.sql.ppl.utils.ArgumentFactory; @@ -539,6 +538,16 @@ private List timestampFunctionArguments( return args; } + @Override + public UnresolvedExpression visitPerFunctionCall(PerFunctionCallContext ctx) { + ParseTree parent = ctx.getParent(); + if (!(parent instanceof TimechartCommandContext)) { + throw new SyntaxCheckException( + "per_second function can only be used within timechart command"); + } + return buildAggregateFunction("per_second", Arrays.asList(ctx.perFunction().functionArg())); + } + /** Literal and value. */ @Override public UnresolvedExpression visitIdentsAsQualifiedName(IdentsAsQualifiedNameContext ctx) { @@ -795,59 +804,4 @@ public UnresolvedExpression visitNumericSpanValue( public UnresolvedExpression visitLogWithBaseSpan(OpenSearchPPLParser.LogWithBaseSpanContext ctx) { return org.opensearch.sql.ast.dsl.AstDSL.stringLiteral(ctx.getText()); } - - @Override - public UnresolvedExpression visitPerFunctionCall(OpenSearchPPLParser.PerFunctionCallContext ctx) { - // Step 1: Check immediate parent is Timechart command - ParseTree parent = ctx.getParent(); - if (!(parent instanceof OpenSearchPPLParser.TimechartCommandContext)) { - throw new SemanticCheckException("per_second() can only be used within timechart commands"); - } - - OpenSearchPPLParser.TimechartCommandContext timechartCtx = - (OpenSearchPPLParser.TimechartCommandContext) parent; - - // Step 2: Extract span information from timechart context - int spanInterval = 1; // default - String spanUnit = "m"; // default to minutes - - // Parse timechart parameters to find span - for (OpenSearchPPLParser.TimechartParameterContext paramCtx : - timechartCtx.timechartParameter()) { - if (paramCtx.spanClause() != null) { - // Extract from span clause (e.g., span(@timestamp, 1, "h")) - OpenSearchPPLParser.SpanClauseContext spanCtx = paramCtx.spanClause(); - if (spanCtx.value != null) { - spanInterval = Integer.parseInt(spanCtx.value.getText()); - } - if (spanCtx.unit != null) { - spanUnit = StringUtils.unquoteText(spanCtx.unit.getText()); - } - } else if (paramCtx.spanLiteral() != null) { - // Extract from span literal (e.g., span=1h) - OpenSearchPPLParser.SpanLiteralContext spanLiteralCtx = paramCtx.spanLiteral(); - if (spanLiteralCtx.integerLiteral() != null) { - spanInterval = Integer.parseInt(spanLiteralCtx.integerLiteral().getText()); - } - if (spanLiteralCtx.timespanUnit() != null) { - spanUnit = spanLiteralCtx.timespanUnit().getText(); - } - } - } - - // Step 3: Use TimeUnitRegistry to calculate total seconds as factor - TimeUnitConfig timeConfig = TimeUnitRegistry.getConfig(spanUnit); - if (timeConfig == null) { - throw new SemanticCheckException("Unsupported time unit: " + spanUnit); - } - - // Factor is the total seconds (not reciprocal, as internal_per_function will do - // sum(field)/factor) - long totalSeconds = timeConfig.toSeconds(spanInterval); - - // Step 4: Extract field argument and return function - UnresolvedExpression field = visit(ctx.perFunction().functionArg()); - return new Function( - "internal_per_function", Arrays.asList(field, AstDSL.doubleLiteral((double) totalSeconds))); - } } 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 7470445df35..7da849d14fa 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 @@ -22,6 +22,7 @@ import static org.opensearch.sql.ast.dsl.AstDSL.defaultSortFieldArgs; import static org.opensearch.sql.ast.dsl.AstDSL.defaultStatsArgs; import static org.opensearch.sql.ast.dsl.AstDSL.describe; +import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.eval; import static org.opensearch.sql.ast.dsl.AstDSL.exprList; import static org.opensearch.sql.ast.dsl.AstDSL.field; @@ -1073,6 +1074,23 @@ public void testPatternsWithoutArguments() { ImmutableMap.of())); } + @Test + public void testTimechartWithPerSecondFunction() { + assertEqual( + "source=t | timechart per_second(a)", + eval( + new Timechart(relation("t"), aggregate("sum", field("a"))) + .span(span(field("@timestamp"), intLiteral(1), SpanUnit.of("m"))) + .limit(10) + .useOther(true), + let(field("per_second(a)"), function("/", field("sum(a)"), doubleLiteral(60.0))))); + } + + @Test + public void testStatsWithPerSecondThrowsException() { + assertThrows(SyntaxCheckException.class, () -> plan("source=t | stats per_second(a)")); + } + protected void assertEqual(String query, Node expectedPlan) { Node actualPlan = plan(query); assertEquals(expectedPlan, actualPlan); @@ -1103,26 +1121,4 @@ public void testRexSedModeWithOffsetFieldThrowsException() { // Test that SED mode and offset_field cannot be used together (align with Splunk behavior) plan("source=test | rex field=email mode=sed offset_field=matchpos \"s/@.*/@company.com/\""); } - - @Test - public void testTimechartWithPerSecondFunction() { - assertEqual( - "source=t | timechart per_second(a)", - new Timechart( - relation("t"), - function("internal_per_function", field("a"), AstDSL.doubleLiteral(60.0))) - .span(AstDSL.span(AstDSL.field("@timestamp"), AstDSL.intLiteral(1), SpanUnit.of("m"))) - .by(null) - .limit(10) - .useOther(true)); - } - - @Test - public void testPerSecondWithStatsThrowsException() { - org.opensearch.sql.exception.SemanticCheckException exception = - assertThrows( - org.opensearch.sql.exception.SemanticCheckException.class, - () -> plan("source=t | stats per_second(a)")); - assertEquals("per_second() can only be used within timechart commands", exception.getMessage()); - } } From 86920fa988645808d811d67572e7ab0dcfc721b9 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 1 Oct 2025 12:24:20 -0700 Subject: [PATCH 03/26] Add SparkSQL verify test Signed-off-by: Chen Dai --- .../sql/ppl/calcite/CalcitePPLTimechartTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java index 703850ccfff..108e298cd4a 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java @@ -81,6 +81,19 @@ public void testTimechartBasic() { verifyPPLToSparkSQL(root, expectedSparkSql); } + @Test + public void testTimechartPerSecond() { + withPPLQuery("source=events | timechart per_second(cpu_usage)") + .expectSparkSQL( + "SELECT `@timestamp`, `sum(cpu_usage)`, `DIVIDE`(`sum(cpu_usage)`, 6.00E1)" + + " `per_second(cpu_usage)`\n" + + "FROM (SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, SUM(`cpu_usage`)" + + " `sum(cpu_usage)`\n" + + "FROM `scott`.`events`\n" + + "GROUP BY `SPAN`(`@timestamp`, 1, 'm')\n" + + "ORDER BY 1 NULLS LAST) `t2`"); + } + @Test public void testTimechartWithSpan() { String ppl = "source=events | timechart span=1h count()"; From 1276017d9a625102fd2c14e95b9f3d6b56876255 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 2 Oct 2025 09:16:43 -0700 Subject: [PATCH 04/26] Fix aggregate function alias issue for timechart visit Signed-off-by: Chen Dai --- .../org/opensearch/sql/calcite/CalciteRelNodeVisitor.java | 3 +++ .../sql/ppl/calcite/CalcitePPLTimechartTest.java | 8 +++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java index 211d6bf1a27..96eae7932bc 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java @@ -1681,6 +1681,9 @@ public RelNode visitFlatten(Flatten node, CalcitePlanContext context) { /** Helper method to get the function name for proper column naming */ private String getValueFunctionName(UnresolvedExpression aggregateFunction) { + if (aggregateFunction instanceof Alias) { + return ((Alias) aggregateFunction).getName(); + } if (!(aggregateFunction instanceof AggregateFunction)) { return "value"; } diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java index 108e298cd4a..a559146cf01 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java @@ -85,13 +85,11 @@ public void testTimechartBasic() { public void testTimechartPerSecond() { withPPLQuery("source=events | timechart per_second(cpu_usage)") .expectSparkSQL( - "SELECT `@timestamp`, `sum(cpu_usage)`, `DIVIDE`(`sum(cpu_usage)`, 6.00E1)" - + " `per_second(cpu_usage)`\n" - + "FROM (SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, SUM(`cpu_usage`)" - + " `sum(cpu_usage)`\n" + "SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, `internal_per_function`(`cpu_usage`," + + " 's', 6.00E1) `per_second(cpu_usage)`\n" + "FROM `scott`.`events`\n" + "GROUP BY `SPAN`(`@timestamp`, 1, 'm')\n" - + "ORDER BY 1 NULLS LAST) `t2`"); + + "ORDER BY 1 NULLS LAST"); } @Test From 0ae12ffa293aaab2d1ec6e57299746e30b096b45 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 2 Oct 2025 11:48:27 -0700 Subject: [PATCH 05/26] Hide internal sum aggregation in output Signed-off-by: Chen Dai --- .../org/opensearch/sql/ast/tree/Timechart.java | 16 ++++++++++------ .../sql/ppl/calcite/CalcitePPLTimechartTest.java | 8 +++++--- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 094301765d3..e920ef8b4a7 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -15,6 +15,7 @@ import org.opensearch.sql.ast.AbstractNodeVisitor; import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; +import org.opensearch.sql.ast.expression.Alias; import org.opensearch.sql.ast.expression.DataType; import org.opensearch.sql.ast.expression.Field; import org.opensearch.sql.ast.expression.Function; @@ -98,13 +99,16 @@ public UnresolvedPlan transformPerSecondFunctions() { // Get the field from per_second(field) UnresolvedExpression field = aggFunc.getField(); - // Create sum(field) to replace per_second(field) - AggregateFunction sumFunc = new AggregateFunction("sum", field); - - // Create eval: per_second(field) = sum(field) / (intervalSec / unit) + // Get the field from per_second(field) and create original name String fieldName = field instanceof Field ? ((Field) field).getField().toString() : field.toString(); String originalName = "per_second(" + fieldName + ")"; + + // Create sum(field) wrapped in alias with original name to preserve column naming + AggregateFunction sumFunc = new AggregateFunction("sum", field); + Alias aliasedSumFunc = new Alias(originalName, sumFunc); + + // Create eval: per_second(field) = sum(field) / (intervalSec / unit) String sumName = "sum(" + fieldName + ")"; Let evalTransform = @@ -113,10 +117,10 @@ public UnresolvedPlan transformPerSecondFunctions() { new Function( "/", Arrays.asList( - new Field(AstDSL.qualifiedName(sumName)), + new Field(AstDSL.qualifiedName(originalName)), new Literal((double) (intervalSec / unit), DataType.DOUBLE)))); - this.aggregateFunction = sumFunc; + this.aggregateFunction = aliasedSumFunc; // Create eval that wraps the new timechart Eval evalNode = new Eval(Arrays.asList(evalTransform)); diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java index a559146cf01..5f0b86be6a8 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java @@ -85,11 +85,13 @@ public void testTimechartBasic() { public void testTimechartPerSecond() { withPPLQuery("source=events | timechart per_second(cpu_usage)") .expectSparkSQL( - "SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, `internal_per_function`(`cpu_usage`," - + " 's', 6.00E1) `per_second(cpu_usage)`\n" + "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)`, 6.00E1)" + + " `per_second(cpu_usage)`\n" + + "FROM (SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, SUM(`cpu_usage`)" + + " `per_second(cpu_usage)`\n" + "FROM `scott`.`events`\n" + "GROUP BY `SPAN`(`@timestamp`, 1, 'm')\n" - + "ORDER BY 1 NULLS LAST"); + + "ORDER BY 1 NULLS LAST) `t2`"); } @Test From 3caab3c742e1aa4373665e444b302c0f8fdc8cb6 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 2 Oct 2025 17:32:43 -0700 Subject: [PATCH 06/26] Leverage AST DSL to make rewrite more readable Signed-off-by: Chen Dai --- .../org/opensearch/sql/ast/dsl/AstDSL.java | 14 ++-- .../opensearch/sql/ast/tree/Timechart.java | 81 +++++++------------ .../sql/ppl/parser/AstBuilderTest.java | 6 +- 3 files changed, 38 insertions(+), 63 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java index a4c5b2f5e01..226465bbeeb 100644 --- a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java @@ -390,31 +390,31 @@ public AllFields allFields() { return AllFields.of(); } - public Field field(UnresolvedExpression field) { + public static Field field(UnresolvedExpression field) { return new Field(field); } - public Field field(UnresolvedExpression field, Argument... fieldArgs) { + public static Field field(UnresolvedExpression field, Argument... fieldArgs) { return field(field, Arrays.asList(fieldArgs)); } - public Field field(String field) { + public static Field field(String field) { return field(qualifiedName(field)); } - public Field field(String field, Argument... fieldArgs) { + public static Field field(String field, Argument... fieldArgs) { return field(field, Arrays.asList(fieldArgs)); } - public Field field(UnresolvedExpression field, List fieldArgs) { + public static Field field(UnresolvedExpression field, List fieldArgs) { return new Field(field, fieldArgs); } - public Field field(String field, List fieldArgs) { + public static Field field(String field, List fieldArgs) { return field(qualifiedName(field), fieldArgs); } - public Alias alias(String name, UnresolvedExpression expr) { + public static Alias alias(String name, UnresolvedExpression expr) { return new Alias(name, expr); } diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index e920ef8b4a7..358d1ae9ea0 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -5,21 +5,23 @@ package org.opensearch.sql.ast.tree; +import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; +import static org.opensearch.sql.ast.dsl.AstDSL.alias; +import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.eval; +import static org.opensearch.sql.ast.dsl.AstDSL.field; +import static org.opensearch.sql.ast.dsl.AstDSL.function; +import static org.opensearch.sql.ast.dsl.AstDSL.let; + import com.google.common.collect.ImmutableList; -import java.util.Arrays; import java.util.List; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.ToString; import org.opensearch.sql.ast.AbstractNodeVisitor; -import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; -import org.opensearch.sql.ast.expression.Alias; -import org.opensearch.sql.ast.expression.DataType; import org.opensearch.sql.ast.expression.Field; -import org.opensearch.sql.ast.expression.Function; -import org.opensearch.sql.ast.expression.Let; import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; @@ -81,68 +83,39 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { * function only. */ public UnresolvedPlan transformPerSecondFunctions() { - if (this.aggregateFunction == null || !(this.aggregateFunction instanceof AggregateFunction)) { - return this; - } - AggregateFunction aggFunc = (AggregateFunction) this.aggregateFunction; if (!"per_second".equals(aggFunc.getFuncName())) { - return this; // No per_second function found + return this; } - // Extract interval seconds from binExpression - long intervalSec = extractIntervalSeconds(); - - // Per-second unit = 1 (later add per_minute=60, per_hour=3600, etc.) - long unit = 1; - - // Get the field from per_second(field) - UnresolvedExpression field = aggFunc.getField(); - - // Get the field from per_second(field) and create original name - String fieldName = - field instanceof Field ? ((Field) field).getField().toString() : field.toString(); + String fieldName = extractFieldName(aggFunc.getField()); String originalName = "per_second(" + fieldName + ")"; + double divisor = (double) (extractIntervalSeconds() / 1); // unit = 1 for per_second + return eval( + timechart(alias(originalName, sum(aggFunc.getField()))), + let(field(originalName), divide(field(originalName), doubleLiteral(divisor)))); + } - // Create sum(field) wrapped in alias with original name to preserve column naming - AggregateFunction sumFunc = new AggregateFunction("sum", field); - Alias aliasedSumFunc = new Alias(originalName, sumFunc); - - // Create eval: per_second(field) = sum(field) / (intervalSec / unit) - String sumName = "sum(" + fieldName + ")"; + private String extractFieldName(UnresolvedExpression field) { + return field instanceof Field ? ((Field) field).getField().toString() : field.toString(); + } - Let evalTransform = - new Let( - new Field(AstDSL.qualifiedName(originalName)), - new Function( - "/", - Arrays.asList( - new Field(AstDSL.qualifiedName(originalName)), - new Literal((double) (intervalSec / unit), DataType.DOUBLE)))); + private UnresolvedExpression sum(UnresolvedExpression field) { + return aggregate("sum", field); + } - this.aggregateFunction = aliasedSumFunc; + private UnresolvedExpression divide(UnresolvedExpression left, UnresolvedExpression right) { + return function("/", left, right); + } - // Create eval that wraps the new timechart - Eval evalNode = new Eval(Arrays.asList(evalTransform)); - Eval newEval = evalNode.attach(this); - return newEval; + private Timechart timechart(UnresolvedExpression newAggregateFunction) { + return this.toBuilder().aggregateFunction(newAggregateFunction).build(); } private long extractIntervalSeconds() { - // Simple extraction assuming span=literal format - if (!(this.binExpression instanceof Span)) { - return 60L; // Default 1 minute - } - Span span = (Span) this.binExpression; - int intervalValue = - ((Literal) span.getValue()).getValue() instanceof Number - ? ((Number) ((Literal) span.getValue()).getValue()).intValue() - : 1; - + int intervalValue = ((Number) ((Literal) span.getValue()).getValue()).intValue(); String unitString = SpanUnit.getName(span.getUnit()); - - // Use existing TimeUnitRegistry TimeUnitConfig config = TimeUnitRegistry.getConfig(unitString); return config != null ? config.toSeconds(intervalValue) : 60L; } 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 7da849d14fa..99738a5c88f 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 @@ -1079,11 +1079,13 @@ public void testTimechartWithPerSecondFunction() { assertEqual( "source=t | timechart per_second(a)", eval( - new Timechart(relation("t"), aggregate("sum", field("a"))) + new Timechart(relation("t"), alias("per_second(a)", aggregate("sum", field("a")))) .span(span(field("@timestamp"), intLiteral(1), SpanUnit.of("m"))) .limit(10) .useOther(true), - let(field("per_second(a)"), function("/", field("sum(a)"), doubleLiteral(60.0))))); + let( + field("per_second(a)"), + function("/", field("per_second(a)"), doubleLiteral(60.0))))); } @Test From 875d88319f11eb62246daeb8039fdcfab15cc7f5 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Mon, 6 Oct 2025 14:51:34 -0700 Subject: [PATCH 07/26] Add UT for transform per function in Timechart AST Signed-off-by: Chen Dai --- .../org/opensearch/sql/ast/dsl/AstDSL.java | 14 +- .../opensearch/sql/ast/tree/Timechart.java | 17 +- .../sql/ast/tree/TimechartTest.java | 187 ++++++++++++++++++ 3 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java diff --git a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java index 226465bbeeb..a4c5b2f5e01 100644 --- a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java @@ -390,31 +390,31 @@ public AllFields allFields() { return AllFields.of(); } - public static Field field(UnresolvedExpression field) { + public Field field(UnresolvedExpression field) { return new Field(field); } - public static Field field(UnresolvedExpression field, Argument... fieldArgs) { + public Field field(UnresolvedExpression field, Argument... fieldArgs) { return field(field, Arrays.asList(fieldArgs)); } - public static Field field(String field) { + public Field field(String field) { return field(qualifiedName(field)); } - public static Field field(String field, Argument... fieldArgs) { + public Field field(String field, Argument... fieldArgs) { return field(field, Arrays.asList(fieldArgs)); } - public static Field field(UnresolvedExpression field, List fieldArgs) { + public Field field(UnresolvedExpression field, List fieldArgs) { return new Field(field, fieldArgs); } - public static Field field(String field, List fieldArgs) { + public Field field(String field, List fieldArgs) { return field(qualifiedName(field), fieldArgs); } - public static Alias alias(String name, UnresolvedExpression expr) { + public Alias alias(String name, UnresolvedExpression expr) { return new Alias(name, expr); } diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 358d1ae9ea0..dcd6b9484a6 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -6,10 +6,8 @@ package org.opensearch.sql.ast.tree; import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; -import static org.opensearch.sql.ast.dsl.AstDSL.alias; import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.eval; -import static org.opensearch.sql.ast.dsl.AstDSL.field; import static org.opensearch.sql.ast.dsl.AstDSL.function; import static org.opensearch.sql.ast.dsl.AstDSL.let; @@ -20,6 +18,7 @@ import lombok.Getter; import lombok.ToString; import org.opensearch.sql.ast.AbstractNodeVisitor; +import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Field; import org.opensearch.sql.ast.expression.Literal; @@ -92,22 +91,16 @@ public UnresolvedPlan transformPerSecondFunctions() { String originalName = "per_second(" + fieldName + ")"; double divisor = (double) (extractIntervalSeconds() / 1); // unit = 1 for per_second return eval( - timechart(alias(originalName, sum(aggFunc.getField()))), - let(field(originalName), divide(field(originalName), doubleLiteral(divisor)))); + timechart(AstDSL.alias(originalName, aggregate("sum", aggFunc.getField()))), + let( + AstDSL.field(originalName), + function("/", AstDSL.field(originalName), doubleLiteral(divisor)))); } private String extractFieldName(UnresolvedExpression field) { return field instanceof Field ? ((Field) field).getField().toString() : field.toString(); } - private UnresolvedExpression sum(UnresolvedExpression field) { - return aggregate("sum", field); - } - - private UnresolvedExpression divide(UnresolvedExpression left, UnresolvedExpression right) { - return function("/", left, right); - } - private Timechart timechart(UnresolvedExpression newAggregateFunction) { return this.toBuilder().aggregateFunction(newAggregateFunction).build(); } diff --git a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java new file mode 100644 index 00000000000..83da4a198ca --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java @@ -0,0 +1,187 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ast.tree; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; +import static org.opensearch.sql.ast.dsl.AstDSL.alias; +import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.field; +import static org.opensearch.sql.ast.dsl.AstDSL.function; +import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral; +import static org.opensearch.sql.ast.dsl.AstDSL.relation; + +import org.junit.jupiter.api.Test; +import org.opensearch.sql.ast.dsl.AstDSL; +import org.opensearch.sql.ast.expression.AggregateFunction; +import org.opensearch.sql.ast.expression.Let; +import org.opensearch.sql.ast.expression.Span; +import org.opensearch.sql.ast.expression.SpanUnit; +import org.opensearch.sql.ast.expression.UnresolvedExpression; + +class TimechartTest { + + @Test + void should_transform_per_second_function_to_sum_and_eval() { + withTimechart(span(1, "m"), per_second("bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(bytes)", divide("per_second(bytes)", 60.0)), + timechart(span(1, "m"), alias("per_second(bytes)", sum("bytes"))))); + } + + @Test + void should_not_transform_non_per_second_functions() { + Timechart timechart = + new Timechart( + relation("t"), + AstDSL.span(field("@timestamp"), intLiteral(1), SpanUnit.of("m")), + aggregate("count", field("bytes")), + null, + 10, + true); + + UnresolvedPlan result = timechart.transformPerSecondFunctions(); + + assertSame(timechart, result); + } + + @Test + void should_calculate_correct_divisor_for_per_second_with_seconds_span() { + withTimechart(span(30, "s"), per_second("bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(bytes)", divide("per_second(bytes)", 30.0)), + timechart(span(30, "s"), alias("per_second(bytes)", sum("bytes"))))); + } + + @Test + void should_calculate_correct_divisor_for_per_second_with_minutes_span() { + withTimechart(span(5, "m"), per_second("bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(bytes)", divide("per_second(bytes)", 300.0)), + timechart(span(5, "m"), alias("per_second(bytes)", sum("bytes"))))); + } + + @Test + void should_calculate_correct_divisor_for_per_second_with_hours_span() { + withTimechart(span(2, "h"), per_second("bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(bytes)", divide("per_second(bytes)", 7200.0)), + timechart(span(2, "h"), alias("per_second(bytes)", sum("bytes"))))); + } + + @Test + void should_calculate_correct_divisor_for_per_second_with_days_span() { + withTimechart(span(1, "d"), per_second("bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(bytes)", divide("per_second(bytes)", 86400.0)), + timechart(span(1, "d"), alias("per_second(bytes)", sum("bytes"))))); + } + + @Test + void should_use_default_divisor_for_unsupported_span_units() { + withTimechart( + AstDSL.span(field("@timestamp"), intLiteral(1), SpanUnit.NONE), per_second("bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(bytes)", divide("per_second(bytes)", 60.0)), + timechart( + AstDSL.span(field("@timestamp"), intLiteral(1), SpanUnit.NONE), + alias("per_second(bytes)", sum("bytes"))))); + } + + @Test + void should_handle_qualified_field_names_in_transformation() { + withTimechart(span(5, "m"), per_second("server.bytes")) + .transformPerFunction() + .shouldBe( + eval( + let("per_second(server.bytes)", divide("per_second(server.bytes)", 300.0)), + timechart(span(5, "m"), alias("per_second(server.bytes)", sum("server.bytes"))))); + } + + // Fluent API for readable test assertions + + private static TransformationAssertion withTimechart(Span spanExpr, AggregateFunction aggFunc) { + return new TransformationAssertion(timechart(spanExpr, aggFunc)); + } + + private static Timechart timechart(Span spanExpr, UnresolvedExpression aggExpr) { + return new Timechart(relation("t"), aggExpr).span(spanExpr).limit(10).useOther(true); + } + + private static Span span(int value, String unit) { + return AstDSL.span(field("@timestamp"), intLiteral(value), SpanUnit.of(unit)); + } + + private static AggregateFunction per_second(String fieldName) { + return (AggregateFunction) aggregate("per_second", field(fieldName)); + } + + private static AggregateFunction sum(String fieldName) { + return (AggregateFunction) aggregate("sum", field(fieldName)); + } + + private static Let let(String fieldName, UnresolvedExpression expression) { + return AstDSL.let(field(fieldName), expression); + } + + private static UnresolvedExpression divide(String fieldName, double divisor) { + return function("/", field(fieldName), doubleLiteral(divisor)); + } + + private static UnresolvedPlan eval(Let letExpr, Timechart timechartExpr) { + return AstDSL.eval(timechartExpr, letExpr); + } + + private static class TransformationAssertion { + private final Timechart timechart; + private UnresolvedPlan result; + + TransformationAssertion(Timechart timechart) { + this.timechart = timechart; + } + + public TransformationAssertion transformPerFunction() { + this.result = timechart.attach(null); + return this; + } + + public void shouldBe(UnresolvedPlan expected) { + // Verify transformation produces expected type and structure + assertEquals(expected.getClass(), result.getClass()); + if (expected instanceof Eval && result instanceof Eval) { + Eval expectedEval = (Eval) expected; + Eval actualEval = (Eval) result; + assertEquals( + expectedEval.getExpressionList().size(), actualEval.getExpressionList().size()); + } + } + } + + // TODO: Add tests for per_minute, per_hour, and per_day functions when implemented + // Example structure for future functions: + // + // @Test + // void should_transform_per_minute_function() { + // withTimechart(span(2, "h"), per_minute("bytes")) + // .transformPerFunction() + // .shouldBe(evalWith( + // let("per_minute(bytes)", divide("per_minute(bytes)", 120.0)), + // timechart(span(2, "h"), alias("per_minute(bytes)", sum("bytes"))))); + // } +} From 346cc5ec65acf87dac45c333068aab85ec2a105f Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Mon, 6 Oct 2025 15:12:41 -0700 Subject: [PATCH 08/26] Refactor UT with parameterized test Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 2 +- .../sql/ast/tree/TimechartTest.java | 103 ++---------------- 2 files changed, 12 insertions(+), 93 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index dcd6b9484a6..a8d283b17ac 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -81,7 +81,7 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { * Transform per_second functions into sum + eval pattern. Simple approach for single aggregation * function only. */ - public UnresolvedPlan transformPerSecondFunctions() { + private UnresolvedPlan transformPerSecondFunctions() { AggregateFunction aggFunc = (AggregateFunction) this.aggregateFunction; if (!"per_second".equals(aggFunc.getFuncName())) { return this; diff --git a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java index 83da4a198ca..7d1290d62d1 100644 --- a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java @@ -6,7 +6,6 @@ package org.opensearch.sql.ast.tree; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertSame; import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; import static org.opensearch.sql.ast.dsl.AstDSL.alias; import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; @@ -16,6 +15,8 @@ import static org.opensearch.sql.ast.dsl.AstDSL.relation; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Let; @@ -25,93 +26,23 @@ class TimechartTest { - @Test - void should_transform_per_second_function_to_sum_and_eval() { - withTimechart(span(1, "m"), per_second("bytes")) + @ParameterizedTest + @CsvSource({"1, m, 60.0", "30, s, 30.0", "5, m, 300.0", "2, h, 7200.0", "1, d, 86400.0"}) + void should_transform_per_second_for_different_spans( + int spanValue, String spanUnit, double expectedDivisor) { + withTimechart(span(spanValue, spanUnit), per_second("bytes")) .transformPerFunction() .shouldBe( eval( - let("per_second(bytes)", divide("per_second(bytes)", 60.0)), - timechart(span(1, "m"), alias("per_second(bytes)", sum("bytes"))))); + let("per_second(bytes)", divide("per_second(bytes)", expectedDivisor)), + timechart(span(spanValue, spanUnit), alias("per_second(bytes)", sum("bytes"))))); } @Test void should_not_transform_non_per_second_functions() { - Timechart timechart = - new Timechart( - relation("t"), - AstDSL.span(field("@timestamp"), intLiteral(1), SpanUnit.of("m")), - aggregate("count", field("bytes")), - null, - 10, - true); - - UnresolvedPlan result = timechart.transformPerSecondFunctions(); - - assertSame(timechart, result); - } - - @Test - void should_calculate_correct_divisor_for_per_second_with_seconds_span() { - withTimechart(span(30, "s"), per_second("bytes")) + withTimechart(span(1, "m"), sum("bytes")) .transformPerFunction() - .shouldBe( - eval( - let("per_second(bytes)", divide("per_second(bytes)", 30.0)), - timechart(span(30, "s"), alias("per_second(bytes)", sum("bytes"))))); - } - - @Test - void should_calculate_correct_divisor_for_per_second_with_minutes_span() { - withTimechart(span(5, "m"), per_second("bytes")) - .transformPerFunction() - .shouldBe( - eval( - let("per_second(bytes)", divide("per_second(bytes)", 300.0)), - timechart(span(5, "m"), alias("per_second(bytes)", sum("bytes"))))); - } - - @Test - void should_calculate_correct_divisor_for_per_second_with_hours_span() { - withTimechart(span(2, "h"), per_second("bytes")) - .transformPerFunction() - .shouldBe( - eval( - let("per_second(bytes)", divide("per_second(bytes)", 7200.0)), - timechart(span(2, "h"), alias("per_second(bytes)", sum("bytes"))))); - } - - @Test - void should_calculate_correct_divisor_for_per_second_with_days_span() { - withTimechart(span(1, "d"), per_second("bytes")) - .transformPerFunction() - .shouldBe( - eval( - let("per_second(bytes)", divide("per_second(bytes)", 86400.0)), - timechart(span(1, "d"), alias("per_second(bytes)", sum("bytes"))))); - } - - @Test - void should_use_default_divisor_for_unsupported_span_units() { - withTimechart( - AstDSL.span(field("@timestamp"), intLiteral(1), SpanUnit.NONE), per_second("bytes")) - .transformPerFunction() - .shouldBe( - eval( - let("per_second(bytes)", divide("per_second(bytes)", 60.0)), - timechart( - AstDSL.span(field("@timestamp"), intLiteral(1), SpanUnit.NONE), - alias("per_second(bytes)", sum("bytes"))))); - } - - @Test - void should_handle_qualified_field_names_in_transformation() { - withTimechart(span(5, "m"), per_second("server.bytes")) - .transformPerFunction() - .shouldBe( - eval( - let("per_second(server.bytes)", divide("per_second(server.bytes)", 300.0)), - timechart(span(5, "m"), alias("per_second(server.bytes)", sum("server.bytes"))))); + .shouldBe(timechart(span(1, "m"), sum("bytes"))); } // Fluent API for readable test assertions @@ -172,16 +103,4 @@ public void shouldBe(UnresolvedPlan expected) { } } } - - // TODO: Add tests for per_minute, per_hour, and per_day functions when implemented - // Example structure for future functions: - // - // @Test - // void should_transform_per_minute_function() { - // withTimechart(span(2, "h"), per_minute("bytes")) - // .transformPerFunction() - // .shouldBe(evalWith( - // let("per_minute(bytes)", divide("per_minute(bytes)", 120.0)), - // timechart(span(2, "h"), alias("per_minute(bytes)", sum("bytes"))))); - // } } From ac88a6f7196dc2f3d74c4ad1e712fd3b31df1603 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Mon, 6 Oct 2025 16:27:35 -0700 Subject: [PATCH 09/26] Avoid function name hardcoding in visitPerFunctionCall Signed-off-by: Chen Dai --- .../src/main/java/org/opensearch/sql/ast/tree/Timechart.java | 4 ++-- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 2 +- .../org/opensearch/sql/ppl/parser/AstExpressionBuilder.java | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index a8d283b17ac..37f67e91230 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -89,7 +89,7 @@ private UnresolvedPlan transformPerSecondFunctions() { String fieldName = extractFieldName(aggFunc.getField()); String originalName = "per_second(" + fieldName + ")"; - double divisor = (double) (extractIntervalSeconds() / 1); // unit = 1 for per_second + double divisor = (double) extractIntervalSeconds(); return eval( timechart(AstDSL.alias(originalName, aggregate("sum", aggFunc.getField()))), let( @@ -110,6 +110,6 @@ private long extractIntervalSeconds() { int intervalValue = ((Number) ((Literal) span.getValue()).getValue()).intValue(); String unitString = SpanUnit.getName(span.getUnit()); TimeUnitConfig config = TimeUnitRegistry.getConfig(unitString); - return config != null ? config.toSeconds(intervalValue) : 60L; + return config.toSeconds(intervalValue); } } diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index ffd7c7f8bbc..90ac0d2332f 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -638,7 +638,7 @@ percentileApproxFunction ; perFunction - : PER_SECOND LT_PRTHS functionArg RT_PRTHS + : funcName=PER_SECOND LT_PRTHS functionArg RT_PRTHS ; numericLiteral 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 5a5ab1f80ac..b3df0a54590 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 @@ -541,11 +541,12 @@ private List timestampFunctionArguments( @Override public UnresolvedExpression visitPerFunctionCall(PerFunctionCallContext ctx) { ParseTree parent = ctx.getParent(); + String functionName = ctx.perFunction().funcName.getText(); if (!(parent instanceof TimechartCommandContext)) { throw new SyntaxCheckException( - "per_second function can only be used within timechart command"); + functionName + " function can only be used within timechart command"); } - return buildAggregateFunction("per_second", Arrays.asList(ctx.perFunction().functionArg())); + return buildAggregateFunction(functionName, Arrays.asList(ctx.perFunction().functionArg())); } /** Literal and value. */ From ab05e3d80751b715a5df6c1e4d2ee33c0fbe1c6c Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 7 Oct 2025 15:34:03 -0700 Subject: [PATCH 10/26] Calculate interval seconds dynamically by timestampdiff Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 96 ++++++++- .../sql/ast/tree/TimechartTest.java | 125 +++++++++-- .../remote/CalciteTimechartPerFunctionIT.java | 199 ++++++++++++++++++ .../resources/timechart_per_second_test.json | 18 ++ .../ppl/calcite/CalcitePPLTimechartTest.java | 2 +- 5 files changed, 410 insertions(+), 30 deletions(-) create mode 100644 integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java create mode 100644 integ-test/src/test/resources/timechart_per_second_test.json diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 37f67e91230..caa09f060ca 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -77,10 +77,7 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { return nodeVisitor.visitTimechart(this, context); } - /** - * Transform per_second functions into sum + eval pattern. Simple approach for single aggregation - * function only. - */ + /** Transform per_second functions into sum + eval pattern with runtime calculation. */ private UnresolvedPlan transformPerSecondFunctions() { AggregateFunction aggFunc = (AggregateFunction) this.aggregateFunction; if (!"per_second".equals(aggFunc.getFuncName())) { @@ -89,12 +86,17 @@ private UnresolvedPlan transformPerSecondFunctions() { String fieldName = extractFieldName(aggFunc.getField()); String originalName = "per_second(" + fieldName + ")"; - double divisor = (double) extractIntervalSeconds(); + + UnresolvedExpression intervalSecondsExpr = getIntervalSecondsExpr(); + UnresolvedExpression perUnitSecondsExpr = getPerUnitSecondsExpr(aggFunc.getFuncName()); + + // PPL divide function handles type coercion automatically (long/double -> double result) + UnresolvedExpression runtimeDivisor = + function("*", function("/", AstDSL.field(originalName), intervalSecondsExpr), perUnitSecondsExpr); + return eval( timechart(AstDSL.alias(originalName, aggregate("sum", aggFunc.getField()))), - let( - AstDSL.field(originalName), - function("/", AstDSL.field(originalName), doubleLiteral(divisor)))); + let(AstDSL.field(originalName), runtimeDivisor)); } private String extractFieldName(UnresolvedExpression field) { @@ -105,6 +107,84 @@ private Timechart timechart(UnresolvedExpression newAggregateFunction) { return this.toBuilder().aggregateFunction(newAggregateFunction).build(); } + /** + * Creates a runtime expression to calculate interval seconds using timestampdiff. Formula: + * timestampdiff(SECOND, @timestamp, timestampadd(unit, value, @timestamp)) + */ + private UnresolvedExpression getIntervalSecondsExpr() { + Span span = (Span) this.binExpression; + UnresolvedExpression timestampField = AstDSL.field("@timestamp"); + UnresolvedExpression spanUnit = AstDSL.stringLiteral(getSpanUnitForTimestampAdd(span)); + UnresolvedExpression spanValue = span.getValue(); + + // timestampadd(unit, value, @timestamp) - calculates bin end time + UnresolvedExpression binEndTime = function("timestampadd", spanUnit, spanValue, timestampField); + + // timestampdiff(SECOND, @timestamp, bin_end_time) - calculates actual seconds in span + return function("timestampdiff", AstDSL.stringLiteral("SECOND"), timestampField, binEndTime); + } + + /** + * Returns the per-unit seconds as an expression for the given function. per_second: 1.0, + * per_minute: 60.0, per_hour: 3600.0 (using double for floating-point arithmetic) + */ + private UnresolvedExpression getPerUnitSecondsExpr(String funcName) { + switch (funcName) { + case "per_second": + return doubleLiteral(1.0); + case "per_minute": + return doubleLiteral(60.0); + case "per_hour": + return doubleLiteral(3600.0); + default: + return doubleLiteral(1.0); // default to per_second + } + } + + /** Converts SpanUnit to the format expected by timestampadd function. */ + private String getSpanUnitForTimestampAdd(Span span) { + String unitString = SpanUnit.getName(span.getUnit()); + // Map common units to timestampadd format + switch (unitString.toLowerCase()) { + case "s": + case "sec": + case "second": + case "seconds": + return "SECOND"; + case "m": + case "min": + case "minute": + case "minutes": + return "MINUTE"; + case "h": + case "hour": + case "hours": + return "HOUR"; + case "d": + case "day": + case "days": + return "DAY"; + case "w": + case "week": + case "weeks": + return "WEEK"; + case "mon": + case "month": + case "months": + return "MONTH"; + case "q": + case "quarter": + case "quarters": + return "QUARTER"; + case "y": + case "year": + case "years": + return "YEAR"; + default: + return "MINUTE"; // default fallback + } + } + private long extractIntervalSeconds() { Span span = (Span) this.binExpression; int intervalValue = ((Number) ((Literal) span.getValue()).getValue()).intValue(); diff --git a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java index 7d1290d62d1..3a26f7f6697 100644 --- a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java @@ -27,22 +27,65 @@ class TimechartTest { @ParameterizedTest - @CsvSource({"1, m, 60.0", "30, s, 30.0", "5, m, 300.0", "2, h, 7200.0", "1, d, 86400.0"}) + @CsvSource({"1, m, MINUTE", "30, s, SECOND", "5, m, MINUTE", "2, h, HOUR", "1, d, DAY"}) void should_transform_per_second_for_different_spans( - int spanValue, String spanUnit, double expectedDivisor) { - withTimechart(span(spanValue, spanUnit), per_second("bytes")) - .transformPerFunction() - .shouldBe( + int spanValue, String spanUnit, String expectedTimestampAddUnit) { + withTimechart(span(spanValue, spanUnit), perSecond("bytes")) + .whenTransformingPerFunction() + .thenExpect( eval( - let("per_second(bytes)", divide("per_second(bytes)", expectedDivisor)), + let( + "per_second(bytes)", + multiply( + divide( + "per_second(bytes)", + timestampdiff( + "SECOND", + "@timestamp", + timestampadd( + expectedTimestampAddUnit, spanValue, "@timestamp"))), + doubleLiteral(1.0))), timechart(span(spanValue, spanUnit), alias("per_second(bytes)", sum("bytes"))))); } @Test - void should_not_transform_non_per_second_functions() { + void should_not_transform_non_per_functions() { withTimechart(span(1, "m"), sum("bytes")) - .transformPerFunction() - .shouldBe(timechart(span(1, "m"), sum("bytes"))); + .whenTransformingPerFunction() + .thenExpect(timechart(span(1, "m"), sum("bytes"))); + } + + @Test + void should_preserve_all_fields_during_per_function_transformation() { + Timechart original = + new Timechart(relation("logs"), perSecond("bytes")) + .span(span(5, "m")) + .by(field("status")) + .limit(20) + .useOther(false); + + Timechart expected = + new Timechart(relation("logs"), alias("per_second(bytes)", sum("bytes"))) + .span(span(5, "m")) + .by(field("status")) + .limit(20) + .useOther(false); + + withTimechart(original) + .whenTransformingPerFunction() + .thenExpect( + eval( + let( + "per_second(bytes)", + multiply( + divide( + "per_second(bytes)", + timestampdiff( + "SECOND", + "@timestamp", + timestampadd("MINUTE", 5, "@timestamp"))), + doubleLiteral(1.0))), + expected)); } // Fluent API for readable test assertions @@ -51,7 +94,12 @@ private static TransformationAssertion withTimechart(Span spanExpr, AggregateFun return new TransformationAssertion(timechart(spanExpr, aggFunc)); } + private static TransformationAssertion withTimechart(Timechart timechart) { + return new TransformationAssertion(timechart); + } + private static Timechart timechart(Span spanExpr, UnresolvedExpression aggExpr) { + // Set child here because expected object won't call attach below return new Timechart(relation("t"), aggExpr).span(spanExpr).limit(10).useOther(true); } @@ -59,7 +107,7 @@ private static Span span(int value, String unit) { return AstDSL.span(field("@timestamp"), intLiteral(value), SpanUnit.of(unit)); } - private static AggregateFunction per_second(String fieldName) { + private static AggregateFunction perSecond(String fieldName) { return (AggregateFunction) aggregate("per_second", field(fieldName)); } @@ -75,6 +123,48 @@ private static UnresolvedExpression divide(String fieldName, double divisor) { return function("/", field(fieldName), doubleLiteral(divisor)); } + private static UnresolvedExpression multiply( + UnresolvedExpression left, UnresolvedExpression right) { + return function("*", left, right); + } + + private static UnresolvedExpression multiply(UnresolvedExpression left, int right) { + return function("*", left, intLiteral(right)); + } + + private static UnresolvedExpression divide( + UnresolvedExpression left, UnresolvedExpression right) { + return function("/", left, right); + } + + private static UnresolvedExpression divide(String fieldName, UnresolvedExpression divisor) { + return function("/", field(fieldName), divisor); + } + + private static UnresolvedExpression timestampadd(String unit, int value, String timestampField) { + return function( + "timestampadd", AstDSL.stringLiteral(unit), intLiteral(value), field(timestampField)); + } + + private static UnresolvedExpression timestampdiff( + String unit, String startField, String endField) { + return function( + "timestampdiff", AstDSL.stringLiteral(unit), field(startField), field(endField)); + } + + private static UnresolvedExpression timestampdiff( + String unit, String startField, UnresolvedExpression end) { + return function("timestampdiff", AstDSL.stringLiteral(unit), field(startField), end); + } + + private static UnresolvedExpression cast(String fieldName, String type) { + return function("cast", field(fieldName), AstDSL.stringLiteral(type)); + } + + private static UnresolvedExpression cast(UnresolvedExpression expr, String type) { + return function("cast", expr, AstDSL.stringLiteral(type)); + } + private static UnresolvedPlan eval(Let letExpr, Timechart timechartExpr) { return AstDSL.eval(timechartExpr, letExpr); } @@ -87,20 +177,13 @@ private static class TransformationAssertion { this.timechart = timechart; } - public TransformationAssertion transformPerFunction() { - this.result = timechart.attach(null); + public TransformationAssertion whenTransformingPerFunction() { + this.result = timechart.attach(timechart.getChild().get(0)); return this; } - public void shouldBe(UnresolvedPlan expected) { - // Verify transformation produces expected type and structure - assertEquals(expected.getClass(), result.getClass()); - if (expected instanceof Eval && result instanceof Eval) { - Eval expectedEval = (Eval) expected; - Eval actualEval = (Eval) result; - assertEquals( - expectedEval.getExpressionList().size(), actualEval.getExpressionList().size()); - } + public void thenExpect(UnresolvedPlan expected) { + assertEquals(expected, result); } } } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java new file mode 100644 index 00000000000..b94cd1e859f --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -0,0 +1,199 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.calcite.remote; + +import static org.junit.jupiter.api.Assertions.*; +import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; +import static org.opensearch.sql.legacy.TestUtils.isIndexExist; +import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient; +import static org.opensearch.sql.util.MatcherUtils.schema; +import static org.opensearch.sql.util.MatcherUtils.verifySchema; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.ppl.PPLIntegTestCase; + +public class CalciteTimechartPerFunctionIT extends PPLIntegTestCase { + + @Override + public void init() throws Exception { + super.init(); + enableCalcite(); + disallowCalciteFallback(); + + // Load shared events index for basic per_second tests + loadIndex(Index.EVENTS); + + // Create dedicated per_second test data with clean math + createPerSecondTestIndex(); + } + + private void createPerSecondTestIndex() throws IOException { + String mapping = + "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"packets\":{\"type\":\"integer\"},\"host\":{\"type\":\"keyword\"}}}}"; + + if (!isIndexExist(client(), "timechart_per_second_test")) { + createIndexByRestClient(client(), "timechart_per_second_test", mapping); + loadDataByRestClient( + client(), + "timechart_per_second_test", + "src/test/resources/timechart_per_second_test.json"); + } + } + + @Test + public void testTimechartPerSecondWithDefaultSpan() throws IOException { + JSONObject result = executeQuery("source=events | timechart per_second(cpu_usage)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + + // Default span should be 1m, so per_second uses runtime calculation + assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); + assertEquals(1, result.getInt("total")); + } + + @Test + public void testTimechartPerSecondWithOneMinuteSpan() throws IOException { + JSONObject result = executeQuery("source=events | timechart span=1m per_second(cpu_usage)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + + // With 1m span: uses timestampdiff(SECOND, @timestamp, timestampadd(MINUTE, 1, @timestamp)) * 1 + assertEquals(5, result.getInt("total")); + } + + @Test + public void testTimechartPerSecondWithTwoHourSpan() throws IOException { + JSONObject result = executeQuery("source=events | timechart span=2h per_second(cpu_usage)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + + // With 2h span: uses runtime calculation with timestampadd(HOUR, 2, @timestamp) + assertEquals(1, result.getInt("total")); + } + + @Test + public void testTimechartPerSecondWithByLimitUseother() throws IOException { + JSONObject result = + executeQuery( + "source=events | timechart span=1m limit=1 useother=true per_second(cpu_usage) by" + + " host"); + + verifySchema( + result, + schema("@timestamp", "timestamp"), + schema("host", "string"), + schema("per_second(cpu_usage)", "double")); + + // Should show top 1 host + OTHER category across time spans + boolean foundOther = false; + for (int i = 0; i < result.getJSONArray("datarows").length(); i++) { + Object[] row = result.getJSONArray("datarows").getJSONArray(i).toList().toArray(); + if ("OTHER".equals(row[1])) { + foundOther = true; + break; + } + } + assertTrue("OTHER category should be present with limit=1", foundOther); + } + + @Test + public void testTimechartPerSecondWithLeapYearFebruary() throws IOException { + // Test February 2024 (leap year - 29 days = 2,505,600 seconds) + // sum(packets) = 360, so per_second = 360 / 2,505,600 * 1 ≈ 0.000144 + JSONObject result = + executeQuery( + "source=timechart_per_second_test | where year(@timestamp)=2024 and month(@timestamp)=2" + + " | timechart span=1mon per_second(packets)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); + + assertEquals(1, result.getInt("total")); + Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); + double perSecondValue = ((Number) row[1]).doubleValue(); + + // Verify approximately 360 / 2,505,600 = 0.000144 + assertEquals(0.000144, perSecondValue, 0.000001); + } + + @Test + public void testTimechartPerSecondWithRegularFebruary() throws IOException { + // Test February 2023 (regular year - 28 days = 2,419,200 seconds) + // sum(packets) = 360, so per_second = 360 / 2,419,200 * 1 ≈ 0.000149 + JSONObject result = + executeQuery( + "source=timechart_per_second_test | where year(@timestamp)=2023 and month(@timestamp)=2" + + " | timechart span=1mon per_second(packets)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); + + assertEquals(1, result.getInt("total")); + Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); + double perSecondValue = ((Number) row[1]).doubleValue(); + + // Verify approximately 360 / 2,419,200 = 0.000149 (higher rate than leap year) + assertEquals(0.000149, perSecondValue, 0.000001); + } + + @Test + public void testTimechartPerSecondWithOctober() throws IOException { + // Test October 2023 (31 days = 2,678,400 seconds) + // sum(packets) = 360, so per_second = 360 / 2,678,400 * 1 ≈ 0.000134 + JSONObject result = + executeQuery( + "source=timechart_per_second_test | where year(@timestamp)=2023 and" + + " month(@timestamp)=10 | timechart span=1mon per_second(packets)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); + + assertEquals(1, result.getInt("total")); + Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); + double perSecondValue = ((Number) row[1]).doubleValue(); + + // Verify approximately 360 / 2,678,400 = 0.000134 (lowest rate - longest month) + assertEquals(0.000134, perSecondValue, 0.000001); + } + + @Test + public void testTimechartPerSecondWithByClauseAndLimit() throws IOException { + // Test with 2025 data, by clause, limit=1, useother=true + JSONObject result = + executeQuery( + "source=timechart_per_second_test | where year(@timestamp)=2025 | timechart span=1h" + + " limit=1 useother=true per_second(packets) by host"); + + verifySchema( + result, + schema("@timestamp", "timestamp"), + schema("host", "string"), + schema("per_second(packets)", "double")); + + // Should have results for each hour with top 1 host + OTHER + boolean foundOther = false; + boolean foundServer1 = false; + + for (int i = 0; i < result.getJSONArray("datarows").length(); i++) { + Object[] row = result.getJSONArray("datarows").getJSONArray(i).toList().toArray(); + String host = (String) row[1]; + + if ("OTHER".equals(host)) { + foundOther = true; + } else if ("server1".equals(host)) { + foundServer1 = true; + } + } + + assertTrue("Should have server1 data", foundServer1); + assertTrue("Should have OTHER category with limit=1", foundOther); + } +} diff --git a/integ-test/src/test/resources/timechart_per_second_test.json b/integ-test/src/test/resources/timechart_per_second_test.json new file mode 100644 index 00000000000..c1013384526 --- /dev/null +++ b/integ-test/src/test/resources/timechart_per_second_test.json @@ -0,0 +1,18 @@ +{"index":{"_id":"1"}} +{"@timestamp":"2024-02-01T00:00:00","packets":120,"host":"server1"} +{"index":{"_id":"2"}} +{"@timestamp":"2024-02-15T12:00:00","packets":240,"host":"server2"} +{"index":{"_id":"3"}} +{"@timestamp":"2023-02-01T00:00:00","packets":120,"host":"server1"} +{"index":{"_id":"4"}} +{"@timestamp":"2023-02-15T12:00:00","packets":240,"host":"server2"} +{"index":{"_id":"5"}} +{"@timestamp":"2023-10-01T00:00:00","packets":120,"host":"server1"} +{"index":{"_id":"6"}} +{"@timestamp":"2023-10-15T12:00:00","packets":240,"host":"server2"} +{"index":{"_id":"7"}} +{"@timestamp":"2025-01-01T00:00:00","packets":120,"host":"server1"} +{"index":{"_id":"8"}} +{"@timestamp":"2025-01-01T01:00:00","packets":240,"host":"server2"} +{"index":{"_id":"9"}} +{"@timestamp":"2025-01-01T02:00:00","packets":360,"host":"server1"} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java index 5f0b86be6a8..f10403265c7 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java @@ -85,7 +85,7 @@ public void testTimechartBasic() { public void testTimechartPerSecond() { withPPLQuery("source=events | timechart per_second(cpu_usage)") .expectSparkSQL( - "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)`, 6.00E1)" + "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)`, TIMESTAMPDIFF('SECOND', `@timestamp`, TIMESTAMPADD('MINUTE', 1, `@timestamp`))) * 1.0E0" + " `per_second(cpu_usage)`\n" + "FROM (SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, SUM(`cpu_usage`)" + " `per_second(cpu_usage)`\n" From cf3282988b02a0716792e7963706613a186a9bbc Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 7 Oct 2025 15:45:56 -0700 Subject: [PATCH 11/26] Fix broken IT Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 5 +++- .../sql/ast/tree/TimechartTest.java | 7 ++--- .../remote/CalciteTimechartPerFunctionIT.java | 26 +++++++++---------- .../ppl/calcite/CalcitePPLTimechartTest.java | 3 ++- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index caa09f060ca..e75d3b8cfad 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -92,7 +92,10 @@ private UnresolvedPlan transformPerSecondFunctions() { // PPL divide function handles type coercion automatically (long/double -> double result) UnresolvedExpression runtimeDivisor = - function("*", function("/", AstDSL.field(originalName), intervalSecondsExpr), perUnitSecondsExpr); + function( + "*", + function("/", AstDSL.field(originalName), intervalSecondsExpr), + perUnitSecondsExpr); return eval( timechart(AstDSL.alias(originalName, aggregate("sum", aggFunc.getField()))), diff --git a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java index 3a26f7f6697..715671567ee 100644 --- a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java @@ -42,8 +42,7 @@ void should_transform_per_second_for_different_spans( timestampdiff( "SECOND", "@timestamp", - timestampadd( - expectedTimestampAddUnit, spanValue, "@timestamp"))), + timestampadd(expectedTimestampAddUnit, spanValue, "@timestamp"))), doubleLiteral(1.0))), timechart(span(spanValue, spanUnit), alias("per_second(bytes)", sum("bytes"))))); } @@ -81,9 +80,7 @@ void should_preserve_all_fields_during_per_function_transformation() { divide( "per_second(bytes)", timestampdiff( - "SECOND", - "@timestamp", - timestampadd("MINUTE", 5, "@timestamp"))), + "SECOND", "@timestamp", timestampadd("MINUTE", 5, "@timestamp"))), doubleLiteral(1.0))), expected)); } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java index b94cd1e859f..690c3485260 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -54,7 +54,7 @@ public void testTimechartPerSecondWithDefaultSpan() throws IOException { // Default span should be 1m, so per_second uses runtime calculation assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); - assertEquals(1, result.getInt("total")); + assertEquals(5, result.getInt("total")); } @Test @@ -106,8 +106,8 @@ public void testTimechartPerSecondWithByLimitUseother() throws IOException { @Test public void testTimechartPerSecondWithLeapYearFebruary() throws IOException { - // Test February 2024 (leap year - 29 days = 2,505,600 seconds) - // sum(packets) = 360, so per_second = 360 / 2,505,600 * 1 ≈ 0.000144 + // Test February 2024 with runtime TIMESTAMPDIFF calculation + // sum(packets) = 360, uses dynamic timestampdiff for actual interval calculation JSONObject result = executeQuery( "source=timechart_per_second_test | where year(@timestamp)=2024 and month(@timestamp)=2" @@ -120,14 +120,14 @@ public void testTimechartPerSecondWithLeapYearFebruary() throws IOException { Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); double perSecondValue = ((Number) row[1]).doubleValue(); - // Verify approximately 360 / 2,505,600 = 0.000144 - assertEquals(0.000144, perSecondValue, 0.000001); + // With dynamic TIMESTAMPDIFF calculation, per_second uses actual runtime calculation + assertTrue("per_second value should be positive", perSecondValue > 0); } @Test public void testTimechartPerSecondWithRegularFebruary() throws IOException { - // Test February 2023 (regular year - 28 days = 2,419,200 seconds) - // sum(packets) = 360, so per_second = 360 / 2,419,200 * 1 ≈ 0.000149 + // Test February 2023 with runtime TIMESTAMPDIFF calculation + // sum(packets) = 360, uses dynamic timestampdiff for actual interval calculation JSONObject result = executeQuery( "source=timechart_per_second_test | where year(@timestamp)=2023 and month(@timestamp)=2" @@ -140,14 +140,14 @@ public void testTimechartPerSecondWithRegularFebruary() throws IOException { Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); double perSecondValue = ((Number) row[1]).doubleValue(); - // Verify approximately 360 / 2,419,200 = 0.000149 (higher rate than leap year) - assertEquals(0.000149, perSecondValue, 0.000001); + // With dynamic TIMESTAMPDIFF calculation, per_second uses actual runtime calculation + assertTrue("per_second value should be positive", perSecondValue > 0); } @Test public void testTimechartPerSecondWithOctober() throws IOException { - // Test October 2023 (31 days = 2,678,400 seconds) - // sum(packets) = 360, so per_second = 360 / 2,678,400 * 1 ≈ 0.000134 + // Test October 2023 with runtime TIMESTAMPDIFF calculation + // sum(packets) = 360, uses dynamic timestampdiff for actual interval calculation JSONObject result = executeQuery( "source=timechart_per_second_test | where year(@timestamp)=2023 and" @@ -160,8 +160,8 @@ public void testTimechartPerSecondWithOctober() throws IOException { Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); double perSecondValue = ((Number) row[1]).doubleValue(); - // Verify approximately 360 / 2,678,400 = 0.000134 (lowest rate - longest month) - assertEquals(0.000134, perSecondValue, 0.000001); + // With dynamic TIMESTAMPDIFF calculation, per_second uses actual runtime calculation + assertTrue("per_second value should be positive", perSecondValue > 0); } @Test diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java index f10403265c7..64035f6c411 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java @@ -85,7 +85,8 @@ public void testTimechartBasic() { public void testTimechartPerSecond() { withPPLQuery("source=events | timechart per_second(cpu_usage)") .expectSparkSQL( - "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)`, TIMESTAMPDIFF('SECOND', `@timestamp`, TIMESTAMPADD('MINUTE', 1, `@timestamp`))) * 1.0E0" + "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)`, TIMESTAMPDIFF('SECOND'," + + " `@timestamp`, TIMESTAMPADD('MINUTE', 1, `@timestamp`))) * 1.0E0" + " `per_second(cpu_usage)`\n" + "FROM (SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, SUM(`cpu_usage`)" + " `per_second(cpu_usage)`\n" From 4e093261f03b8cf4275430537a671a9d92875652 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 7 Oct 2025 19:25:49 -0700 Subject: [PATCH 12/26] Fix broken UT and integer division issue Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 64 +----- .../sql/ast/tree/TimechartTest.java | 55 ++--- .../remote/CalciteTimechartPerFunctionIT.java | 203 ++++++++---------- .../resources/timechart_per_second_test.json | 18 -- .../ppl/calcite/CalcitePPLTimechartTest.java | 4 +- 5 files changed, 114 insertions(+), 230 deletions(-) delete mode 100644 integ-test/src/test/resources/timechart_per_second_test.json diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index e75d3b8cfad..7f5f459d4c5 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -10,6 +10,7 @@ import static org.opensearch.sql.ast.dsl.AstDSL.eval; import static org.opensearch.sql.ast.dsl.AstDSL.function; import static org.opensearch.sql.ast.dsl.AstDSL.let; +import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import com.google.common.collect.ImmutableList; import java.util.List; @@ -21,12 +22,9 @@ import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Field; -import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.expression.UnresolvedExpression; -import org.opensearch.sql.calcite.utils.binning.time.TimeUnitConfig; -import org.opensearch.sql.calcite.utils.binning.time.TimeUnitRegistry; /** AST node represent Timechart operation. */ @Getter @@ -87,16 +85,16 @@ private UnresolvedPlan transformPerSecondFunctions() { String fieldName = extractFieldName(aggFunc.getField()); String originalName = "per_second(" + fieldName + ")"; - UnresolvedExpression intervalSecondsExpr = getIntervalSecondsExpr(); - UnresolvedExpression perUnitSecondsExpr = getPerUnitSecondsExpr(aggFunc.getFuncName()); - - // PPL divide function handles type coercion automatically (long/double -> double result) + Span span = (Span) this.binExpression; + UnresolvedExpression spanUnit = stringLiteral(getSpanUnitForTimestampAdd(span)); + UnresolvedExpression spanValue = span.getValue(); + UnresolvedExpression binStartTime = AstDSL.field("@timestamp"); + UnresolvedExpression binEndTime = function("timestampadd", spanUnit, spanValue, binStartTime); + UnresolvedExpression intervalSeconds = + function("timestampdiff", stringLiteral("SECOND"), binStartTime, binEndTime); + UnresolvedExpression perUnitSeconds = doubleLiteral(1.0); UnresolvedExpression runtimeDivisor = - function( - "*", - function("/", AstDSL.field(originalName), intervalSecondsExpr), - perUnitSecondsExpr); - + function("/", function("*", AstDSL.field(originalName), perUnitSeconds), intervalSeconds); return eval( timechart(AstDSL.alias(originalName, aggregate("sum", aggFunc.getField()))), let(AstDSL.field(originalName), runtimeDivisor)); @@ -110,40 +108,6 @@ private Timechart timechart(UnresolvedExpression newAggregateFunction) { return this.toBuilder().aggregateFunction(newAggregateFunction).build(); } - /** - * Creates a runtime expression to calculate interval seconds using timestampdiff. Formula: - * timestampdiff(SECOND, @timestamp, timestampadd(unit, value, @timestamp)) - */ - private UnresolvedExpression getIntervalSecondsExpr() { - Span span = (Span) this.binExpression; - UnresolvedExpression timestampField = AstDSL.field("@timestamp"); - UnresolvedExpression spanUnit = AstDSL.stringLiteral(getSpanUnitForTimestampAdd(span)); - UnresolvedExpression spanValue = span.getValue(); - - // timestampadd(unit, value, @timestamp) - calculates bin end time - UnresolvedExpression binEndTime = function("timestampadd", spanUnit, spanValue, timestampField); - - // timestampdiff(SECOND, @timestamp, bin_end_time) - calculates actual seconds in span - return function("timestampdiff", AstDSL.stringLiteral("SECOND"), timestampField, binEndTime); - } - - /** - * Returns the per-unit seconds as an expression for the given function. per_second: 1.0, - * per_minute: 60.0, per_hour: 3600.0 (using double for floating-point arithmetic) - */ - private UnresolvedExpression getPerUnitSecondsExpr(String funcName) { - switch (funcName) { - case "per_second": - return doubleLiteral(1.0); - case "per_minute": - return doubleLiteral(60.0); - case "per_hour": - return doubleLiteral(3600.0); - default: - return doubleLiteral(1.0); // default to per_second - } - } - /** Converts SpanUnit to the format expected by timestampadd function. */ private String getSpanUnitForTimestampAdd(Span span) { String unitString = SpanUnit.getName(span.getUnit()); @@ -187,12 +151,4 @@ private String getSpanUnitForTimestampAdd(Span span) { return "MINUTE"; // default fallback } } - - private long extractIntervalSeconds() { - Span span = (Span) this.binExpression; - int intervalValue = ((Number) ((Literal) span.getValue()).getValue()).intValue(); - String unitString = SpanUnit.getName(span.getUnit()); - TimeUnitConfig config = TimeUnitRegistry.getConfig(unitString); - return config.toSeconds(intervalValue); - } } diff --git a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java index 715671567ee..c6e2fe06f9a 100644 --- a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java @@ -36,14 +36,12 @@ void should_transform_per_second_for_different_spans( eval( let( "per_second(bytes)", - multiply( - divide( - "per_second(bytes)", - timestampdiff( - "SECOND", - "@timestamp", - timestampadd(expectedTimestampAddUnit, spanValue, "@timestamp"))), - doubleLiteral(1.0))), + divide( + multiply("per_second(bytes)", 1.0), + timestampdiff( + "SECOND", + "@timestamp", + timestampadd(expectedTimestampAddUnit, spanValue, "@timestamp")))), timechart(span(spanValue, spanUnit), alias("per_second(bytes)", sum("bytes"))))); } @@ -76,12 +74,10 @@ void should_preserve_all_fields_during_per_function_transformation() { eval( let( "per_second(bytes)", - multiply( - divide( - "per_second(bytes)", - timestampdiff( - "SECOND", "@timestamp", timestampadd("MINUTE", 5, "@timestamp"))), - doubleLiteral(1.0))), + divide( + multiply("per_second(bytes)", 1.0), + timestampdiff( + "SECOND", "@timestamp", timestampadd("MINUTE", 5, "@timestamp")))), expected)); } @@ -116,17 +112,8 @@ private static Let let(String fieldName, UnresolvedExpression expression) { return AstDSL.let(field(fieldName), expression); } - private static UnresolvedExpression divide(String fieldName, double divisor) { - return function("/", field(fieldName), doubleLiteral(divisor)); - } - - private static UnresolvedExpression multiply( - UnresolvedExpression left, UnresolvedExpression right) { - return function("*", left, right); - } - - private static UnresolvedExpression multiply(UnresolvedExpression left, int right) { - return function("*", left, intLiteral(right)); + private static UnresolvedExpression multiply(String fieldName, double right) { + return function("*", field(fieldName), doubleLiteral(right)); } private static UnresolvedExpression divide( @@ -134,34 +121,16 @@ private static UnresolvedExpression divide( return function("/", left, right); } - private static UnresolvedExpression divide(String fieldName, UnresolvedExpression divisor) { - return function("/", field(fieldName), divisor); - } - private static UnresolvedExpression timestampadd(String unit, int value, String timestampField) { return function( "timestampadd", AstDSL.stringLiteral(unit), intLiteral(value), field(timestampField)); } - private static UnresolvedExpression timestampdiff( - String unit, String startField, String endField) { - return function( - "timestampdiff", AstDSL.stringLiteral(unit), field(startField), field(endField)); - } - private static UnresolvedExpression timestampdiff( String unit, String startField, UnresolvedExpression end) { return function("timestampdiff", AstDSL.stringLiteral(unit), field(startField), end); } - private static UnresolvedExpression cast(String fieldName, String type) { - return function("cast", field(fieldName), AstDSL.stringLiteral(type)); - } - - private static UnresolvedExpression cast(UnresolvedExpression expr, String type) { - return function("cast", expr, AstDSL.stringLiteral(type)); - } - private static UnresolvedPlan eval(Let letExpr, Timechart timechartExpr) { return AstDSL.eval(timechartExpr, letExpr); } diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java index 690c3485260..eaf2476773a 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -5,16 +5,17 @@ package org.opensearch.sql.calcite.remote; -import static org.junit.jupiter.api.Assertions.*; import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; import static org.opensearch.sql.legacy.TestUtils.isIndexExist; -import static org.opensearch.sql.legacy.TestUtils.loadDataByRestClient; +import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; import static org.opensearch.sql.util.MatcherUtils.verifySchema; import java.io.IOException; import org.json.JSONObject; import org.junit.jupiter.api.Test; +import org.opensearch.client.Request; import org.opensearch.sql.ppl.PPLIntegTestCase; public class CalciteTimechartPerFunctionIT extends PPLIntegTestCase { @@ -25,152 +26,137 @@ public void init() throws Exception { enableCalcite(); disallowCalciteFallback(); - // Load shared events index for basic per_second tests - loadIndex(Index.EVENTS); - - // Create dedicated per_second test data with clean math - createPerSecondTestIndex(); + // Create dedicated per_function test data directly in init() + createPerFunctionTestIndex(); } - private void createPerSecondTestIndex() throws IOException { + private void createPerFunctionTestIndex() throws IOException { String mapping = "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"packets\":{\"type\":\"integer\"},\"host\":{\"type\":\"keyword\"}}}}"; - if (!isIndexExist(client(), "timechart_per_second_test")) { - createIndexByRestClient(client(), "timechart_per_second_test", mapping); - loadDataByRestClient( - client(), - "timechart_per_second_test", - "src/test/resources/timechart_per_second_test.json"); - } - } - - @Test - public void testTimechartPerSecondWithDefaultSpan() throws IOException { - JSONObject result = executeQuery("source=events | timechart per_second(cpu_usage)"); + if (!isIndexExist(client(), "timechart_per_function_test")) { + createIndexByRestClient(client(), "timechart_per_function_test", mapping); - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); - - // Default span should be 1m, so per_second uses runtime calculation - assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); - assertEquals(5, result.getInt("total")); + // Insert test data directly with clean numbers that avoid decimal issues + insertTestData(); + } } - @Test - public void testTimechartPerSecondWithOneMinuteSpan() throws IOException { - JSONObject result = executeQuery("source=events | timechart span=1m per_second(cpu_usage)"); - - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + private void insertTestData() throws IOException { + // Create data with clean numbers that result in whole numbers or simple decimals + // Using multiples of 60 to get clean per_second calculations with 1m spans + String[] testData = { + "{\"@timestamp\":\"2025-09-08T10:00:00\",\"packets\":60,\"host\":\"server1\"}", // 60/60 = 1.0 + "{\"@timestamp\":\"2025-09-08T10:01:00\",\"packets\":120,\"host\":\"server1\"}", // 120/60 = + // 2.0 + "{\"@timestamp\":\"2025-09-08T10:02:00\",\"packets\":180,\"host\":\"server1\"}", // 180/60 = + // 3.0 + "{\"@timestamp\":\"2025-09-08T10:02:30\",\"packets\":30,\"host\":\"server2\"}" // For the by + // host tests + }; + + for (String data : testData) { + Request request = new Request("POST", "/timechart_per_function_test/_doc"); + request.setJsonEntity(data); + client().performRequest(request); + } - // With 1m span: uses timestampdiff(SECOND, @timestamp, timestampadd(MINUTE, 1, @timestamp)) * 1 - assertEquals(5, result.getInt("total")); + // Refresh index to make data searchable + Request refresh = new Request("POST", "/timechart_per_function_test/_refresh"); + client().performRequest(refresh); } @Test - public void testTimechartPerSecondWithTwoHourSpan() throws IOException { - JSONObject result = executeQuery("source=events | timechart span=2h per_second(cpu_usage)"); + public void testTimechartPerSecondWithDefaultSpan() throws IOException { + JSONObject result = + executeQuery("source=timechart_per_function_test | timechart span=1m per_second(packets)"); verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); + result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - // With 2h span: uses runtime calculation with timestampadd(HOUR, 2, @timestamp) - assertEquals(1, result.getInt("total")); + // With 1m span: 60/60=1.0, 120/60=2.0, (180+30)/60=3.5 + // The 10:02:30 data falls into the 10:02:00 bucket + verifyDataRows( + result, + rows("2025-09-08 10:00:00", 1.0), + rows("2025-09-08 10:01:00", 2.0), + rows("2025-09-08 10:02:00", 3.5)); } @Test - public void testTimechartPerSecondWithByLimitUseother() throws IOException { + public void testTimechartPerSecondWithTwentySecondSpan() throws IOException { JSONObject result = - executeQuery( - "source=events | timechart span=1m limit=1 useother=true per_second(cpu_usage) by" - + " host"); + executeQuery("source=timechart_per_function_test | timechart span=20s per_second(packets)"); verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); + + // With 20s span: 60/20=3.0, 120/20=6.0, 180/20=9.0, 30/20=1.5 (correct floating point) + verifyDataRows( result, - schema("@timestamp", "timestamp"), - schema("host", "string"), - schema("per_second(cpu_usage)", "double")); - - // Should show top 1 host + OTHER category across time spans - boolean foundOther = false; - for (int i = 0; i < result.getJSONArray("datarows").length(); i++) { - Object[] row = result.getJSONArray("datarows").getJSONArray(i).toList().toArray(); - if ("OTHER".equals(row[1])) { - foundOther = true; - break; - } - } - assertTrue("OTHER category should be present with limit=1", foundOther); + rows("2025-09-08 10:00:00", 3.0), + rows("2025-09-08 10:01:00", 6.0), + rows("2025-09-08 10:02:00", 9.0), + rows("2025-09-08 10:02:20", 1.5)); } @Test - public void testTimechartPerSecondWithLeapYearFebruary() throws IOException { - // Test February 2024 with runtime TIMESTAMPDIFF calculation - // sum(packets) = 360, uses dynamic timestampdiff for actual interval calculation + public void testTimechartPerSecondWithOneHourSpan() throws IOException { JSONObject result = - executeQuery( - "source=timechart_per_second_test | where year(@timestamp)=2024 and month(@timestamp)=2" - + " | timechart span=1mon per_second(packets)"); + executeQuery("source=timechart_per_function_test | timechart span=1h per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - assertEquals(1, result.getInt("total")); - Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); - double perSecondValue = ((Number) row[1]).doubleValue(); - - // With dynamic TIMESTAMPDIFF calculation, per_second uses actual runtime calculation - assertTrue("per_second value should be positive", perSecondValue > 0); + // With 1h span: (60+120+180+30)/3600 = 390/3600 = 0.10833333333333334 + verifyDataRows(result, rows("2025-09-08 10:00:00", 0.10833333333333334)); } @Test - public void testTimechartPerSecondWithRegularFebruary() throws IOException { - // Test February 2023 with runtime TIMESTAMPDIFF calculation - // sum(packets) = 360, uses dynamic timestampdiff for actual interval calculation + public void testTimechartPerSecondWithByLimitUseother() throws IOException { JSONObject result = executeQuery( - "source=timechart_per_second_test | where year(@timestamp)=2023 and month(@timestamp)=2" - + " | timechart span=1mon per_second(packets)"); + "source=timechart_per_function_test | timechart span=1m limit=1 useother=true" + + " per_second(packets) by host"); verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - - assertEquals(1, result.getInt("total")); - Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); - double perSecondValue = ((Number) row[1]).doubleValue(); + result, + schema("@timestamp", "timestamp"), + schema("host", "string"), + schema("per_second(packets)", "double")); - // With dynamic TIMESTAMPDIFF calculation, per_second uses actual runtime calculation - assertTrue("per_second value should be positive", perSecondValue > 0); + // With limit=1 and useother=true, expected 4 rows instead of 6 + // server1 appears in buckets, OTHER shows correct floating point: 30/60 = 0.5 + verifyDataRows( + result, + rows("2025-09-08 10:00:00", "server1", 1.0), + rows("2025-09-08 10:01:00", "server1", 2.0), + rows("2025-09-08 10:02:00", "server1", 3.0), + rows("2025-09-08 10:02:00", "OTHER", 0.5)); // OTHER shows correct 30/60 = 0.5 } @Test - public void testTimechartPerSecondWithOctober() throws IOException { - // Test October 2023 with runtime TIMESTAMPDIFF calculation - // sum(packets) = 360, uses dynamic timestampdiff for actual interval calculation + public void testTimechartPerSecondDataRows() throws IOException { JSONObject result = - executeQuery( - "source=timechart_per_second_test | where year(@timestamp)=2023 and" - + " month(@timestamp)=10 | timechart span=1mon per_second(packets)"); + executeQuery("source=timechart_per_function_test | timechart span=1m per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - assertEquals(1, result.getInt("total")); - Object[] row = result.getJSONArray("datarows").getJSONArray(0).toList().toArray(); - double perSecondValue = ((Number) row[1]).doubleValue(); - - // With dynamic TIMESTAMPDIFF calculation, per_second uses actual runtime calculation - assertTrue("per_second value should be positive", perSecondValue > 0); + // Verify the same data as testTimechartPerSecondWithDefaultSpan + verifyDataRows( + result, + rows("2025-09-08 10:00:00", 1.0), + rows("2025-09-08 10:01:00", 2.0), + rows("2025-09-08 10:02:00", 3.5)); } @Test public void testTimechartPerSecondWithByClauseAndLimit() throws IOException { - // Test with 2025 data, by clause, limit=1, useother=true JSONObject result = executeQuery( - "source=timechart_per_second_test | where year(@timestamp)=2025 | timechart span=1h" - + " limit=1 useother=true per_second(packets) by host"); + "source=timechart_per_function_test | timechart span=30s limit=2 per_second(packets) by" + + " host"); verifySchema( result, @@ -178,22 +164,13 @@ public void testTimechartPerSecondWithByClauseAndLimit() throws IOException { schema("host", "string"), schema("per_second(packets)", "double")); - // Should have results for each hour with top 1 host + OTHER - boolean foundOther = false; - boolean foundServer1 = false; - - for (int i = 0; i < result.getJSONArray("datarows").length(); i++) { - Object[] row = result.getJSONArray("datarows").getJSONArray(i).toList().toArray(); - String host = (String) row[1]; - - if ("OTHER".equals(host)) { - foundOther = true; - } else if ("server1".equals(host)) { - foundServer1 = true; - } - } - - assertTrue("Should have server1 data", foundServer1); - assertTrue("Should have OTHER category with limit=1", foundOther); + // With 30s span and limit=2, expected 4 rows instead of 8 + // Only buckets with actual data, no zero-filled buckets + verifyDataRows( + result, + rows("2025-09-08 10:00:00", "server1", 2.0), // 60/30 = 2.0 + rows("2025-09-08 10:01:00", "server1", 4.0), // 120/30 = 4.0 + rows("2025-09-08 10:02:00", "server1", 6.0), // 180/30 = 6.0 + rows("2025-09-08 10:02:30", "server2", 1.0)); // 30/30 = 1.0 } } diff --git a/integ-test/src/test/resources/timechart_per_second_test.json b/integ-test/src/test/resources/timechart_per_second_test.json deleted file mode 100644 index c1013384526..00000000000 --- a/integ-test/src/test/resources/timechart_per_second_test.json +++ /dev/null @@ -1,18 +0,0 @@ -{"index":{"_id":"1"}} -{"@timestamp":"2024-02-01T00:00:00","packets":120,"host":"server1"} -{"index":{"_id":"2"}} -{"@timestamp":"2024-02-15T12:00:00","packets":240,"host":"server2"} -{"index":{"_id":"3"}} -{"@timestamp":"2023-02-01T00:00:00","packets":120,"host":"server1"} -{"index":{"_id":"4"}} -{"@timestamp":"2023-02-15T12:00:00","packets":240,"host":"server2"} -{"index":{"_id":"5"}} -{"@timestamp":"2023-10-01T00:00:00","packets":120,"host":"server1"} -{"index":{"_id":"6"}} -{"@timestamp":"2023-10-15T12:00:00","packets":240,"host":"server2"} -{"index":{"_id":"7"}} -{"@timestamp":"2025-01-01T00:00:00","packets":120,"host":"server1"} -{"index":{"_id":"8"}} -{"@timestamp":"2025-01-01T01:00:00","packets":240,"host":"server2"} -{"index":{"_id":"9"}} -{"@timestamp":"2025-01-01T02:00:00","packets":360,"host":"server1"} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java index 64035f6c411..356a2b0cede 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLTimechartTest.java @@ -85,8 +85,8 @@ public void testTimechartBasic() { public void testTimechartPerSecond() { withPPLQuery("source=events | timechart per_second(cpu_usage)") .expectSparkSQL( - "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)`, TIMESTAMPDIFF('SECOND'," - + " `@timestamp`, TIMESTAMPADD('MINUTE', 1, `@timestamp`))) * 1.0E0" + "SELECT `@timestamp`, `DIVIDE`(`per_second(cpu_usage)` * 1.0E0, TIMESTAMPDIFF('SECOND'," + + " `@timestamp`, TIMESTAMPADD('MINUTE', 1, `@timestamp`)))" + " `per_second(cpu_usage)`\n" + "FROM (SELECT `SPAN`(`@timestamp`, 1, 'm') `@timestamp`, SUM(`cpu_usage`)" + " `per_second(cpu_usage)`\n" From 768ba119bca74a010e360fad65fe36ad801354bd Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 7 Oct 2025 20:00:16 -0700 Subject: [PATCH 13/26] Refactor transform method with fluent AST DSL APIs Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 7f5f459d4c5..7792b988e72 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -9,7 +9,6 @@ import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.eval; import static org.opensearch.sql.ast.dsl.AstDSL.function; -import static org.opensearch.sql.ast.dsl.AstDSL.let; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import com.google.common.collect.ImmutableList; @@ -22,6 +21,7 @@ import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Field; +import org.opensearch.sql.ast.expression.Let; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.expression.UnresolvedExpression; @@ -84,20 +84,21 @@ private UnresolvedPlan transformPerSecondFunctions() { String fieldName = extractFieldName(aggFunc.getField()); String originalName = "per_second(" + fieldName + ")"; - Span span = (Span) this.binExpression; - UnresolvedExpression spanUnit = stringLiteral(getSpanUnitForTimestampAdd(span)); - UnresolvedExpression spanValue = span.getValue(); - UnresolvedExpression binStartTime = AstDSL.field("@timestamp"); - UnresolvedExpression binEndTime = function("timestampadd", spanUnit, spanValue, binStartTime); - UnresolvedExpression intervalSeconds = - function("timestampdiff", stringLiteral("SECOND"), binStartTime, binEndTime); - UnresolvedExpression perUnitSeconds = doubleLiteral(1.0); - UnresolvedExpression runtimeDivisor = - function("/", function("*", AstDSL.field(originalName), perUnitSeconds), intervalSeconds); return eval( - timechart(AstDSL.alias(originalName, aggregate("sum", aggFunc.getField()))), - let(AstDSL.field(originalName), runtimeDivisor)); + timechart(AstDSL.alias(originalName, sum(aggFunc.getField()))), + let( + originalName, + divide( + multiply(originalName, perUnitSeconds()), + timestampdiff( + "SECOND", + "@timestamp", // bin start time + timestampadd( + getSpanUnitForTimestampAdd(span), + span.getValue(), + "@timestamp") // bin end time + )))); } private String extractFieldName(UnresolvedExpression field) { @@ -108,6 +109,10 @@ private Timechart timechart(UnresolvedExpression newAggregateFunction) { return this.toBuilder().aggregateFunction(newAggregateFunction).build(); } + private UnresolvedExpression perUnitSeconds() { + return doubleLiteral(1.0); + } + /** Converts SpanUnit to the format expected by timestampadd function. */ private String getSpanUnitForTimestampAdd(Span span) { String unitString = SpanUnit.getName(span.getUnit()); @@ -151,4 +156,32 @@ private String getSpanUnitForTimestampAdd(Span span) { return "MINUTE"; // default fallback } } + + // Private DSL methods for clean transformation code + + private UnresolvedExpression sum(UnresolvedExpression field) { + return aggregate("sum", field); + } + + private UnresolvedExpression multiply(String fieldName, UnresolvedExpression value) { + return function("*", AstDSL.field(fieldName), value); + } + + private UnresolvedExpression divide(UnresolvedExpression left, UnresolvedExpression right) { + return function("/", left, right); + } + + private UnresolvedExpression timestampadd( + String unit, UnresolvedExpression value, String timestampField) { + return function("timestampadd", stringLiteral(unit), value, AstDSL.field(timestampField)); + } + + private UnresolvedExpression timestampdiff( + String unit, String startField, UnresolvedExpression end) { + return function("timestampdiff", stringLiteral(unit), AstDSL.field(startField), end); + } + + private Let let(String fieldName, UnresolvedExpression expression) { + return AstDSL.let(AstDSL.field(fieldName), expression); + } } From 508152882b901039beec2c7488fb426427b0ab36 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 7 Oct 2025 20:22:17 -0700 Subject: [PATCH 14/26] Extract span unit to interval unit util Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 73 ++++--------------- .../sql/calcite/utils/PlanUtils.java | 16 ++++ 2 files changed, 32 insertions(+), 57 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 7792b988e72..6663196e8d6 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -13,6 +13,7 @@ import com.google.common.collect.ImmutableList; import java.util.List; +import java.util.Locale; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -25,6 +26,7 @@ import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.expression.UnresolvedExpression; +import org.opensearch.sql.calcite.utils.PlanUtils; /** AST node represent Timechart operation. */ @Getter @@ -82,27 +84,26 @@ private UnresolvedPlan transformPerSecondFunctions() { return this; } - String fieldName = extractFieldName(aggFunc.getField()); - String originalName = "per_second(" + fieldName + ")"; Span span = (Span) this.binExpression; + String aggFuncName = aggregateFunctionName(aggFunc); return eval( - timechart(AstDSL.alias(originalName, sum(aggFunc.getField()))), + timechart(AstDSL.alias(aggFuncName, sum(aggFunc.getField()))), let( - originalName, + aggFuncName, divide( - multiply(originalName, perUnitSeconds()), + multiply(aggFuncName, perUnitSeconds()), timestampdiff( "SECOND", "@timestamp", // bin start time - timestampadd( - getSpanUnitForTimestampAdd(span), - span.getValue(), - "@timestamp") // bin end time + timestampadd(span.getUnit(), span.getValue(), "@timestamp") // bin end time )))); } - private String extractFieldName(UnresolvedExpression field) { - return field instanceof Field ? ((Field) field).getField().toString() : field.toString(); + private String aggregateFunctionName(AggregateFunction aggFunc) { + UnresolvedExpression field = aggFunc.getField(); + String fieldName = + field instanceof Field ? ((Field) field).getField().toString() : field.toString(); + return String.format(Locale.ROOT, "%s(%s)", aggFunc.getFuncName(), fieldName); } private Timechart timechart(UnresolvedExpression newAggregateFunction) { @@ -113,50 +114,6 @@ private UnresolvedExpression perUnitSeconds() { return doubleLiteral(1.0); } - /** Converts SpanUnit to the format expected by timestampadd function. */ - private String getSpanUnitForTimestampAdd(Span span) { - String unitString = SpanUnit.getName(span.getUnit()); - // Map common units to timestampadd format - switch (unitString.toLowerCase()) { - case "s": - case "sec": - case "second": - case "seconds": - return "SECOND"; - case "m": - case "min": - case "minute": - case "minutes": - return "MINUTE"; - case "h": - case "hour": - case "hours": - return "HOUR"; - case "d": - case "day": - case "days": - return "DAY"; - case "w": - case "week": - case "weeks": - return "WEEK"; - case "mon": - case "month": - case "months": - return "MONTH"; - case "q": - case "quarter": - case "quarters": - return "QUARTER"; - case "y": - case "year": - case "years": - return "YEAR"; - default: - return "MINUTE"; // default fallback - } - } - // Private DSL methods for clean transformation code private UnresolvedExpression sum(UnresolvedExpression field) { @@ -172,8 +129,10 @@ private UnresolvedExpression divide(UnresolvedExpression left, UnresolvedExpress } private UnresolvedExpression timestampadd( - String unit, UnresolvedExpression value, String timestampField) { - return function("timestampadd", stringLiteral(unit), value, AstDSL.field(timestampField)); + SpanUnit unit, UnresolvedExpression value, String timestampField) { + UnresolvedExpression intervalUnit = + stringLiteral(PlanUtils.spanUnitToIntervalUnit(unit).toString()); + return function("timestampadd", intervalUnit, value, AstDSL.field(timestampField)); } private UnresolvedExpression timestampdiff( diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java b/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java index 4d3bef062fa..d49c7b7b607 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java @@ -74,6 +74,22 @@ static SpanUnit intervalUnitToSpanUnit(IntervalUnit unit) { }; } + static IntervalUnit spanUnitToIntervalUnit(SpanUnit unit) { + return switch (unit) { + case MILLISECOND, MS -> IntervalUnit.MICROSECOND; + case SECOND, SECONDS, SEC, SECS, S -> IntervalUnit.SECOND; + case MINUTE, MINUTES, MIN, MINS, m -> IntervalUnit.MINUTE; + case HOUR, HOURS, HR, HRS, H -> IntervalUnit.HOUR; + case DAY, DAYS, D -> IntervalUnit.DAY; + case WEEK, WEEKS, W -> IntervalUnit.WEEK; + case MONTH, MONTHS, MON, M -> IntervalUnit.MONTH; + case QUARTER, QUARTERS, QTR, QTRS, Q -> IntervalUnit.QUARTER; + case YEAR, YEARS, Y -> IntervalUnit.YEAR; + case UNKNOWN -> IntervalUnit.UNKNOWN; + default -> throw new UnsupportedOperationException("Unsupported span unit: " + unit); + }; + } + static RexNode makeOver( CalcitePlanContext context, BuiltinFunctionName functionName, From e1cb01c8ea444cedfb82964d943071a6044effbf Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 09:16:46 -0700 Subject: [PATCH 15/26] Refactor transform method with built-in DSL APIs Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 55 +++++++------------ 1 file changed, 19 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index 6663196e8d6..c12b8ca17c9 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -9,6 +9,7 @@ import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.eval; import static org.opensearch.sql.ast.dsl.AstDSL.function; +import static org.opensearch.sql.ast.dsl.AstDSL.let; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import com.google.common.collect.ImmutableList; @@ -22,7 +23,7 @@ import org.opensearch.sql.ast.dsl.AstDSL; import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Field; -import org.opensearch.sql.ast.expression.Let; +import org.opensearch.sql.ast.expression.Function; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.expression.UnresolvedExpression; @@ -85,21 +86,21 @@ private UnresolvedPlan transformPerSecondFunctions() { } Span span = (Span) this.binExpression; - String aggFuncName = aggregateFunctionName(aggFunc); + String aggName = aggregationName(aggFunc); + Field aggField = AstDSL.field(aggName); + Field spanStartTime = AstDSL.field("@timestamp"); + Function spanEndTime = timestampadd(span.getUnit(), span.getValue(), spanStartTime); return eval( - timechart(AstDSL.alias(aggFuncName, sum(aggFunc.getField()))), + timechart(AstDSL.alias(aggName, aggregate("sum", aggFunc.getField()))), let( - aggFuncName, - divide( - multiply(aggFuncName, perUnitSeconds()), - timestampdiff( - "SECOND", - "@timestamp", // bin start time - timestampadd(span.getUnit(), span.getValue(), "@timestamp") // bin end time - )))); + aggField, + function( + "/", + function("*", aggField, perUnitSeconds()), + timestampdiff("SECOND", spanStartTime, spanEndTime)))); } - private String aggregateFunctionName(AggregateFunction aggFunc) { + private String aggregationName(AggregateFunction aggFunc) { UnresolvedExpression field = aggFunc.getField(); String fieldName = field instanceof Field ? ((Field) field).getField().toString() : field.toString(); @@ -114,33 +115,15 @@ private UnresolvedExpression perUnitSeconds() { return doubleLiteral(1.0); } - // Private DSL methods for clean transformation code - - private UnresolvedExpression sum(UnresolvedExpression field) { - return aggregate("sum", field); - } - - private UnresolvedExpression multiply(String fieldName, UnresolvedExpression value) { - return function("*", AstDSL.field(fieldName), value); - } - - private UnresolvedExpression divide(UnresolvedExpression left, UnresolvedExpression right) { - return function("/", left, right); - } - - private UnresolvedExpression timestampadd( - SpanUnit unit, UnresolvedExpression value, String timestampField) { + private Function timestampadd( + SpanUnit unit, UnresolvedExpression value, UnresolvedExpression timestampField) { UnresolvedExpression intervalUnit = stringLiteral(PlanUtils.spanUnitToIntervalUnit(unit).toString()); - return function("timestampadd", intervalUnit, value, AstDSL.field(timestampField)); - } - - private UnresolvedExpression timestampdiff( - String unit, String startField, UnresolvedExpression end) { - return function("timestampdiff", stringLiteral(unit), AstDSL.field(startField), end); + return function("timestampadd", intervalUnit, value, timestampField); } - private Let let(String fieldName, UnresolvedExpression expression) { - return AstDSL.let(AstDSL.field(fieldName), expression); + private Function timestampdiff( + String unit, UnresolvedExpression start, UnresolvedExpression end) { + return function("timestampdiff", stringLiteral(unit), start, end); } } From 5a4400c09ef30ce9dd9dd3231a78f9a3b374f7eb Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 09:53:49 -0700 Subject: [PATCH 16/26] Fix broken AST builder UT Signed-off-by: Chen Dai --- .../opensearch/sql/ppl/parser/AstBuilderTest.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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 6e4b323b6d0..a3d6f686af6 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 @@ -1102,7 +1102,18 @@ public void testTimechartWithPerSecondFunction() { .useOther(true), let( field("per_second(a)"), - function("/", field("per_second(a)"), doubleLiteral(60.0))))); + function( + "/", + function("*", field("per_second(a)"), doubleLiteral(1.0)), + function( + "timestampdiff", + stringLiteral("SECOND"), + field("@timestamp"), + function( + "timestampadd", + stringLiteral("MINUTE"), + intLiteral(1), + field("@timestamp"))))))); } @Test From 47cfd7117c355beb5601ea1f684b096a16d6109f Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 11:02:48 -0700 Subject: [PATCH 17/26] Update javadoc and remove old ITs Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 18 ++++++-- .../remote/CalciteTimechartCommandIT.java | 42 ------------------- 2 files changed, 15 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index c12b8ca17c9..b41759ed30c 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -65,7 +65,8 @@ public Timechart useOther(Boolean useOther) { @Override public UnresolvedPlan attach(UnresolvedPlan child) { - return toBuilder().child(child).build().transformPerSecondFunctions(); + // Transform after child attached to avoid unintentionally overriding it + return toBuilder().child(child).build().transformPerSecondFunction(); } @Override @@ -78,8 +79,18 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { return nodeVisitor.visitTimechart(this, context); } - /** Transform per_second functions into sum + eval pattern with runtime calculation. */ - private UnresolvedPlan transformPerSecondFunctions() { + /** + * Transform per function to Eval-based post-processing on sum result by Timechart. Specifically, + * calculate how many seconds are in the time bucket based on the span option dynamically, then + * divide the aggregated sum value by the number of seconds to get the per-second rate. + * + *

For example, with span=5m per_second(field): per second rate = sum(field) / 300 seconds + * + *

TODO: extend to support additional per_* functions + * + * @return Eval+Timechart AST if per function present, or the original AST otherwise. + */ + private UnresolvedPlan transformPerSecondFunction() { AggregateFunction aggFunc = (AggregateFunction) this.aggregateFunction; if (!"per_second".equals(aggFunc.getFuncName())) { return this; @@ -90,6 +101,7 @@ private UnresolvedPlan transformPerSecondFunctions() { Field aggField = AstDSL.field(aggName); Field spanStartTime = AstDSL.field("@timestamp"); Function spanEndTime = timestampadd(span.getUnit(), span.getValue(), spanStartTime); + return eval( timechart(AstDSL.alias(aggName, aggregate("sum", aggFunc.getField()))), let( diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java index 02a9aa33250..3b4ca27dab5 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartCommandIT.java @@ -483,48 +483,6 @@ private void createEventsManyHostsIndex() throws IOException { } } - @Test - public void testTimechartPerSecond() throws IOException { - JSONObject result = executeQuery("source=events | timechart span=1m per_second(cpu_usage)"); - - // Verify schema - per_second should return a double value - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); - - // The test data should have some results, exact values depend on the test data - // but we can verify that results are returned and are numeric - assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); - - // Verify that per_second values are calculated correctly (should be sum/factor where factor = - // 60 for 1 minute) - // The exact verification of values would depend on the specific test data in events index - assertEquals(5, result.getInt("total")); - } - - @Test - public void testTimechartPerSecondWithHourSpan() throws IOException { - JSONObject result = executeQuery("source=events | timechart span=1h per_second(cpu_usage)"); - - // Verify schema - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); - - // With 1 hour span, the factor should be 3600 seconds - assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); - } - - @Test - public void testTimechartPerSecondWithDaySpan() throws IOException { - JSONObject result = executeQuery("source=events | timechart span=1d per_second(cpu_usage)"); - - // Verify schema - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(cpu_usage)", "double")); - - // With 1 day span, the factor should be 86400 seconds - assertTrue("Results should not be empty", result.getJSONArray("datarows").length() > 0); - } - private void createEventsNullIndex() throws IOException { String eventsMapping = "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"host\":{\"type\":\"text\"},\"cpu_usage\":{\"type\":\"double\"},\"region\":{\"type\":\"keyword\"}}}}"; From 79efdb368cdb1142a0c4c549a75331ed72ce797c Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 11:49:58 -0700 Subject: [PATCH 18/26] Refactor per function rate expresion build Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 58 +++++++++++++++---- .../sql/ppl/parser/AstExpressionBuilder.java | 7 ++- 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index b41759ed30c..b9f03da8936 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -9,8 +9,14 @@ import static org.opensearch.sql.ast.dsl.AstDSL.doubleLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.eval; import static org.opensearch.sql.ast.dsl.AstDSL.function; -import static org.opensearch.sql.ast.dsl.AstDSL.let; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; +import static org.opensearch.sql.ast.expression.IntervalUnit.SECOND; +import static org.opensearch.sql.calcite.plan.OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.DIVIDE; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.MULTIPLY; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.SUM; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.TIMESTAMPADD; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.TIMESTAMPDIFF; import com.google.common.collect.ImmutableList; import java.util.List; @@ -24,6 +30,8 @@ import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.Field; import org.opensearch.sql.ast.expression.Function; +import org.opensearch.sql.ast.expression.IntervalUnit; +import org.opensearch.sql.ast.expression.Let; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; import org.opensearch.sql.ast.expression.UnresolvedExpression; @@ -99,17 +107,14 @@ private UnresolvedPlan transformPerSecondFunction() { Span span = (Span) this.binExpression; String aggName = aggregationName(aggFunc); Field aggField = AstDSL.field(aggName); - Field spanStartTime = AstDSL.field("@timestamp"); + Field spanStartTime = AstDSL.field(IMPLICIT_FIELD_TIMESTAMP); Function spanEndTime = timestampadd(span.getUnit(), span.getValue(), spanStartTime); return eval( - timechart(AstDSL.alias(aggName, aggregate("sum", aggFunc.getField()))), - let( - aggField, - function( - "/", - function("*", aggField, perUnitSeconds()), - timestampdiff("SECOND", spanStartTime, spanEndTime)))); + timechart(AstDSL.alias(aggName, sum(aggFunc.getField()))), + let(aggField) + .multiply(perUnitSeconds()) + .dividedBy(timestampdiff(SECOND, spanStartTime, spanEndTime))); } private String aggregationName(AggregateFunction aggFunc) { @@ -127,15 +132,44 @@ private UnresolvedExpression perUnitSeconds() { return doubleLiteral(1.0); } + private UnresolvedExpression sum(UnresolvedExpression field) { + return aggregate(SUM.getName().getFunctionName(), field); + } + private Function timestampadd( SpanUnit unit, UnresolvedExpression value, UnresolvedExpression timestampField) { UnresolvedExpression intervalUnit = stringLiteral(PlanUtils.spanUnitToIntervalUnit(unit).toString()); - return function("timestampadd", intervalUnit, value, timestampField); + return function(TIMESTAMPADD.getName().getFunctionName(), intervalUnit, value, timestampField); } private Function timestampdiff( - String unit, UnresolvedExpression start, UnresolvedExpression end) { - return function("timestampdiff", stringLiteral(unit), start, end); + IntervalUnit unit, UnresolvedExpression start, UnresolvedExpression end) { + return function( + TIMESTAMPDIFF.getName().getFunctionName(), stringLiteral(unit.toString()), start, end); + } + + private PerFunctionRateExprBuilder let(Field field) { + return new PerFunctionRateExprBuilder(field); + } + + /** Fluent builder for creating Let expressions with mathematical operations. */ + private static class PerFunctionRateExprBuilder { + private final Field field; + private UnresolvedExpression expr; + + PerFunctionRateExprBuilder(Field field) { + this.field = field; + this.expr = field; + } + + PerFunctionRateExprBuilder multiply(UnresolvedExpression right) { + this.expr = function(MULTIPLY.getName().getFunctionName(), expr, right); + return this; + } + + Let dividedBy(UnresolvedExpression divisor) { + return AstDSL.let(field, function(DIVIDE.getName().getFunctionName(), expr, divisor)); + } } } 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 93ba1ae7dda..f037376f5c2 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 @@ -553,12 +553,13 @@ private List timestampFunctionArguments( @Override public UnresolvedExpression visitPerFunctionCall(PerFunctionCallContext ctx) { ParseTree parent = ctx.getParent(); - String functionName = ctx.perFunction().funcName.getText(); + String perFuncName = ctx.perFunction().funcName.getText(); if (!(parent instanceof TimechartCommandContext)) { throw new SyntaxCheckException( - functionName + " function can only be used within timechart command"); + perFuncName + " function can only be used within timechart command"); } - return buildAggregateFunction(functionName, Arrays.asList(ctx.perFunction().functionArg())); + return buildAggregateFunction( + perFuncName, Collections.singletonList(ctx.perFunction().functionArg())); } /** Literal and value. */ From 38a1039519158fbcba512c481bd957a3382300a7 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 13:47:16 -0700 Subject: [PATCH 19/26] Refactor test data into json files Signed-off-by: Chen Dai --- .../remote/CalciteTimechartPerFunctionIT.java | 42 +------------------ .../sql/legacy/SQLIntegTestCase.java | 7 +++- .../org/opensearch/sql/legacy/TestUtils.java | 5 +++ .../opensearch/sql/legacy/TestsConstants.java | 2 + .../timechart_per_function_index_mapping.json | 15 +++++++ .../resources/timechart_per_function.json | 8 ++++ 6 files changed, 37 insertions(+), 42 deletions(-) create mode 100644 integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json create mode 100644 integ-test/src/test/resources/timechart_per_function.json diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java index eaf2476773a..2343c60df6b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -5,8 +5,6 @@ package org.opensearch.sql.calcite.remote; -import static org.opensearch.sql.legacy.TestUtils.createIndexByRestClient; -import static org.opensearch.sql.legacy.TestUtils.isIndexExist; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; @@ -15,7 +13,6 @@ import java.io.IOException; import org.json.JSONObject; import org.junit.jupiter.api.Test; -import org.opensearch.client.Request; import org.opensearch.sql.ppl.PPLIntegTestCase; public class CalciteTimechartPerFunctionIT extends PPLIntegTestCase { @@ -26,44 +23,7 @@ public void init() throws Exception { enableCalcite(); disallowCalciteFallback(); - // Create dedicated per_function test data directly in init() - createPerFunctionTestIndex(); - } - - private void createPerFunctionTestIndex() throws IOException { - String mapping = - "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"packets\":{\"type\":\"integer\"},\"host\":{\"type\":\"keyword\"}}}}"; - - if (!isIndexExist(client(), "timechart_per_function_test")) { - createIndexByRestClient(client(), "timechart_per_function_test", mapping); - - // Insert test data directly with clean numbers that avoid decimal issues - insertTestData(); - } - } - - private void insertTestData() throws IOException { - // Create data with clean numbers that result in whole numbers or simple decimals - // Using multiples of 60 to get clean per_second calculations with 1m spans - String[] testData = { - "{\"@timestamp\":\"2025-09-08T10:00:00\",\"packets\":60,\"host\":\"server1\"}", // 60/60 = 1.0 - "{\"@timestamp\":\"2025-09-08T10:01:00\",\"packets\":120,\"host\":\"server1\"}", // 120/60 = - // 2.0 - "{\"@timestamp\":\"2025-09-08T10:02:00\",\"packets\":180,\"host\":\"server1\"}", // 180/60 = - // 3.0 - "{\"@timestamp\":\"2025-09-08T10:02:30\",\"packets\":30,\"host\":\"server2\"}" // For the by - // host tests - }; - - for (String data : testData) { - Request request = new Request("POST", "/timechart_per_function_test/_doc"); - request.setJsonEntity(data); - client().performRequest(request); - } - - // Refresh index to make data searchable - Request refresh = new Request("POST", "/timechart_per_function_test/_refresh"); - client().performRequest(refresh); + loadIndex(Index.TIMECHART_PER_FUNCTION); } @Test diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index fb3cad3f9f4..e4e7a6b6b29 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -906,7 +906,12 @@ public enum Index { "events_null", "events_null", "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"host\":{\"type\":\"text\"},\"cpu_usage\":{\"type\":\"double\"},\"region\":{\"type\":\"keyword\"}}}}", - "src/test/resources/events_null.json"); + "src/test/resources/events_null.json"), + TIMECHART_PER_FUNCTION( + "timechart_per_function_test", + "_doc", + getMappingFile("timechart_per_function_index_mapping.json"), + "src/test/resources/timechart_per_function.json"); private final String name; private final String type; diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java index a94e89ec0e6..2ea8aa17026 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java @@ -330,6 +330,11 @@ public static String getOtelLogsIndexMapping() { return getMappingFile(mappingFile); } + public static String getTimechartPerFunctionIndexMapping() { + String mappingFile = "timechart_per_function_index_mapping.json"; + return getMappingFile(mappingFile); + } + public static void loadBulk(Client client, String jsonPath, String defaultIndex) throws Exception { System.out.println(String.format("Loading file %s into opensearch cluster", jsonPath)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 76923dbd984..e2f37156cd5 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -91,6 +91,8 @@ public class TestsConstants { public static final String TEST_INDEX_LOGS = TEST_INDEX + "_logs"; public static final String TEST_INDEX_OTEL_LOGS = TEST_INDEX + "_otel_logs"; public static final String TEST_INDEX_TIME_DATE_NULL = TEST_INDEX + "_time_date_null"; + public static final String TEST_INDEX_TIMECHART_PER_FUNCTION = + TEST_INDEX + "_timechart_per_function"; public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; public static final String TS_DATE_FORMAT = "yyyy-MM-dd HH:mm:ss.SSS"; diff --git a/integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json new file mode 100644 index 00000000000..fcf3528fca1 --- /dev/null +++ b/integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json @@ -0,0 +1,15 @@ +{ + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "packets": { + "type": "integer" + }, + "host": { + "type": "keyword" + } + } + } +} diff --git a/integ-test/src/test/resources/timechart_per_function.json b/integ-test/src/test/resources/timechart_per_function.json new file mode 100644 index 00000000000..6257949c743 --- /dev/null +++ b/integ-test/src/test/resources/timechart_per_function.json @@ -0,0 +1,8 @@ +{"index":{"_id":"1"}} +{"@timestamp":"2025-09-08T10:00:00","packets":60,"host":"server1"} +{"index":{"_id":"2"}} +{"@timestamp":"2025-09-08T10:01:00","packets":120,"host":"server1"} +{"index":{"_id":"3"}} +{"@timestamp":"2025-09-08T10:02:00","packets":180,"host":"server1"} +{"index":{"_id":"4"}} +{"@timestamp":"2025-09-08T10:02:30","packets":30,"host":"server2"} From 832528f75994b741be2fbf8d05cc8bd72db65356 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 14:15:19 -0700 Subject: [PATCH 20/26] Add explain IT Signed-off-by: Chen Dai --- .../sql/calcite/remote/CalciteExplainIT.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java index 0e22507852e..ef6b008f554 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteExplainIT.java @@ -310,6 +310,16 @@ public void testExplainWithTimechartCount() throws IOException { assertYamlEqualsJsonIgnoreId(expected, result); } + @Test + public void testExplainTimechartPerSecond() throws IOException { + var result = explainQueryToString("source=events | timechart span=2m per_second(cpu_usage)"); + assertTrue( + result.contains( + "per_second(cpu_usage)=[DIVIDE(*($1, 1.0E0), " + + "TIMESTAMPDIFF('SECOND':VARCHAR, $0, TIMESTAMPADD('MINUTE':VARCHAR, 2, $0)))]")); + assertTrue(result.contains("per_second(cpu_usage)=[SUM($0)]")); + } + @Test public void noPushDownForAggOnWindow() throws IOException { enabledOnlyWhenPushdownIsEnabled(); From f2fedd383b61ff8398c9129e820e261091500de8 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Wed, 8 Oct 2025 15:44:41 -0700 Subject: [PATCH 21/26] Refactor IT test data and cases Signed-off-by: Chen Dai --- .../remote/CalciteTimechartPerFunctionIT.java | 80 +++++-------------- .../resources/timechart_per_function.json | 4 +- 2 files changed, 20 insertions(+), 64 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java index 2343c60df6b..295f52506bd 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -29,93 +29,53 @@ public void init() throws Exception { @Test public void testTimechartPerSecondWithDefaultSpan() throws IOException { JSONObject result = - executeQuery("source=timechart_per_function_test | timechart span=1m per_second(packets)"); + executeQuery("source=timechart_per_function_test | timechart per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - - // With 1m span: 60/60=1.0, 120/60=2.0, (180+30)/60=3.5 - // The 10:02:30 data falls into the 10:02:00 bucket verifyDataRows( result, - rows("2025-09-08 10:00:00", 1.0), - rows("2025-09-08 10:01:00", 2.0), - rows("2025-09-08 10:02:00", 3.5)); + rows("2025-09-08 10:00:00", 1.0), // 60 / 1m + rows("2025-09-08 10:01:00", 2.0), // 120 / 1m + rows("2025-09-08 10:02:00", 4.0)); // (60+180) / 1m } @Test - public void testTimechartPerSecondWithTwentySecondSpan() throws IOException { + public void testTimechartPerSecondWithSpecifiedSpan() throws IOException { JSONObject result = - executeQuery("source=timechart_per_function_test | timechart span=20s per_second(packets)"); + executeQuery("source=timechart_per_function_test | timechart span=2m per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - - // With 20s span: 60/20=3.0, 120/20=6.0, 180/20=9.0, 30/20=1.5 (correct floating point) verifyDataRows( result, - rows("2025-09-08 10:00:00", 3.0), - rows("2025-09-08 10:01:00", 6.0), - rows("2025-09-08 10:02:00", 9.0), - rows("2025-09-08 10:02:20", 1.5)); + rows("2025-09-08 10:00:00", 1.5), // (60+120) / 2m + rows("2025-09-08 10:02:00", 2.0)); // (60+180) / 2m } @Test - public void testTimechartPerSecondWithOneHourSpan() throws IOException { - JSONObject result = - executeQuery("source=timechart_per_function_test | timechart span=1h per_second(packets)"); - - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - - // With 1h span: (60+120+180+30)/3600 = 390/3600 = 0.10833333333333334 - verifyDataRows(result, rows("2025-09-08 10:00:00", 0.10833333333333334)); - } - - @Test - public void testTimechartPerSecondWithByLimitUseother() throws IOException { + public void testTimechartPerSecondWithByClause() throws IOException { JSONObject result = executeQuery( - "source=timechart_per_function_test | timechart span=1m limit=1 useother=true" - + " per_second(packets) by host"); + "source=timechart_per_function_test | timechart span=2m per_second(packets) by host"); verifySchema( result, schema("@timestamp", "timestamp"), schema("host", "string"), schema("per_second(packets)", "double")); - - // With limit=1 and useother=true, expected 4 rows instead of 6 - // server1 appears in buckets, OTHER shows correct floating point: 30/60 = 0.5 verifyDataRows( result, - rows("2025-09-08 10:00:00", "server1", 1.0), - rows("2025-09-08 10:01:00", "server1", 2.0), - rows("2025-09-08 10:02:00", "server1", 3.0), - rows("2025-09-08 10:02:00", "OTHER", 0.5)); // OTHER shows correct 30/60 = 0.5 + rows("2025-09-08 10:00:00", "server1", 1.5), // (60+120) / 2m + rows("2025-09-08 10:02:00", "server1", 0.5), // 60 / 2m + rows("2025-09-08 10:02:00", "server2", 1.5)); // 180 / 2m } @Test - public void testTimechartPerSecondDataRows() throws IOException { - JSONObject result = - executeQuery("source=timechart_per_function_test | timechart span=1m per_second(packets)"); - - verifySchema( - result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); - - // Verify the same data as testTimechartPerSecondWithDefaultSpan - verifyDataRows( - result, - rows("2025-09-08 10:00:00", 1.0), - rows("2025-09-08 10:01:00", 2.0), - rows("2025-09-08 10:02:00", 3.5)); - } - - @Test - public void testTimechartPerSecondWithByClauseAndLimit() throws IOException { + public void testTimechartPerSecondWithLimitAndByClause() throws IOException { JSONObject result = executeQuery( - "source=timechart_per_function_test | timechart span=30s limit=2 per_second(packets) by" + "source=timechart_per_function_test | timechart span=2m limit=1 per_second(packets) by" + " host"); verifySchema( @@ -123,14 +83,10 @@ public void testTimechartPerSecondWithByClauseAndLimit() throws IOException { schema("@timestamp", "timestamp"), schema("host", "string"), schema("per_second(packets)", "double")); - - // With 30s span and limit=2, expected 4 rows instead of 8 - // Only buckets with actual data, no zero-filled buckets verifyDataRows( result, - rows("2025-09-08 10:00:00", "server1", 2.0), // 60/30 = 2.0 - rows("2025-09-08 10:01:00", "server1", 4.0), // 120/30 = 4.0 - rows("2025-09-08 10:02:00", "server1", 6.0), // 180/30 = 6.0 - rows("2025-09-08 10:02:30", "server2", 1.0)); // 30/30 = 1.0 + rows("2025-09-08 10:00:00", "server1", 1.5), + rows("2025-09-08 10:02:00", "server1", 0.5), + rows("2025-09-08 10:02:00", "OTHER", 1.5)); } } diff --git a/integ-test/src/test/resources/timechart_per_function.json b/integ-test/src/test/resources/timechart_per_function.json index 6257949c743..2650fb9d5b1 100644 --- a/integ-test/src/test/resources/timechart_per_function.json +++ b/integ-test/src/test/resources/timechart_per_function.json @@ -3,6 +3,6 @@ {"index":{"_id":"2"}} {"@timestamp":"2025-09-08T10:01:00","packets":120,"host":"server1"} {"index":{"_id":"3"}} -{"@timestamp":"2025-09-08T10:02:00","packets":180,"host":"server1"} +{"@timestamp":"2025-09-08T10:02:00","packets":60,"host":"server1"} {"index":{"_id":"4"}} -{"@timestamp":"2025-09-08T10:02:30","packets":30,"host":"server2"} +{"@timestamp":"2025-09-08T10:02:30","packets":180,"host":"server2"} From 5ecf154fd2e8f883f74bd26eba02861e2e006dc5 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 9 Oct 2025 09:25:20 -0700 Subject: [PATCH 22/26] Rename IT test dataset Signed-off-by: Chen Dai --- .../remote/CalciteTimechartPerFunctionIT.java | 13 +++++-------- .../org/opensearch/sql/legacy/SQLIntegTestCase.java | 10 +++++----- .../java/org/opensearch/sql/legacy/TestUtils.java | 5 ----- .../org/opensearch/sql/legacy/TestsConstants.java | 2 -- ...echart_per_function.json => events_traffic.json} | 0 ...pping.json => events_traffic_index_mapping.json} | 2 +- 6 files changed, 11 insertions(+), 21 deletions(-) rename integ-test/src/test/resources/{timechart_per_function.json => events_traffic.json} (100%) rename integ-test/src/test/resources/indexDefinitions/{timechart_per_function_index_mapping.json => events_traffic_index_mapping.json} (99%) diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java index 295f52506bd..7a2eaeee62c 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -23,13 +23,12 @@ public void init() throws Exception { enableCalcite(); disallowCalciteFallback(); - loadIndex(Index.TIMECHART_PER_FUNCTION); + loadIndex(Index.EVENTS_TRAFFIC); } @Test public void testTimechartPerSecondWithDefaultSpan() throws IOException { - JSONObject result = - executeQuery("source=timechart_per_function_test | timechart per_second(packets)"); + JSONObject result = executeQuery("source=events_traffic | timechart per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); @@ -43,7 +42,7 @@ public void testTimechartPerSecondWithDefaultSpan() throws IOException { @Test public void testTimechartPerSecondWithSpecifiedSpan() throws IOException { JSONObject result = - executeQuery("source=timechart_per_function_test | timechart span=2m per_second(packets)"); + executeQuery("source=events_traffic | timechart span=2m per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); @@ -56,8 +55,7 @@ public void testTimechartPerSecondWithSpecifiedSpan() throws IOException { @Test public void testTimechartPerSecondWithByClause() throws IOException { JSONObject result = - executeQuery( - "source=timechart_per_function_test | timechart span=2m per_second(packets) by host"); + executeQuery("source=events_traffic | timechart span=2m per_second(packets) by host"); verifySchema( result, @@ -75,8 +73,7 @@ public void testTimechartPerSecondWithByClause() throws IOException { public void testTimechartPerSecondWithLimitAndByClause() throws IOException { JSONObject result = executeQuery( - "source=timechart_per_function_test | timechart span=2m limit=1 per_second(packets) by" - + " host"); + "source=events_traffic | timechart span=2m limit=1 per_second(packets) by host"); verifySchema( result, diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index e4e7a6b6b29..80616df7bfa 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -907,11 +907,11 @@ public enum Index { "events_null", "{\"mappings\":{\"properties\":{\"@timestamp\":{\"type\":\"date\"},\"host\":{\"type\":\"text\"},\"cpu_usage\":{\"type\":\"double\"},\"region\":{\"type\":\"keyword\"}}}}", "src/test/resources/events_null.json"), - TIMECHART_PER_FUNCTION( - "timechart_per_function_test", - "_doc", - getMappingFile("timechart_per_function_index_mapping.json"), - "src/test/resources/timechart_per_function.json"); + EVENTS_TRAFFIC( + "events_traffic", + "events_traffic", + getMappingFile("events_traffic_index_mapping.json"), + "src/test/resources/events_traffic.json"); private final String name; private final String type; diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java index 2ea8aa17026..a94e89ec0e6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java @@ -330,11 +330,6 @@ public static String getOtelLogsIndexMapping() { return getMappingFile(mappingFile); } - public static String getTimechartPerFunctionIndexMapping() { - String mappingFile = "timechart_per_function_index_mapping.json"; - return getMappingFile(mappingFile); - } - public static void loadBulk(Client client, String jsonPath, String defaultIndex) throws Exception { System.out.println(String.format("Loading file %s into opensearch cluster", jsonPath)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index e2f37156cd5..76923dbd984 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -91,8 +91,6 @@ public class TestsConstants { public static final String TEST_INDEX_LOGS = TEST_INDEX + "_logs"; public static final String TEST_INDEX_OTEL_LOGS = TEST_INDEX + "_otel_logs"; public static final String TEST_INDEX_TIME_DATE_NULL = TEST_INDEX + "_time_date_null"; - public static final String TEST_INDEX_TIMECHART_PER_FUNCTION = - TEST_INDEX + "_timechart_per_function"; public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; public static final String TS_DATE_FORMAT = "yyyy-MM-dd HH:mm:ss.SSS"; diff --git a/integ-test/src/test/resources/timechart_per_function.json b/integ-test/src/test/resources/events_traffic.json similarity index 100% rename from integ-test/src/test/resources/timechart_per_function.json rename to integ-test/src/test/resources/events_traffic.json diff --git a/integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/events_traffic_index_mapping.json similarity index 99% rename from integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json rename to integ-test/src/test/resources/indexDefinitions/events_traffic_index_mapping.json index fcf3528fca1..d8fe3b4cf55 100644 --- a/integ-test/src/test/resources/indexDefinitions/timechart_per_function_index_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/events_traffic_index_mapping.json @@ -12,4 +12,4 @@ } } } -} +} \ No newline at end of file From aef965ade81fdac79a95bc4f405fbe96a619b83b Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 9 Oct 2025 11:51:26 -0700 Subject: [PATCH 23/26] Refactor per second related logic Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 108 +++++++++++------- .../sql/ast/tree/TimechartTest.java | 4 +- 2 files changed, 67 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index b9f03da8936..c856baf6a09 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -11,6 +11,9 @@ import static org.opensearch.sql.ast.dsl.AstDSL.function; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import static org.opensearch.sql.ast.expression.IntervalUnit.SECOND; +import static org.opensearch.sql.ast.tree.Timechart.PerFunctionRateExprBuilder.sum; +import static org.opensearch.sql.ast.tree.Timechart.PerFunctionRateExprBuilder.timestampadd; +import static org.opensearch.sql.ast.tree.Timechart.PerFunctionRateExprBuilder.timestampdiff; import static org.opensearch.sql.calcite.plan.OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP; import static org.opensearch.sql.expression.function.BuiltinFunctionName.DIVIDE; import static org.opensearch.sql.expression.function.BuiltinFunctionName.MULTIPLY; @@ -21,9 +24,12 @@ import com.google.common.collect.ImmutableList; import java.util.List; import java.util.Locale; +import java.util.Map; +import java.util.Optional; import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.RequiredArgsConstructor; import lombok.ToString; import org.opensearch.sql.ast.AbstractNodeVisitor; import org.opensearch.sql.ast.dsl.AstDSL; @@ -88,73 +94,68 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { } /** - * Transform per function to Eval-based post-processing on sum result by Timechart. Specifically, + * Transform per function to eval-based post-processing on sum result by timechart. Specifically, * calculate how many seconds are in the time bucket based on the span option dynamically, then * divide the aggregated sum value by the number of seconds to get the per-second rate. * *

For example, with span=5m per_second(field): per second rate = sum(field) / 300 seconds * - *

TODO: extend to support additional per_* functions - * - * @return Eval+Timechart AST if per function present, or the original AST otherwise. + * @return eval+timechart if per function present, or the original timechart otherwise. */ private UnresolvedPlan transformPerSecondFunction() { - AggregateFunction aggFunc = (AggregateFunction) this.aggregateFunction; - if (!"per_second".equals(aggFunc.getFuncName())) { + Optional perFuncOpt = PerFunction.from(aggregateFunction); + if (perFuncOpt.isEmpty()) { return this; } + PerFunction perFunc = perFuncOpt.get(); Span span = (Span) this.binExpression; - String aggName = aggregationName(aggFunc); - Field aggField = AstDSL.field(aggName); Field spanStartTime = AstDSL.field(IMPLICIT_FIELD_TIMESTAMP); Function spanEndTime = timestampadd(span.getUnit(), span.getValue(), spanStartTime); + Function spanSeconds = timestampdiff(SECOND, spanStartTime, spanEndTime); return eval( - timechart(AstDSL.alias(aggName, sum(aggFunc.getField()))), - let(aggField) - .multiply(perUnitSeconds()) - .dividedBy(timestampdiff(SECOND, spanStartTime, spanEndTime))); - } - - private String aggregationName(AggregateFunction aggFunc) { - UnresolvedExpression field = aggFunc.getField(); - String fieldName = - field instanceof Field ? ((Field) field).getField().toString() : field.toString(); - return String.format(Locale.ROOT, "%s(%s)", aggFunc.getFuncName(), fieldName); + timechart(AstDSL.alias(perFunc.aggName, sum(perFunc.aggArg))), + let(perFunc.aggName).multiply(perFunc.seconds).dividedBy(spanSeconds)); } private Timechart timechart(UnresolvedExpression newAggregateFunction) { return this.toBuilder().aggregateFunction(newAggregateFunction).build(); } - private UnresolvedExpression perUnitSeconds() { - return doubleLiteral(1.0); - } - - private UnresolvedExpression sum(UnresolvedExpression field) { - return aggregate(SUM.getName().getFunctionName(), field); - } - - private Function timestampadd( - SpanUnit unit, UnresolvedExpression value, UnresolvedExpression timestampField) { - UnresolvedExpression intervalUnit = - stringLiteral(PlanUtils.spanUnitToIntervalUnit(unit).toString()); - return function(TIMESTAMPADD.getName().getFunctionName(), intervalUnit, value, timestampField); - } - - private Function timestampdiff( - IntervalUnit unit, UnresolvedExpression start, UnresolvedExpression end) { - return function( - TIMESTAMPDIFF.getName().getFunctionName(), stringLiteral(unit.toString()), start, end); + /** TODO: extend to support additional per_* functions */ + @RequiredArgsConstructor + private static final class PerFunction { + private static final Map UNIT_SECONDS = Map.of("per_second", 1); + private final String aggName; + private final UnresolvedExpression aggArg; + private final int seconds; + + static Optional from(UnresolvedExpression aggFunc) { + if (!(aggFunc instanceof AggregateFunction agg)) { + return Optional.empty(); + } + + String aggFuncName = agg.getFuncName().toLowerCase(Locale.ROOT); + if (!UNIT_SECONDS.containsKey(aggFuncName)) { + return Optional.empty(); + } + + String fieldName = + (agg.getField() instanceof Field) + ? ((Field) agg.getField()).getField().toString() + : agg.getField().toString(); + String aggName = String.format(Locale.ROOT, "%s(%s)", aggFuncName, fieldName); + return Optional.of(new PerFunction(aggName, agg.getField(), UNIT_SECONDS.get(aggFuncName))); + } } - private PerFunctionRateExprBuilder let(Field field) { - return new PerFunctionRateExprBuilder(field); + private PerFunctionRateExprBuilder let(String fieldName) { + return new PerFunctionRateExprBuilder(AstDSL.field(fieldName)); } /** Fluent builder for creating Let expressions with mathematical operations. */ - private static class PerFunctionRateExprBuilder { + static class PerFunctionRateExprBuilder { private final Field field; private UnresolvedExpression expr; @@ -163,13 +164,34 @@ private static class PerFunctionRateExprBuilder { this.expr = field; } - PerFunctionRateExprBuilder multiply(UnresolvedExpression right) { - this.expr = function(MULTIPLY.getName().getFunctionName(), expr, right); + PerFunctionRateExprBuilder multiply(Integer multiplier) { + // Promote to double literal to avoid integer division in downstream + this.expr = + function( + MULTIPLY.getName().getFunctionName(), expr, doubleLiteral(multiplier.doubleValue())); return this; } Let dividedBy(UnresolvedExpression divisor) { return AstDSL.let(field, function(DIVIDE.getName().getFunctionName(), expr, divisor)); } + + static UnresolvedExpression sum(UnresolvedExpression field) { + return aggregate(SUM.getName().getFunctionName(), field); + } + + static Function timestampadd( + SpanUnit unit, UnresolvedExpression value, UnresolvedExpression timestampField) { + UnresolvedExpression intervalUnit = + stringLiteral(PlanUtils.spanUnitToIntervalUnit(unit).toString()); + return function( + TIMESTAMPADD.getName().getFunctionName(), intervalUnit, value, timestampField); + } + + static Function timestampdiff( + IntervalUnit unit, UnresolvedExpression start, UnresolvedExpression end) { + return function( + TIMESTAMPDIFF.getName().getFunctionName(), stringLiteral(unit.toString()), start, end); + } } } diff --git a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java index c6e2fe06f9a..c23964d75a7 100644 --- a/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java +++ b/core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java @@ -29,7 +29,7 @@ class TimechartTest { @ParameterizedTest @CsvSource({"1, m, MINUTE", "30, s, SECOND", "5, m, MINUTE", "2, h, HOUR", "1, d, DAY"}) void should_transform_per_second_for_different_spans( - int spanValue, String spanUnit, String expectedTimestampAddUnit) { + int spanValue, String spanUnit, String expectedIntervalUnit) { withTimechart(span(spanValue, spanUnit), perSecond("bytes")) .whenTransformingPerFunction() .thenExpect( @@ -41,7 +41,7 @@ void should_transform_per_second_for_different_spans( timestampdiff( "SECOND", "@timestamp", - timestampadd(expectedTimestampAddUnit, spanValue, "@timestamp")))), + timestampadd(expectedIntervalUnit, spanValue, "@timestamp")))), timechart(span(spanValue, spanUnit), alias("per_second(bytes)", sum("bytes"))))); } From 83ee450e35b65a1eb1383c4daa36e48e932901a4 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 9 Oct 2025 14:17:49 -0700 Subject: [PATCH 24/26] Update doctest Signed-off-by: Chen Dai --- docs/user/ppl/cmd/timechart.rst | 37 +++++++++++++++++++++++++++++--- doctest/test_data/events.json | 16 +++++++------- doctest/test_mapping/events.json | 3 +++ 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/docs/user/ppl/cmd/timechart.rst b/docs/user/ppl/cmd/timechart.rst index a5708769035..0e1c2cf5360 100644 --- a/docs/user/ppl/cmd/timechart.rst +++ b/docs/user/ppl/cmd/timechart.rst @@ -57,14 +57,28 @@ Syntax * When set to true, values beyond the limit are grouped into an "OTHER" category. * Only applies when using the "by" clause and when there are more distinct values than the limit. +* **by**: optional. Groups the results by the specified field in addition to time intervals. + + * If not specified, the aggregation is performed across all documents in each time interval. + * **aggregation_function**: mandatory. The aggregation function to apply to each time bucket. * Currently, only a single aggregation function is supported. - * Available functions: All aggregation functions supported by the :doc:`stats ` command are supported. + * Available functions: All aggregation functions supported by the :doc:`stats ` command, as well as the timechart-specific aggregations listed below. -* **by**: optional. Groups the results by the specified field in addition to time intervals. +PER_SECOND +---------- - * If not specified, the aggregation is performed across all documents in each time interval. +Description +>>>>>>>>>>> + +Usage: per_second(field) calculates the per-second rate for a numeric field within each time bucket. + +The calculation formula is: `per_second(field) = sum(field) / span_in_seconds`, where `span_in_seconds` is the span interval in seconds. + +Note: This function is available since 3.4.0. + +Return type: DOUBLE Notes ===== @@ -332,3 +346,20 @@ PPL query:: | 2024-07-01 00:00:00 | null | 1 | +---------------------+--------+-------+ +Example 11: Calculate packets per second rate +============================================= + +This example calculates the per-second packet rate for network traffic data using the per_second() function. + +PPL query:: + + os> source=events | timechart span=30m per_second(packets) by host + fetched rows / total rows = 4/4 + +---------------------+---------+---------------------+ + | @timestamp | host | per_second(packets) | + |---------------------+---------+---------------------| + | 2023-01-01 10:00:00 | server1 | 0.1 | + | 2023-01-01 10:00:00 | server2 | 0.05 | + | 2023-01-01 10:30:00 | server1 | 0.1 | + | 2023-01-01 10:30:00 | server2 | 0.05 | + +---------------------+---------+---------------------+ diff --git a/doctest/test_data/events.json b/doctest/test_data/events.json index e873691fb90..ea63088151a 100644 --- a/doctest/test_data/events.json +++ b/doctest/test_data/events.json @@ -1,8 +1,8 @@ -{"@timestamp":"2023-01-01T10:00:00Z","event_time":"2023-01-01T09:55:00Z","host":"server1","message":"Starting up","level":"INFO","category":"orders","status":"pending"} -{"@timestamp":"2023-01-01T10:05:00Z","event_time":"2023-01-01T10:00:00Z","host":"server2","message":"Initializing","level":"INFO","category":"users","status":"active"} -{"@timestamp":"2023-01-01T10:10:00Z","event_time":"2023-01-01T10:05:00Z","host":"server1","message":"Ready to serve","level":"INFO","category":"orders","status":"processing"} -{"@timestamp":"2023-01-01T10:15:00Z","event_time":"2023-01-01T10:10:00Z","host":"server2","message":"Ready","level":"INFO","category":"users","status":"inactive"} -{"@timestamp":"2023-01-01T10:20:00Z","event_time":"2023-01-01T10:15:00Z","host":"server1","message":"Processing requests","level":"INFO","category":"orders","status":"completed"} -{"@timestamp":"2023-01-01T10:25:00Z","event_time":"2023-01-01T10:20:00Z","host":"server2","message":"Handling connections","level":"INFO","category":"users","status":"pending"} -{"@timestamp":"2023-01-01T10:30:00Z","event_time":"2023-01-01T10:25:00Z","host":"server1","message":"Shutting down","level":"WARN","category":"orders","status":"cancelled"} -{"@timestamp":"2023-01-01T10:35:00Z","event_time":"2023-01-01T10:30:00Z","host":"server2","message":"Maintenance mode","level":"WARN","category":"users","status":"inactive"} +{"@timestamp":"2023-01-01T10:00:00Z","event_time":"2023-01-01T09:55:00Z","host":"server1","message":"Starting up","level":"INFO","category":"orders","status":"pending","packets":60} +{"@timestamp":"2023-01-01T10:05:00Z","event_time":"2023-01-01T10:00:00Z","host":"server2","message":"Initializing","level":"INFO","category":"users","status":"active","packets":30} +{"@timestamp":"2023-01-01T10:10:00Z","event_time":"2023-01-01T10:05:00Z","host":"server1","message":"Ready to serve","level":"INFO","category":"orders","status":"processing","packets":60} +{"@timestamp":"2023-01-01T10:15:00Z","event_time":"2023-01-01T10:10:00Z","host":"server2","message":"Ready","level":"INFO","category":"users","status":"inactive","packets":30} +{"@timestamp":"2023-01-01T10:20:00Z","event_time":"2023-01-01T10:15:00Z","host":"server1","message":"Processing requests","level":"INFO","category":"orders","status":"completed","packets":60} +{"@timestamp":"2023-01-01T10:25:00Z","event_time":"2023-01-01T10:20:00Z","host":"server2","message":"Handling connections","level":"INFO","category":"users","status":"pending","packets":30} +{"@timestamp":"2023-01-01T10:30:00Z","event_time":"2023-01-01T10:25:00Z","host":"server1","message":"Shutting down","level":"WARN","category":"orders","status":"cancelled","packets":180} +{"@timestamp":"2023-01-01T10:35:00Z","event_time":"2023-01-01T10:30:00Z","host":"server2","message":"Maintenance mode","level":"WARN","category":"users","status":"inactive","packets":90} diff --git a/doctest/test_mapping/events.json b/doctest/test_mapping/events.json index 664f042324b..2c405a23fbb 100644 --- a/doctest/test_mapping/events.json +++ b/doctest/test_mapping/events.json @@ -23,6 +23,9 @@ }, "status": { "type": "keyword" + }, + "packets": { + "type": "integer" } } } From 519ad5945fed6550482f44acdecd9344e0604ca0 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Thu, 9 Oct 2025 14:54:53 -0700 Subject: [PATCH 25/26] Rename transform function Signed-off-by: Chen Dai --- .../opensearch/sql/ast/tree/Timechart.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java index c856baf6a09..05646a363f6 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/Timechart.java @@ -80,7 +80,7 @@ public Timechart useOther(Boolean useOther) { @Override public UnresolvedPlan attach(UnresolvedPlan child) { // Transform after child attached to avoid unintentionally overriding it - return toBuilder().child(child).build().transformPerSecondFunction(); + return toBuilder().child(child).build().transformPerFunction(); } @Override @@ -102,7 +102,7 @@ public T accept(AbstractNodeVisitor nodeVisitor, C context) { * * @return eval+timechart if per function present, or the original timechart otherwise. */ - private UnresolvedPlan transformPerSecondFunction() { + private UnresolvedPlan transformPerFunction() { Optional perFuncOpt = PerFunction.from(aggregateFunction); if (perFuncOpt.isEmpty()) { return this; @@ -125,28 +125,34 @@ private Timechart timechart(UnresolvedExpression newAggregateFunction) { /** TODO: extend to support additional per_* functions */ @RequiredArgsConstructor - private static final class PerFunction { + static class PerFunction { private static final Map UNIT_SECONDS = Map.of("per_second", 1); private final String aggName; private final UnresolvedExpression aggArg; private final int seconds; - static Optional from(UnresolvedExpression aggFunc) { - if (!(aggFunc instanceof AggregateFunction agg)) { + static Optional from(UnresolvedExpression aggExpr) { + if (!(aggExpr instanceof AggregateFunction)) { return Optional.empty(); } - String aggFuncName = agg.getFuncName().toLowerCase(Locale.ROOT); + AggregateFunction aggFunc = (AggregateFunction) aggExpr; + String aggFuncName = aggFunc.getFuncName().toLowerCase(Locale.ROOT); if (!UNIT_SECONDS.containsKey(aggFuncName)) { return Optional.empty(); } + String aggName = toAggName(aggFunc); + return Optional.of( + new PerFunction(aggName, aggFunc.getField(), UNIT_SECONDS.get(aggFuncName))); + } + + private static String toAggName(AggregateFunction aggFunc) { String fieldName = - (agg.getField() instanceof Field) - ? ((Field) agg.getField()).getField().toString() - : agg.getField().toString(); - String aggName = String.format(Locale.ROOT, "%s(%s)", aggFuncName, fieldName); - return Optional.of(new PerFunction(aggName, agg.getField(), UNIT_SECONDS.get(aggFuncName))); + (aggFunc.getField() instanceof Field) + ? ((Field) aggFunc.getField()).getField().toString() + : aggFunc.getField().toString(); + return String.format(Locale.ROOT, "%s(%s)", aggFunc.getFuncName(), fieldName); } } From 995171e7da73d18e63d482f3cdc94241ddadd1ce Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Mon, 13 Oct 2025 10:48:14 -0700 Subject: [PATCH 26/26] Address PR comments Signed-off-by: Chen Dai --- .../sql/calcite/utils/PlanUtils.java | 63 +++++++++++++++---- .../remote/CalciteTimechartPerFunctionIT.java | 30 +++++++-- .../src/test/resources/events_traffic.json | 4 ++ 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java b/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java index d49c7b7b607..0998120e489 100644 --- a/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java +++ b/core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java @@ -75,19 +75,56 @@ static SpanUnit intervalUnitToSpanUnit(IntervalUnit unit) { } static IntervalUnit spanUnitToIntervalUnit(SpanUnit unit) { - return switch (unit) { - case MILLISECOND, MS -> IntervalUnit.MICROSECOND; - case SECOND, SECONDS, SEC, SECS, S -> IntervalUnit.SECOND; - case MINUTE, MINUTES, MIN, MINS, m -> IntervalUnit.MINUTE; - case HOUR, HOURS, HR, HRS, H -> IntervalUnit.HOUR; - case DAY, DAYS, D -> IntervalUnit.DAY; - case WEEK, WEEKS, W -> IntervalUnit.WEEK; - case MONTH, MONTHS, MON, M -> IntervalUnit.MONTH; - case QUARTER, QUARTERS, QTR, QTRS, Q -> IntervalUnit.QUARTER; - case YEAR, YEARS, Y -> IntervalUnit.YEAR; - case UNKNOWN -> IntervalUnit.UNKNOWN; - default -> throw new UnsupportedOperationException("Unsupported span unit: " + unit); - }; + switch (unit) { + case MILLISECOND: + case MS: + return IntervalUnit.MICROSECOND; + case SECOND: + case SECONDS: + case SEC: + case SECS: + case S: + return IntervalUnit.SECOND; + case MINUTE: + case MINUTES: + case MIN: + case MINS: + case m: + return IntervalUnit.MINUTE; + case HOUR: + case HOURS: + case HR: + case HRS: + case H: + return IntervalUnit.HOUR; + case DAY: + case DAYS: + case D: + return IntervalUnit.DAY; + case WEEK: + case WEEKS: + case W: + return IntervalUnit.WEEK; + case MONTH: + case MONTHS: + case MON: + case M: + return IntervalUnit.MONTH; + case QUARTER: + case QUARTERS: + case QTR: + case QTRS: + case Q: + return IntervalUnit.QUARTER; + case YEAR: + case YEARS: + case Y: + return IntervalUnit.YEAR; + case UNKNOWN: + return IntervalUnit.UNKNOWN; + default: + throw new UnsupportedOperationException("Unsupported span unit: " + unit); + } } static RexNode makeOver( diff --git a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java index 7a2eaeee62c..9965d459a22 100644 --- a/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalciteTimechartPerFunctionIT.java @@ -28,7 +28,9 @@ public void init() throws Exception { @Test public void testTimechartPerSecondWithDefaultSpan() throws IOException { - JSONObject result = executeQuery("source=events_traffic | timechart per_second(packets)"); + JSONObject result = + executeQuery( + "source=events_traffic | where month(@timestamp) = 9 | timechart per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); @@ -42,7 +44,9 @@ public void testTimechartPerSecondWithDefaultSpan() throws IOException { @Test public void testTimechartPerSecondWithSpecifiedSpan() throws IOException { JSONObject result = - executeQuery("source=events_traffic | timechart span=2m per_second(packets)"); + executeQuery( + "source=events_traffic | where month(@timestamp) = 9 | timechart span=2m" + + " per_second(packets)"); verifySchema( result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); @@ -55,7 +59,9 @@ public void testTimechartPerSecondWithSpecifiedSpan() throws IOException { @Test public void testTimechartPerSecondWithByClause() throws IOException { JSONObject result = - executeQuery("source=events_traffic | timechart span=2m per_second(packets) by host"); + executeQuery( + "source=events_traffic | where month(@timestamp) = 9 | timechart span=2m" + + " per_second(packets) by host"); verifySchema( result, @@ -73,7 +79,8 @@ public void testTimechartPerSecondWithByClause() throws IOException { public void testTimechartPerSecondWithLimitAndByClause() throws IOException { JSONObject result = executeQuery( - "source=events_traffic | timechart span=2m limit=1 per_second(packets) by host"); + "source=events_traffic | where month(@timestamp) = 9 | timechart span=2m limit=1" + + " per_second(packets) by host"); verifySchema( result, @@ -86,4 +93,19 @@ public void testTimechartPerSecondWithLimitAndByClause() throws IOException { rows("2025-09-08 10:02:00", "server1", 0.5), rows("2025-09-08 10:02:00", "OTHER", 1.5)); } + + @Test + public void testTimechartPerSecondWithVariableMonthLengths() throws IOException { + JSONObject result = + executeQuery( + "source=events_traffic | where month(@timestamp) != 9 | timechart span=1M" + + " per_second(packets)"); + + verifySchema( + result, schema("@timestamp", "timestamp"), schema("per_second(packets)", "double")); + verifyDataRows( + result, + rows("2025-02-01 00:00:00", 7.75), // 18748800 / 28 days' seconds + rows("2025-10-01 00:00:00", 7.0)); // 18748800 / 31 days' seconds + } } diff --git a/integ-test/src/test/resources/events_traffic.json b/integ-test/src/test/resources/events_traffic.json index 2650fb9d5b1..bcb3fe17f2a 100644 --- a/integ-test/src/test/resources/events_traffic.json +++ b/integ-test/src/test/resources/events_traffic.json @@ -6,3 +6,7 @@ {"@timestamp":"2025-09-08T10:02:00","packets":60,"host":"server1"} {"index":{"_id":"4"}} {"@timestamp":"2025-09-08T10:02:30","packets":180,"host":"server2"} +{"index":{"_id":"5"}} +{"@timestamp":"2025-02-15T14:00:00","packets":18748800,"host":"server1"} +{"index":{"_id":"6"}} +{"@timestamp":"2025-10-15T14:00:00","packets":18748800,"host":"server1"}