Skip to content

Commit 2030961

Browse files
yuancuyuancu
committed
Migrate TimecharTest to use Chart
Signed-off-by: Yuanchun Shen <[email protected]>
1 parent 6dc5d00 commit 2030961

File tree

6 files changed

+74
-45
lines changed

6 files changed

+74
-45
lines changed

‎core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,7 @@ public static Bin bin(UnresolvedExpression field, Argument... arguments) {
728728
}
729729

730730
/** Get a reference to the implicit timestamp field {@code @timestamp} */
731-
public static Field referImplicitTimestampField() {
731+
public static Field implicitTimestampField() {
732732
return AstDSL.field(OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP);
733733
}
734734
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,13 @@
1111
import static org.opensearch.sql.ast.dsl.AstDSL.function;
1212
import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral;
1313
import static org.opensearch.sql.ast.expression.IntervalUnit.MILLISECOND;
14-
import static org.opensearch.sql.calcite.plan.OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP;
14+
import static org.opensearch.sql.ast.tree.Chart.PerFunctionRateExprBuilder.timestampadd;
15+
import static org.opensearch.sql.ast.tree.Chart.PerFunctionRateExprBuilder.timestampdiff;
1516
import static org.opensearch.sql.expression.function.BuiltinFunctionName.DIVIDE;
1617
import static org.opensearch.sql.expression.function.BuiltinFunctionName.MULTIPLY;
1718
import static org.opensearch.sql.expression.function.BuiltinFunctionName.SUM;
1819
import static org.opensearch.sql.expression.function.BuiltinFunctionName.TIMESTAMPADD;
1920
import static org.opensearch.sql.expression.function.BuiltinFunctionName.TIMESTAMPDIFF;
20-
import static org.opensearch.sql.ast.tree.Chart.PerFunctionRateExprBuilder.timestampadd;
21-
import static org.opensearch.sql.ast.tree.Chart.PerFunctionRateExprBuilder.timestampdiff;
2221

2322
import com.google.common.collect.ImmutableList;
2423
import java.util.List;
@@ -107,7 +106,7 @@ private UnresolvedPlan transformPerFunction() {
107106
}
108107

109108
Span span = (Span) spanExpr;
110-
Field spanStartTime = AstDSL.field(IMPLICIT_FIELD_TIMESTAMP);
109+
Field spanStartTime = AstDSL.implicitTimestampField();
111110
Function spanEndTime = timestampadd(span.getUnit(), span.getValue(), spanStartTime);
112111
Function spanMillis = timestampdiff(MILLISECOND, spanStartTime, spanEndTime);
113112
final int SECOND_IN_MILLISECOND = 1000;
@@ -135,9 +134,10 @@ static class PerFunction {
135134
private final int seconds;
136135

137136
static Optional<PerFunction> from(UnresolvedExpression aggExpr) {
138-
if (aggExpr instanceof Alias) {
139-
return from(((Alias) aggExpr).getDelegated());
140-
};
137+
if (aggExpr instanceof Alias) {
138+
return from(((Alias) aggExpr).getDelegated());
139+
}
140+
;
141141
if (!(aggExpr instanceof AggregateFunction)) {
142142
return Optional.empty();
143143
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.opensearch.sql.ast.AbstractNodeVisitor;
3535
import org.opensearch.sql.ast.dsl.AstDSL;
3636
import org.opensearch.sql.ast.expression.AggregateFunction;
37-
import org.opensearch.sql.ast.expression.Argument;
3837
import org.opensearch.sql.ast.expression.Field;
3938
import org.opensearch.sql.ast.expression.Function;
4039
import org.opensearch.sql.ast.expression.IntervalUnit;

‎core/src/test/java/org/opensearch/sql/ast/tree/TimechartTest.java‎ renamed to ‎core/src/test/java/org/opensearch/sql/ast/tree/PerFunctionsTest.java‎

Lines changed: 63 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,25 @@
1313
import static org.opensearch.sql.ast.dsl.AstDSL.function;
1414
import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral;
1515
import static org.opensearch.sql.ast.dsl.AstDSL.relation;
16+
import static org.opensearch.sql.calcite.plan.OpenSearchConstants.IMPLICIT_FIELD_TIMESTAMP;
1617

18+
import java.util.List;
1719
import java.util.stream.Stream;
1820
import org.junit.jupiter.api.Test;
1921
import org.junit.jupiter.params.ParameterizedTest;
2022
import org.junit.jupiter.params.provider.Arguments;
2123
import org.junit.jupiter.params.provider.MethodSource;
2224
import org.opensearch.sql.ast.dsl.AstDSL;
2325
import org.opensearch.sql.ast.expression.AggregateFunction;
26+
import org.opensearch.sql.ast.expression.Argument;
2427
import org.opensearch.sql.ast.expression.Let;
28+
import org.opensearch.sql.ast.expression.Literal;
2529
import org.opensearch.sql.ast.expression.Span;
2630
import org.opensearch.sql.ast.expression.SpanUnit;
2731
import org.opensearch.sql.ast.expression.UnresolvedExpression;
32+
import org.opensearch.sql.expression.function.BuiltinFunctionName;
2833

29-
class TimechartTest {
30-
34+
class PerFunctionsTest {
3135
/**
3236
* @return test sources for per_* function test.
3337
*/
@@ -57,8 +61,9 @@ void should_transform_per_second_for_different_spans(
5761
multiply("per_second(bytes)", 1000.0),
5862
timestampdiff(
5963
"MILLISECOND",
60-
"@timestamp",
61-
timestampadd(expectedIntervalUnit, spanValue, "@timestamp")))),
64+
IMPLICIT_FIELD_TIMESTAMP,
65+
timestampadd(
66+
expectedIntervalUnit, spanValue, IMPLICIT_FIELD_TIMESTAMP)))),
6267
timechart(span(spanValue, spanUnit), alias("per_second(bytes)", sum("bytes")))));
6368
}
6469

@@ -76,8 +81,9 @@ void should_transform_per_minute_for_different_spans(
7681
multiply("per_minute(bytes)", 60000.0),
7782
timestampdiff(
7883
"MILLISECOND",
79-
"@timestamp",
80-
timestampadd(expectedIntervalUnit, spanValue, "@timestamp")))),
84+
IMPLICIT_FIELD_TIMESTAMP,
85+
timestampadd(
86+
expectedIntervalUnit, spanValue, IMPLICIT_FIELD_TIMESTAMP)))),
8187
timechart(span(spanValue, spanUnit), alias("per_minute(bytes)", sum("bytes")))));
8288
}
8389

@@ -95,8 +101,9 @@ void should_transform_per_hour_for_different_spans(
95101
multiply("per_hour(bytes)", 3600000.0),
96102
timestampdiff(
97103
"MILLISECOND",
98-
"@timestamp",
99-
timestampadd(expectedIntervalUnit, spanValue, "@timestamp")))),
104+
IMPLICIT_FIELD_TIMESTAMP,
105+
timestampadd(
106+
expectedIntervalUnit, spanValue, IMPLICIT_FIELD_TIMESTAMP)))),
100107
timechart(span(spanValue, spanUnit), alias("per_hour(bytes)", sum("bytes")))));
101108
}
102109

@@ -114,8 +121,9 @@ void should_transform_per_day_for_different_spans(
114121
multiply("per_day(bytes)", 8.64E7),
115122
timestampdiff(
116123
"MILLISECOND",
117-
"@timestamp",
118-
timestampadd(expectedIntervalUnit, spanValue, "@timestamp")))),
124+
IMPLICIT_FIELD_TIMESTAMP,
125+
timestampadd(
126+
expectedIntervalUnit, spanValue, IMPLICIT_FIELD_TIMESTAMP)))),
119127
timechart(span(spanValue, spanUnit), alias("per_day(bytes)", sum("bytes")))));
120128
}
121129

@@ -128,19 +136,27 @@ void should_not_transform_non_per_functions() {
128136

129137
@Test
130138
void should_preserve_all_fields_during_per_function_transformation() {
131-
Timechart original =
132-
new Timechart(relation("logs"), perSecond("bytes"))
133-
.span(span(5, "m"))
134-
.by(field("status"))
135-
.limit(20)
136-
.useOther(false);
137-
138-
Timechart expected =
139-
new Timechart(relation("logs"), alias("per_second(bytes)", sum("bytes")))
140-
.span(span(5, "m"))
141-
.by(field("status"))
142-
.limit(20)
143-
.useOther(false);
139+
Chart original =
140+
Chart.builder()
141+
.child(relation("logs"))
142+
.aggregationFunction(perSecond("bytes"))
143+
.rowSplit(span(5, "m"))
144+
.columnSplit(field("status"))
145+
.arguments(
146+
List.of(
147+
new Argument("limit", intLiteral(20)), new Argument("useOther", Literal.FALSE)))
148+
.build();
149+
150+
Chart expected =
151+
Chart.builder()
152+
.child(relation("logs"))
153+
.aggregationFunction(alias("per_second(bytes)", sum("bytes")))
154+
.rowSplit(span(5, "m"))
155+
.columnSplit(field("status"))
156+
.arguments(
157+
List.of(
158+
new Argument("limit", intLiteral(20)), new Argument("useOther", Literal.FALSE)))
159+
.build();
144160

145161
withTimechart(original)
146162
.whenTransformingPerFunction()
@@ -151,7 +167,9 @@ void should_preserve_all_fields_during_per_function_transformation() {
151167
divide(
152168
multiply("per_second(bytes)", 1000.0),
153169
timestampdiff(
154-
"MILLISECOND", "@timestamp", timestampadd("MINUTE", 5, "@timestamp")))),
170+
"MILLISECOND",
171+
IMPLICIT_FIELD_TIMESTAMP,
172+
timestampadd("MINUTE", 5, IMPLICIT_FIELD_TIMESTAMP)))),
155173
expected));
156174
}
157175

@@ -161,17 +179,21 @@ private static TransformationAssertion withTimechart(Span spanExpr, AggregateFun
161179
return new TransformationAssertion(timechart(spanExpr, aggFunc));
162180
}
163181

164-
private static TransformationAssertion withTimechart(Timechart timechart) {
182+
private static TransformationAssertion withTimechart(Chart timechart) {
165183
return new TransformationAssertion(timechart);
166184
}
167185

168-
private static Timechart timechart(Span spanExpr, UnresolvedExpression aggExpr) {
186+
private static Chart timechart(Span spanExpr, UnresolvedExpression aggExpr) {
169187
// Set child here because expected object won't call attach below
170-
return new Timechart(relation("t"), aggExpr).span(spanExpr).limit(10).useOther(true);
188+
return Chart.builder()
189+
.child(relation("t"))
190+
.aggregationFunction(aggExpr)
191+
.rowSplit(spanExpr)
192+
.build();
171193
}
172194

173195
private static Span span(int value, String unit) {
174-
return AstDSL.span(field("@timestamp"), intLiteral(value), SpanUnit.of(unit));
196+
return AstDSL.span(AstDSL.implicitTimestampField(), intLiteral(value), SpanUnit.of(unit));
175197
}
176198

177199
private static AggregateFunction perSecond(String fieldName) {
@@ -209,23 +231,31 @@ private static UnresolvedExpression divide(
209231

210232
private static UnresolvedExpression timestampadd(String unit, int value, String timestampField) {
211233
return function(
212-
"timestampadd", AstDSL.stringLiteral(unit), intLiteral(value), field(timestampField));
234+
BuiltinFunctionName.TIMESTAMPADD.getName().getFunctionName(),
235+
AstDSL.stringLiteral(unit),
236+
intLiteral(value),
237+
field(timestampField));
213238
}
214239

215240
private static UnresolvedExpression timestampdiff(
216241
String unit, String startField, UnresolvedExpression end) {
217-
return function("timestampdiff", AstDSL.stringLiteral(unit), field(startField), end);
242+
243+
return function(
244+
BuiltinFunctionName.TIMESTAMPDIFF.getName().getFunctionName(),
245+
AstDSL.stringLiteral(unit),
246+
field(startField),
247+
end);
218248
}
219249

220-
private static UnresolvedPlan eval(Let letExpr, Timechart timechartExpr) {
250+
private static UnresolvedPlan eval(Let letExpr, Chart timechartExpr) {
221251
return AstDSL.eval(timechartExpr, letExpr);
222252
}
223253

224254
private static class TransformationAssertion {
225-
private final Timechart timechart;
255+
private final Chart timechart;
226256
private UnresolvedPlan result;
227257

228-
TransformationAssertion(Timechart timechart) {
258+
TransformationAssertion(Chart timechart) {
229259
this.timechart = timechart;
230260
}
231261

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ private List<UnresolvedExpression> parseAggTerms(
744744
@Override
745745
public UnresolvedPlan visitTimechartCommand(OpenSearchPPLParser.TimechartCommandContext ctx) {
746746
UnresolvedExpression binExpression =
747-
AstDSL.span(AstDSL.referImplicitTimestampField(), AstDSL.intLiteral(1), SpanUnit.m);
747+
AstDSL.span(AstDSL.implicitTimestampField(), AstDSL.intLiteral(1), SpanUnit.m);
748748
Integer limit = 10;
749749
Boolean useOther = true;
750750
// Process timechart parameters

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ public UnresolvedExpression visitSpanClause(SpanClauseContext ctx) {
678678
if (ctx.fieldExpression() != null) {
679679
fieldExpression = visit(ctx.fieldExpression());
680680
} else {
681-
fieldExpression = AstDSL.referImplicitTimestampField();
681+
fieldExpression = AstDSL.implicitTimestampField();
682682
}
683683
Literal literal = (Literal) visit(ctx.value);
684684
return AstDSL.spanFromSpanLengthLiteral(fieldExpression, literal);
@@ -984,7 +984,7 @@ public UnresolvedExpression visitTimechartParameter(
984984
// Convert span=1h to span(@timestamp, 1h)
985985
Literal spanLiteral = (Literal) visit(ctx.spanLiteral());
986986
timechartParameter =
987-
AstDSL.spanFromSpanLengthLiteral(AstDSL.referImplicitTimestampField(), spanLiteral);
987+
AstDSL.spanFromSpanLengthLiteral(AstDSL.implicitTimestampField(), spanLiteral);
988988
} else if (ctx.LIMIT() != null) {
989989
Literal limit = (Literal) visit(ctx.integerLiteral());
990990
if ((Integer) limit.getValue() < 0) {

0 commit comments

Comments
 (0)