Skip to content

Commit 5c17695

Browse files
yuancuyuancu
committed
Support param timefield to specify span field in timechart
Signed-off-by: Yuanchun Shen <[email protected]>
1 parent daf1795 commit 5c17695

File tree

6 files changed

+89
-76
lines changed

6 files changed

+89
-76
lines changed

‎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)");

‎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
@@ -1562,6 +1563,7 @@ searchableKeyWord
15621563
| SED
15631564
| MAX_MATCH
15641565
| OFFSET_FIELD
1566+
| TIMEFIELD
15651567
| patternMethod
15661568
| patternMode
15671569
// 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;
@@ -760,41 +758,28 @@ private List<UnresolvedExpression> parseAggTerms(
760758
/** Timechart command. */
761759
@Override
762760
public UnresolvedPlan visitTimechartCommand(OpenSearchPPLParser.TimechartCommandContext ctx) {
763-
UnresolvedExpression binExpression =
764-
AstDSL.span(AstDSL.implicitTimestampField(), AstDSL.intLiteral(1), SpanUnit.m);
765-
Integer limit = 10;
766-
Boolean useOther = true;
767-
// Process timechart parameters
768-
for (OpenSearchPPLParser.TimechartParameterContext paramCtx : ctx.timechartParameter()) {
769-
UnresolvedExpression param = internalVisitExpression(paramCtx);
770-
if (param instanceof Span) {
771-
binExpression = param;
772-
} else if (param instanceof Literal literal) {
773-
if (DataType.BOOLEAN.equals(literal.getType())) {
774-
useOther = (Boolean) literal.getValue();
775-
} else if (DataType.INTEGER.equals(literal.getType())
776-
|| DataType.LONG.equals(literal.getType())) {
777-
limit = (Integer) literal.getValue();
778-
}
779-
}
780-
}
761+
List<Argument> arguments = ArgumentFactory.getArgumentList(ctx, expressionBuilder);
762+
ArgumentMap argMap = ArgumentMap.of(arguments);
763+
Literal spanLiteral = argMap.getOrDefault("spanliteral", AstDSL.stringLiteral("1m"));
764+
String timeFieldName =
765+
Optional.ofNullable(argMap.get("timefield"))
766+
.map(l -> (String) l.getValue())
767+
.orElse(OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP);
768+
Field spanField = AstDSL.field(timeFieldName);
769+
Alias span =
770+
AstDSL.alias(timeFieldName, AstDSL.spanFromSpanLengthLiteral(spanField, spanLiteral));
781771
UnresolvedExpression aggregateFunction = parseAggTerms(List.of(ctx.statsAggTerm())).getFirst();
782-
783772
UnresolvedExpression byField =
784-
ctx.fieldExpression() != null ? internalVisitExpression(ctx.fieldExpression()) : null;
785-
List<Argument> arguments =
786-
List.of(
787-
new Argument("limit", AstDSL.intLiteral(limit)),
788-
new Argument("useother", AstDSL.booleanLiteral(useOther)));
789-
binExpression = AstDSL.alias(OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP, binExpression);
790-
if (byField != null) {
791-
byField =
792-
AstDSL.alias(
793-
StringUtils.unquoteIdentifier(getTextInQuery(ctx.fieldExpression())), byField);
794-
}
773+
Optional.ofNullable(ctx.fieldExpression())
774+
.map(
775+
f ->
776+
AstDSL.alias(
777+
StringUtils.unquoteIdentifier(getTextInQuery(f)),
778+
internalVisitExpression(f)))
779+
.orElse(null);
795780
return Chart.builder()
796781
.aggregationFunction(aggregateFunction)
797-
.rowSplit(binExpression)
782+
.rowSplit(span)
798783
.columnSplit(byField)
799784
.arguments(arguments)
800785
.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
@@ -749,7 +749,7 @@ public UnresolvedExpression visitMaxOption(OpenSearchPPLParser.MaxOptionContext
749749
return new Argument("max", (Literal) this.visit(ctx.integerLiteral()));
750750
}
751751

752-
private QualifiedName visitIdentifiers(List<? extends ParserRuleContext> ctx) {
752+
public QualifiedName visitIdentifiers(List<? extends ParserRuleContext> ctx) {
753753
return new QualifiedName(
754754
ctx.stream()
755755
.map(RuleContext::getText)
@@ -978,47 +978,6 @@ public UnresolvedExpression visitTimeModifierValue(
978978
return AstDSL.stringLiteral(osDateMathExpression);
979979
}
980980

981-
@Override
982-
public UnresolvedExpression visitTimechartParameter(
983-
OpenSearchPPLParser.TimechartParameterContext ctx) {
984-
UnresolvedExpression timechartParameter;
985-
if (ctx.SPAN() != null) {
986-
// Convert span=1h to span(@timestamp, 1h)
987-
Literal spanLiteral = (Literal) visit(ctx.spanLiteral());
988-
timechartParameter =
989-
AstDSL.spanFromSpanLengthLiteral(AstDSL.implicitTimestampField(), spanLiteral);
990-
} else if (ctx.LIMIT() != null) {
991-
Literal limit = (Literal) visit(ctx.integerLiteral());
992-
if ((Integer) limit.getValue() < 0) {
993-
throw new IllegalArgumentException("Limit must be a non-negative number");
994-
}
995-
timechartParameter = limit;
996-
} else if (ctx.USEOTHER() != null) {
997-
UnresolvedExpression useOther;
998-
if (ctx.booleanLiteral() != null) {
999-
useOther = visit(ctx.booleanLiteral());
1000-
} else if (ctx.ident() != null) {
1001-
QualifiedName ident = visitIdentifiers(List.of(ctx.ident()));
1002-
String useOtherValue = ident.toString();
1003-
if ("true".equalsIgnoreCase(useOtherValue) || "t".equalsIgnoreCase(useOtherValue)) {
1004-
useOther = AstDSL.booleanLiteral(true);
1005-
} else if ("false".equalsIgnoreCase(useOtherValue) || "f".equalsIgnoreCase(useOtherValue)) {
1006-
useOther = AstDSL.booleanLiteral(false);
1007-
} else {
1008-
throw new IllegalArgumentException(
1009-
"Invalid useOther value: " + ctx.ident().getText() + ". Expected true/false or t/f");
1010-
}
1011-
} else {
1012-
throw new IllegalArgumentException("value for useOther must be a boolean or identifier");
1013-
}
1014-
timechartParameter = useOther;
1015-
} else {
1016-
throw new IllegalArgumentException(
1017-
String.format("A parameter of timechart must be a span, limit or useOther, got %s", ctx));
1018-
}
1019-
return timechartParameter;
1020-
}
1021-
1022981
/**
1023982
* Process time range expressions (EARLIEST='value' or LATEST='value') It creates a Comparison
1024983
* 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
@@ -32,6 +32,7 @@
3232
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SortFieldContext;
3333
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.StreamstatsCommandContext;
3434
import org.opensearch.sql.ppl.antlr.parser.OpenSearchPPLParser.SuffixSortFieldContext;
35+
import org.opensearch.sql.ppl.parser.AstExpressionBuilder;
3536

3637
/** Util class to get all arguments as a list from the PPL command. */
3738
public class ArgumentFactory {
@@ -232,6 +233,60 @@ public static List<Argument> getArgumentList(ChartCommandContext ctx) {
232233
return arguments;
233234
}
234235

236+
public static List<Argument> getArgumentList(
237+
OpenSearchPPLParser.TimechartCommandContext timechartCtx,
238+
AstExpressionBuilder expressionBuilder) {
239+
List<Argument> arguments = new ArrayList<>();
240+
for (OpenSearchPPLParser.TimechartParameterContext ctx : timechartCtx.timechartParameter()) {
241+
if (ctx.SPAN() != null) {
242+
arguments.add(
243+
new Argument("spanliteral", (Literal) expressionBuilder.visit(ctx.spanLiteral())));
244+
} else if (ctx.LIMIT() != null) {
245+
Literal limit = getArgumentValue(ctx.integerLiteral());
246+
if ((Integer) limit.getValue() < 0) {
247+
throw new IllegalArgumentException("Limit must be a non-negative number");
248+
}
249+
arguments.add(new Argument("limit", limit));
250+
} else if (ctx.USEOTHER() != null) {
251+
Literal useOther;
252+
if (ctx.booleanLiteral() != null) {
253+
useOther = getArgumentValue(ctx.booleanLiteral());
254+
} else if (ctx.ident() != null) {
255+
String identLiteral = expressionBuilder.visitIdentifiers(List.of(ctx.ident())).toString();
256+
if ("true".equalsIgnoreCase(identLiteral) || "t".equalsIgnoreCase(identLiteral)) {
257+
useOther = AstDSL.booleanLiteral(true);
258+
} else if ("false".equalsIgnoreCase(identLiteral) || "f".equalsIgnoreCase(identLiteral)) {
259+
useOther = AstDSL.booleanLiteral(false);
260+
} else {
261+
throw new IllegalArgumentException(
262+
"Invalid useOther value: "
263+
+ ctx.ident().getText()
264+
+ ". Expected true/false or t/f");
265+
}
266+
} else {
267+
throw new IllegalArgumentException("value for useOther must be a boolean or identifier");
268+
}
269+
arguments.add(new Argument("useother", useOther));
270+
} else if (ctx.TIMEFIELD() != null) {
271+
Literal timeField;
272+
if (ctx.ident() != null) {
273+
timeField =
274+
AstDSL.stringLiteral(
275+
expressionBuilder.visitIdentifiers(List.of(ctx.ident())).toString());
276+
} else {
277+
timeField = getArgumentValue(ctx.stringLiteral());
278+
}
279+
arguments.add(new Argument("timefield", timeField));
280+
} else {
281+
throw new IllegalArgumentException(
282+
String.format(
283+
"A parameter of timechart must be a span, limit, useother, or timefield, got %s",
284+
ctx));
285+
}
286+
}
287+
return arguments;
288+
}
289+
235290
/**
236291
* Get list of {@link Argument}.
237292
*

0 commit comments

Comments
 (0)