Skip to content

Commit 96c6a39

Browse files
authored
Throw an exception if ChartStep gets no aggregation columns (#513)
* Moved code from AbstractChartStep to ChartStep, added an exception if ChartStep gets no aggregation columns and added missing unit tests * removed get prefix from method names * rebased * removed unnecessary nullcheck
1 parent 109d7ae commit 96c6a39

4 files changed

Lines changed: 73 additions & 90 deletions

File tree

src/main/java/com/teragrep/pth_10/steps/chart/AbstractChartStep.java

Lines changed: 0 additions & 71 deletions
This file was deleted.

src/main/java/com/teragrep/pth_10/steps/chart/ChartStep.java

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
*/
4646
package com.teragrep.pth_10.steps.chart;
4747

48+
import com.teragrep.functions.dpf_02.AbstractStep;
4849
import org.apache.spark.sql.Column;
4950
import org.apache.spark.sql.Dataset;
5051
import org.apache.spark.sql.Row;
@@ -53,29 +54,38 @@
5354

5455
import java.util.List;
5556

56-
public final class ChartStep extends AbstractChartStep {
57+
public final class ChartStep extends AbstractStep {
5758

58-
public ChartStep(List<Column> listOfExpr, List<Column> listOfGroupBy) {
59-
super(listOfExpr, listOfGroupBy);
59+
private final List<Column> listOfAggrExpressions;
60+
private final List<Column> groupByList;
61+
62+
public ChartStep(final List<Column> listOfAggrExpressions, final List<Column> groupByList) {
63+
this.listOfAggrExpressions = listOfAggrExpressions;
64+
this.groupByList = groupByList;
6065
this.properties.add(CommandProperty.AGGREGATE);
6166
}
6267

6368
@Override
64-
public Dataset<Row> get(Dataset<Row> dataset) {
65-
if (dataset == null) {
66-
return null;
67-
}
68-
69-
if (listOfExpr.isEmpty()) {
70-
return null;
69+
public Dataset<Row> get(final Dataset<Row> dataset) {
70+
if (listOfAggrExpressions.isEmpty()) {
71+
throw new RuntimeException("ChartStep did not receive the necessary aggregation columns");
7172
}
7273

73-
Column mainExpr = listOfExpr.get(0);
74+
final Column mainExpr = listOfAggrExpressions.get(0);
7475

7576
// skip first one as .agg has strange arguments
76-
Seq<Column> seqOfExpr = JavaConversions.asScalaBuffer(listOfExpr.subList(1, listOfExpr.size()));
77-
Seq<Column> seqOfGroupBy = JavaConversions.asScalaBuffer(listOfGroupBy);
77+
final Seq<Column> seqOfExpr = JavaConversions
78+
.asScalaBuffer(listOfAggrExpressions.subList(1, listOfAggrExpressions.size()));
79+
final Seq<Column> seqOfGroupBy = JavaConversions.asScalaBuffer(groupByList);
7880

7981
return dataset.groupBy(seqOfGroupBy).agg(mainExpr, seqOfExpr);
8082
}
83+
84+
public List<Column> aggrExpressionsList() {
85+
return listOfAggrExpressions;
86+
}
87+
88+
public List<Column> groupByList() {
89+
return groupByList;
90+
}
8191
}

src/test/java/com/teragrep/pth_10/chartTransformationTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,4 +406,48 @@ void testSplittingByNumericalString() {
406406
Assertions.assertArrayEquals(expectedCount, count.stream().map(r -> r.getAs(0).toString()).toArray());
407407
});
408408
}
409+
410+
@Test
411+
public void testChartCountWithoutColumn() {
412+
String query = "index = index_A | chart count";
413+
414+
this.streamingTestUtil.performDPLTest(query, this.testFile, res -> {
415+
final StructType expectedSchema = new StructType(new StructField[] {
416+
new StructField("count", DataTypes.LongType, true, new MetadataBuilder().build())
417+
});
418+
Assertions.assertEquals(expectedSchema, res.schema()); // check that the schema is correct
419+
420+
List<String> resultList = res
421+
.select("count")
422+
.collectAsList()
423+
.stream()
424+
.map(r -> r.getAs(0).toString())
425+
.collect(Collectors.toList());
426+
427+
Assertions.assertEquals(1, resultList.size()); // only one row of data
428+
Assertions.assertEquals("5", resultList.get(0)); // the one row has the value 5
429+
});
430+
}
431+
432+
@Test
433+
public void testChartCountWithEmptyDataset() {
434+
String query = "index = index_A earliest = 2011-10-10T10:10:10.100+03:00 | chart count";
435+
436+
this.streamingTestUtil.performDPLTest(query, this.testFile, res -> {
437+
final StructType expectedSchema = new StructType(new StructField[] {
438+
new StructField("count", DataTypes.LongType, true, new MetadataBuilder().build())
439+
});
440+
Assertions.assertEquals(expectedSchema, res.schema()); // check that the schema is correct
441+
442+
List<String> resultList = res
443+
.select("count")
444+
.collectAsList()
445+
.stream()
446+
.map(r -> r.getAs(0).toString())
447+
.collect(Collectors.toList());
448+
449+
Assertions.assertEquals(1, resultList.size()); // only one row of data
450+
Assertions.assertEquals("0", resultList.get(0)); // the one row has the value 0
451+
});
452+
}
409453
}

src/test/java/com/teragrep/pth_10/translationTests/ChartTest.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,9 +81,9 @@ void testChartTranslation() {
8181
Assertions
8282
.assertEquals(
8383
"[countaggregator(input[0, java.lang.Long, true].longValue AS value, staticinvoke(class java.lang.Long, ObjectType(class java.lang.Long), valueOf, input[0, bigint, true], true, false, true), input[0, java.lang.Long, true].longValue) AS `count(_raw)`]",
84-
Arrays.toString(cs.getListOfExpr().toArray())
84+
Arrays.toString(cs.aggrExpressionsList().toArray())
8585
);
86-
Assertions.assertEquals("[_time]", Arrays.toString(cs.getListOfGroupBy().toArray()));
86+
Assertions.assertEquals("[_time]", Arrays.toString(cs.groupByList().toArray()));
8787

8888
}
8989

@@ -105,9 +105,9 @@ void testChartTranslation_multiGroupBy() {
105105
Assertions
106106
.assertEquals(
107107
"[countaggregator(input[0, java.lang.Long, true].longValue AS value, staticinvoke(class java.lang.Long, ObjectType(class java.lang.Long), valueOf, input[0, bigint, true], true, false, true), input[0, java.lang.Long, true].longValue) AS `count(_raw)`]",
108-
Arrays.toString(cs.getListOfExpr().toArray())
108+
Arrays.toString(cs.aggrExpressionsList().toArray())
109109
);
110-
Assertions.assertEquals("[_time, fieldTwo]", Arrays.toString(cs.getListOfGroupBy().toArray()));
110+
Assertions.assertEquals("[_time, fieldTwo]", Arrays.toString(cs.groupByList().toArray()));
111111
}
112112

113113
@Test
@@ -128,9 +128,9 @@ void testChartTranslation_multiGroupByNoComma() {
128128
Assertions
129129
.assertEquals(
130130
"[countaggregator(input[0, java.lang.Long, true].longValue AS value, staticinvoke(class java.lang.Long, ObjectType(class java.lang.Long), valueOf, input[0, bigint, true], true, false, true), input[0, java.lang.Long, true].longValue) AS `count(_raw)`]",
131-
Arrays.toString(cs.getListOfExpr().toArray())
131+
Arrays.toString(cs.aggrExpressionsList().toArray())
132132
);
133-
Assertions.assertEquals("[fieldTwo, _time]", Arrays.toString(cs.getListOfGroupBy().toArray()));
133+
Assertions.assertEquals("[fieldTwo, _time]", Arrays.toString(cs.groupByList().toArray()));
134134

135135
}
136136
}

0 commit comments

Comments
 (0)