Skip to content

Commit 5c2f50b

Browse files
yuancuasifabashar
yuancu
authored and
asifabashar
committed
Specify timestamp field with timefield in timechart command (opensearch-project#4784)
* Support param timefield to specify span field in timechart Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Update doc to introduce timefield parameter Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Update ASTBuilderTest for chart: default args are handled in rel node visitor Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Fix ast expression builder test Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Fix anomanyzer test Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Support using specified timefield in per functions Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Omit by-timestamp clause in timechart command Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Mask timefield argument in anonymizer Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> * Anonymize argument span Signed-off-by: Yuanchun Shen <yuanchu@amazon.com> --------- Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>
1 parent 9ecd4a9 commit 5c2f50b

File tree

13 files changed

+166
-145
lines changed

13 files changed

+166
-145
lines changed

‎core/src/main/java/org/opensearch/sql/ast/tree/Chart.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,14 @@ private UnresolvedPlan transformPerFunction() {
9797

9898
PerFunction perFunc = perFuncOpt.get();
9999
// For chart, the rowSplit should contain the span information
100-
UnresolvedExpression spanExpr = rowSplit;
101-
if (rowSplit instanceof Alias) {
102-
spanExpr = ((Alias) rowSplit).getDelegated();
103-
}
100+
UnresolvedExpression spanExpr =
101+
rowSplit instanceof Alias ? ((Alias) rowSplit).getDelegated() : rowSplit;
104102
if (!(spanExpr instanceof Span)) {
105103
return this; // Cannot transform without span information
106104
}
107105

108106
Span span = (Span) spanExpr;
109-
Field spanStartTime = AstDSL.implicitTimestampField();
107+
Field spanStartTime = (Field) span.getField();
110108
Function spanEndTime = timestampadd(span.getUnit(), span.getValue(), spanStartTime);
111109
Function spanMillis = timestampdiff(MILLISECOND, spanStartTime, spanEndTime);
112110
final int SECOND_IN_MILLISECOND = 1000;

‎docs/user/ppl/cmd/timechart.rst‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ Description
1616
Syntax
1717
======
1818

19-
timechart [span=<time_interval>] [limit=<number>] [useother=<boolean>] <aggregation_function> [by <field>]
19+
timechart [timefield=<field_name>] [span=<time_interval>] [limit=<number>] [useother=<boolean>] <aggregation_function> [by <field>]
20+
21+
* timefield: optional. Specifies the timestamp field to use for time interval grouping. **Default**: ``@timestamp``.
2022

2123
* span: optional. Specifies the time interval for grouping data. **Default:** 1m (1 minute).
2224

@@ -92,7 +94,7 @@ Return type: DOUBLE
9294
Notes
9395
=====
9496

95-
* The ``timechart`` command requires a timestamp field named ``@timestamp`` in the data.
97+
* The ``timechart`` command requires a timestamp field in the data. By default, it uses the ``@timestamp`` field, but you can specify a different field using the ``timefield`` parameter.
9698
* Results are returned in an unpivoted format with separate rows for each time-field combination that has data.
9799
* Only combinations with actual data are included in the results - empty combinations are omitted rather than showing null or zero values.
98100
* The "top N" values for the ``limit`` parameter are selected based on the sum of values across all time intervals for each distinct field value.

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.json.JSONObject;
1414
import org.junit.jupiter.api.Test;
1515
import org.opensearch.client.ResponseException;
16+
import org.opensearch.sql.common.utils.StringUtils;
1617
import org.opensearch.sql.ppl.PPLIntegTestCase;
1718

1819
public class CalciteTimechartCommandIT extends PPLIntegTestCase {
@@ -64,7 +65,7 @@ public void testTimechartWithMinuteSpanAndGroupBy() throws IOException {
6465
}
6566

6667
@Test
67-
public void testTimechartWithoutTimestampField() throws IOException {
68+
public void testTimechartWithoutTimestampField() {
6869
Throwable exception =
6970
assertThrows(
7071
ResponseException.class,
@@ -74,6 +75,16 @@ public void testTimechartWithoutTimestampField() throws IOException {
7475
verifyErrorMessageContains(exception, "Field [@timestamp] not found.");
7576
}
7677

78+
@Test
79+
public void testTimechartWithCustomTimeField() throws IOException {
80+
JSONObject result =
81+
executeQuery(
82+
StringUtils.format(
83+
"source=%s | timechart timefield=birthdate span=1year count()", TEST_INDEX_BANK));
84+
verifySchema(result, schema("birthdate", "timestamp"), schema("count()", "bigint"));
85+
verifyDataRows(result, rows("2017-01-01 00:00:00", 2), rows("2018-01-01 00:00:00", 5));
86+
}
87+
7788
@Test
7889
public void testTimechartWithMinuteSpanNoGroupBy() throws IOException {
7990
JSONObject result = executeQuery("source=events | timechart span=1m avg(cpu_usage)");

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

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

8+
import static org.opensearch.sql.util.MatcherUtils.closeTo;
89
import static org.opensearch.sql.util.MatcherUtils.rows;
910
import static org.opensearch.sql.util.MatcherUtils.schema;
1011
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
@@ -13,6 +14,8 @@
1314
import java.io.IOException;
1415
import org.json.JSONObject;
1516
import org.junit.jupiter.api.Test;
17+
import org.opensearch.sql.common.utils.StringUtils;
18+
import org.opensearch.sql.legacy.TestsConstants;
1619
import org.opensearch.sql.ppl.PPLIntegTestCase;
1720

1821
public class CalciteTimechartPerFunctionIT extends PPLIntegTestCase {
@@ -24,6 +27,7 @@ public void init() throws Exception {
2427
disallowCalciteFallback();
2528

2629
loadIndex(Index.EVENTS_TRAFFIC);
30+
loadIndex(Index.BANK);
2731
}
2832

2933
@Test
@@ -208,4 +212,26 @@ public void testTimechartPerDayWithByClause() throws IOException {
208212
rows("2025-09-08 10:02:00", "server1", 43200.0), // 60 * 720
209213
rows("2025-09-08 10:02:00", "server2", 129600.0)); // 180 * 720
210214
}
215+
216+
@Test
217+
public void testTimechartPerMonthWithSpecifiedSpan() throws IOException {
218+
JSONObject result =
219+
executeQuery(
220+
StringUtils.format(
221+
"source=%s | timechart timefield=birthdate span=1month per_day(balance) by gender",
222+
TestsConstants.TEST_INDEX_BANK));
223+
verifySchema(
224+
result,
225+
schema("birthdate", "timestamp"),
226+
schema("gender", "string"),
227+
schema("per_day(balance)", "double"));
228+
verifyDataRows(
229+
result,
230+
closeTo("2017-10-01 00:00:00", "M", 1265.3225806451612),
231+
closeTo("2017-11-01 00:00:00", "M", 189.53333333333333),
232+
closeTo("2018-06-01 00:00:00", "F", 1094.6),
233+
closeTo("2018-06-01 00:00:00", "M", 547.2666666666667),
234+
closeTo("2018-08-01 00:00:00", "F", 2858.9032258064517),
235+
closeTo("2018-11-01 00:00:00", "M", 139.33333333333334));
236+
}
211237
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ LIMIT: 'LIMIT';
145145
USEOTHER: 'USEOTHER';
146146
OTHERSTR: 'OTHERSTR';
147147
NULLSTR: 'NULLSTR';
148+
TIMEFIELD: 'TIMEFIELD';
148149
INPUT: 'INPUT';
149150
OUTPUT: 'OUTPUT';
150151
PATH: 'PATH';

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ timechartParameter
328328
: LIMIT EQUAL integerLiteral
329329
| SPAN EQUAL spanLiteral
330330
| USEOTHER EQUAL (booleanLiteral | ident)
331+
| TIMEFIELD EQUAL (ident | stringLiteral)
331332
;
332333

333334
spanLiteral
@@ -1572,6 +1573,7 @@ searchableKeyWord
15721573
| SED
15731574
| MAX_MATCH
15741575
| OFFSET_FIELD
1576+
| TIMEFIELD
15751577
| patternMethod
15761578
| patternMode
15771579
// AGGREGATIONS AND WINDOW

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

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@
6363
import org.opensearch.sql.ast.expression.SearchAnd;
6464
import org.opensearch.sql.ast.expression.SearchExpression;
6565
import org.opensearch.sql.ast.expression.SearchGroup;
66-
import org.opensearch.sql.ast.expression.Span;
67-
import org.opensearch.sql.ast.expression.SpanUnit;
6866
import org.opensearch.sql.ast.expression.UnresolvedArgument;
6967
import org.opensearch.sql.ast.expression.UnresolvedExpression;
7068
import org.opensearch.sql.ast.expression.WindowFrame;
@@ -771,41 +769,28 @@ private List<UnresolvedExpression> parseAggTerms(
771769
/** Timechart command. */
772770
@Override
773771
public UnresolvedPlan visitTimechartCommand(OpenSearchPPLParser.TimechartCommandContext ctx) {
774-
UnresolvedExpression binExpression =
775-
AstDSL.span(AstDSL.implicitTimestampField(), AstDSL.intLiteral(1), SpanUnit.m);
776-
Integer limit = 10;
777-
Boolean useOther = true;
778-
// Process timechart parameters
779-
for (OpenSearchPPLParser.TimechartParameterContext paramCtx : ctx.timechartParameter()) {
780-
UnresolvedExpression param = internalVisitExpression(paramCtx);
781-
if (param instanceof Span) {
782-
binExpression = param;
783-
} else if (param instanceof Literal literal) {
784-
if (DataType.BOOLEAN.equals(literal.getType())) {
785-
useOther = (Boolean) literal.getValue();
786-
} else if (DataType.INTEGER.equals(literal.getType())
787-
|| DataType.LONG.equals(literal.getType())) {
788-
limit = (Integer) literal.getValue();
789-
}
790-
}
791-
}
772+
List<Argument> arguments = ArgumentFactory.getArgumentList(ctx, expressionBuilder);
773+
ArgumentMap argMap = ArgumentMap.of(arguments);
774+
Literal spanLiteral = argMap.getOrDefault("spanliteral", AstDSL.stringLiteral("1m"));
775+
String timeFieldName =
776+
Optional.ofNullable(argMap.get("timefield"))
777+
.map(l -> (String) l.getValue())
778+
.orElse(OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP);
779+
Field spanField = AstDSL.field(timeFieldName);
780+
Alias span =
781+
AstDSL.alias(timeFieldName, AstDSL.spanFromSpanLengthLiteral(spanField, spanLiteral));
792782
UnresolvedExpression aggregateFunction = parseAggTerms(List.of(ctx.statsAggTerm())).getFirst();
793-
794783
UnresolvedExpression byField =
795-
ctx.fieldExpression() != null ? internalVisitExpression(ctx.fieldExpression()) : null;
796-
List<Argument> arguments =
797-
List.of(
798-
new Argument("limit", AstDSL.intLiteral(limit)),
799-
new Argument("useother", AstDSL.booleanLiteral(useOther)));
800-
binExpression = AstDSL.alias(OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP, binExpression);
801-
if (byField != null) {
802-
byField =
803-
AstDSL.alias(
804-
StringUtils.unquoteIdentifier(getTextInQuery(ctx.fieldExpression())), byField);
805-
}
784+
Optional.ofNullable(ctx.fieldExpression())
785+
.map(
786+
f ->
787+
AstDSL.alias(
788+
StringUtils.unquoteIdentifier(getTextInQuery(f)),
789+
internalVisitExpression(f)))
790+
.orElse(null);
806791
return Chart.builder()
807792
.aggregationFunction(aggregateFunction)
808-
.rowSplit(binExpression)
793+
.rowSplit(span)
809794
.columnSplit(byField)
810795
.arguments(arguments)
811796
.build();

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

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@ public UnresolvedExpression visitMaxOption(OpenSearchPPLParser.MaxOptionContext
757757
return new Argument("max", (Literal) this.visit(ctx.integerLiteral()));
758758
}
759759

760-
private QualifiedName visitIdentifiers(List<? extends ParserRuleContext> ctx) {
760+
public QualifiedName visitIdentifiers(List<? extends ParserRuleContext> ctx) {
761761
return new QualifiedName(
762762
ctx.stream()
763763
.map(RuleContext::getText)
@@ -995,47 +995,6 @@ public UnresolvedExpression visitTimeModifierValue(
995995
return AstDSL.stringLiteral(osDateMathExpression);
996996
}
997997

998-
@Override
999-
public UnresolvedExpression visitTimechartParameter(
1000-
OpenSearchPPLParser.TimechartParameterContext ctx) {
1001-
UnresolvedExpression timechartParameter;
1002-
if (ctx.SPAN() != null) {
1003-
// Convert span=1h to span(@timestamp, 1h)
1004-
Literal spanLiteral = (Literal) visit(ctx.spanLiteral());
1005-
timechartParameter =
1006-
AstDSL.spanFromSpanLengthLiteral(AstDSL.implicitTimestampField(), spanLiteral);
1007-
} else if (ctx.LIMIT() != null) {
1008-
Literal limit = (Literal) visit(ctx.integerLiteral());
1009-
if ((Integer) limit.getValue() < 0) {
1010-
throw new IllegalArgumentException("Limit must be a non-negative number");
1011-
}
1012-
timechartParameter = limit;
1013-
} else if (ctx.USEOTHER() != null) {
1014-
UnresolvedExpression useOther;
1015-
if (ctx.booleanLiteral() != null) {
1016-
useOther = visit(ctx.booleanLiteral());
1017-
} else if (ctx.ident() != null) {
1018-
QualifiedName ident = visitIdentifiers(List.of(ctx.ident()));
1019-
String useOtherValue = ident.toString();
1020-
if ("true".equalsIgnoreCase(useOtherValue) || "t".equalsIgnoreCase(useOtherValue)) {
1021-
useOther = AstDSL.booleanLiteral(true);
1022-
} else if ("false".equalsIgnoreCase(useOtherValue) || "f".equalsIgnoreCase(useOtherValue)) {
1023-
useOther = AstDSL.booleanLiteral(false);
1024-
} else {
1025-
throw new IllegalArgumentException(
1026-
"Invalid useOther value: " + ctx.ident().getText() + ". Expected true/false or t/f");
1027-
}
1028-
} else {
1029-
throw new IllegalArgumentException("value for useOther must be a boolean or identifier");
1030-
}
1031-
timechartParameter = useOther;
1032-
} else {
1033-
throw new IllegalArgumentException(
1034-
String.format("A parameter of timechart must be a span, limit or useOther, got %s", ctx));
1035-
}
1036-
return timechartParameter;
1037-
}
1038-
1039998
/**
1040999
* Process time range expressions (EARLIEST='value' or LATEST='value') It creates a Comparison
10411000
* filter like @timestamp >= timeModifierValue

‎ppl/src/main/java/org/opensearch/sql/ppl/utils/ArgumentFactory.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SortFieldContext;
3434
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.StreamstatsCommandContext;
3535
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SuffixSortFieldContext;
36+
import org.opensearch.sql.ppl.parser.AstExpressionBuilder;
3637

3738
/** Util class to get all arguments as a list from the PPL command. */
3839
public class ArgumentFactory {
@@ -253,6 +254,60 @@ public static List<Argument> getArgumentList(ChartCommandContext ctx) {
253254
return arguments;
254255
}
255256

257+
public static List<Argument> getArgumentList(
258+
OpenSearchPPLParser.TimechartCommandContext timechartCtx,
259+
AstExpressionBuilder expressionBuilder) {
260+
List<Argument> arguments = new ArrayList<>();
261+
for (OpenSearchPPLParser.TimechartParameterContext ctx : timechartCtx.timechartParameter()) {
262+
if (ctx.SPAN() != null) {
263+
arguments.add(
264+
new Argument("spanliteral", (Literal) expressionBuilder.visit(ctx.spanLiteral())));
265+
} else if (ctx.LIMIT() != null) {
266+
Literal limit = getArgumentValue(ctx.integerLiteral());
267+
if ((Integer) limit.getValue() < 0) {
268+
throw new IllegalArgumentException("Limit must be a non-negative number");
269+
}
270+
arguments.add(new Argument("limit", limit));
271+
} else if (ctx.USEOTHER() != null) {
272+
Literal useOther;
273+
if (ctx.booleanLiteral() != null) {
274+
useOther = getArgumentValue(ctx.booleanLiteral());
275+
} else if (ctx.ident() != null) {
276+
String identLiteral = expressionBuilder.visitIdentifiers(List.of(ctx.ident())).toString();
277+
if ("true".equalsIgnoreCase(identLiteral) || "t".equalsIgnoreCase(identLiteral)) {
278+
useOther = AstDSL.booleanLiteral(true);
279+
} else if ("false".equalsIgnoreCase(identLiteral) || "f".equalsIgnoreCase(identLiteral)) {
280+
useOther = AstDSL.booleanLiteral(false);
281+
} else {
282+
throw new IllegalArgumentException(
283+
"Invalid useOther value: "
284+
+ ctx.ident().getText()
285+
+ ". Expected true/false or t/f");
286+
}
287+
} else {
288+
throw new IllegalArgumentException("value for useOther must be a boolean or identifier");
289+
}
290+
arguments.add(new Argument("useother", useOther));
291+
} else if (ctx.TIMEFIELD() != null) {
292+
Literal timeField;
293+
if (ctx.ident() != null) {
294+
timeField =
295+
AstDSL.stringLiteral(
296+
expressionBuilder.visitIdentifiers(List.of(ctx.ident())).toString());
297+
} else {
298+
timeField = getArgumentValue(ctx.stringLiteral());
299+
}
300+
arguments.add(new Argument("timefield", timeField));
301+
} else {
302+
throw new IllegalArgumentException(
303+
String.format(
304+
"A parameter of timechart must be a span, limit, useother, or timefield, got %s",
305+
ctx));
306+
}
307+
}
308+
return arguments;
309+
}
310+
256311
/**
257312
* Get list of {@link Argument}.
258313
*

0 commit comments

Comments
 (0)