kgyrtkirk commented on code in PR #15086:
URL: https://github.com/apache/druid/pull/15086#discussion_r1350155258
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java:
##########
@@ -91,119 +88,117 @@ public static Object parametersForWindowQueryTest()
throws Exception
private final String filename;
- public CalciteWindowQueryTest(
- String filename
- )
+ public CalciteWindowQueryTest(String filename)
{
this.filename = filename;
}
- @Test
- @SuppressWarnings("unchecked")
- public void windowQueryTest() throws IOException
+ class TestCase implements QueryResultsVerifier
{
- assumeTrue("These tests are only run in sqlCompatible mode!",
NullHandling.sqlCompatible());
- final Function<String, String> stringManipulator;
- if (NullHandling.sqlCompatible()) {
- stringManipulator = s -> "".equals(s) ? null : s;
- } else {
- stringManipulator = Function.identity();
+ private WindowQueryTestInputClass input;
+ private ObjectMapper queryJackson;
+
+ public TestCase(String filename) throws Exception
+ {
+ final URL systemResource =
ClassLoader.getSystemResource("calcite/tests/window/" + filename);
+
+ final Object objectFromYaml = YAML_JACKSON.readValue(systemResource,
Object.class);
+
+ queryJackson = queryFramework().queryJsonMapper();
+ input = queryJackson.convertValue(objectFromYaml,
WindowQueryTestInputClass.class);
+
}
- final URL systemResource =
ClassLoader.getSystemResource("calcite/tests/window/" + filename);
+ public TestType getType()
+ {
+ return input.type;
+ }
- final Object objectFromYaml = YAML_JACKSON.readValue(systemResource,
Object.class);
+ public String getSql()
+ {
+ return input.sql;
+ }
- final ObjectMapper queryJackson = queryFramework().queryJsonMapper();
- final WindowQueryTestInputClass input =
queryJackson.convertValue(objectFromYaml, WindowQueryTestInputClass.class);
+ @Override
+ public void verifyResults(QueryResults results) throws Exception
+ {
+ if (results.exception != null) {
+ throw new RE(results.exception, "Failed to execute because of
exception.");
+ }
+ Assert.assertEquals(1, results.recordedQueries.size());
+
+ final WindowOperatorQuery query =
getWindowOperatorQuery(results.recordedQueries);
+ for (int i = 0; i < input.expectedOperators.size(); ++i) {
+ final OperatorFactory expectedOperator =
input.expectedOperators.get(i);
+ final OperatorFactory actualOperator = query.getOperators().get(i);
+ if (!expectedOperator.validateEquivalent(actualOperator)) {
+ assertEquals("Operator Mismatch, index[" + i + "]",
+ queryJackson.writeValueAsString(expectedOperator),
+ queryJackson.writeValueAsString(actualOperator));
+ fail("validateEquivalent failed; but textual comparision of
operators didn't reported the mismatch!");
+ }
+ }
+ final RowSignature outputSignature = query.getRowSignature();
+ ColumnType[] types = new ColumnType[outputSignature.size()];
+ for (int i = 0; i < outputSignature.size(); ++i) {
+ types[i] = outputSignature.getColumnType(i).get();
+ Assert.assertEquals(types[i],
results.signature.getColumnType(i).get());
+ }
- Function<Object, String> jacksonToString = value -> {
- try {
- return queryJackson.writeValueAsString(value);
+ maybeDumpActualResults(results.results);
+ for (Object[] result : input.expectedResults) {
+ for (int i = 0; i < result.length; i++) {
+ // Jackson deserializes numbers as the minimum size required to
+ // store the value. This means that
+ // Longs can become Integer objects and then they fail equality
+ // checks. We read the expected
+ // results using Jackson, so, we coerce the expected results to the
+ // type expected.
+ if (result[i] != null) {
+ if (result[i] instanceof Number) {
+ switch (types[i].getType()) {
+ case LONG:
+ result[i] = ((Number) result[i]).longValue();
+ break;
+ case DOUBLE:
+ result[i] = ((Number) result[i]).doubleValue();
+ break;
+ case FLOAT:
+ result[i] = ((Number) result[i]).floatValue();
+ break;
+ default:
+ throw new ISE("result[%s] was type[%s]!? Expected it to be
numerical", i, types[i].getType());
+ }
+ }
+ }
+ }
}
- catch (JsonProcessingException e) {
- throw new RE(e);
+ assertResultsValid(filename, input.expectedResults, results);
+ }
+
+ private void maybeDumpActualResults(List<Object[]> results) throws
Exception
+ {
+ for (Object[] row : results) {
+ System.out.println(" - " + queryJackson.writeValueAsString(row));
Review Comment:
I'm afraid that would partially defeat the original purpose of these line ;
as in that case it would prefix the results with the timestamp/etc.
so instead
```
- [1.0,21.0]
- [2.0,21.0]
- [3.0,21.0]
- [4.0,21.0]
- [5.0,21.0]
- [6.0,21.0]
```
we would get:
```
2023-10-09T10:57:46,840 INFO [main]
org.apache.druid.sql.calcite.BaseCalciteQueryTest - - [1.0,21.0]
2023-10-09T10:57:46,841 INFO [main]
org.apache.druid.sql.calcite.BaseCalciteQueryTest - - [2.0,21.0]
2023-10-09T10:57:46,841 INFO [main]
org.apache.druid.sql.calcite.BaseCalciteQueryTest - - [3.0,21.0]
2023-10-09T10:57:46,841 INFO [main]
org.apache.druid.sql.calcite.BaseCalciteQueryTest - - [4.0,21.0]
2023-10-09T10:57:46,841 INFO [main]
org.apache.druid.sql.calcite.BaseCalciteQueryTest - - [5.0,21.0]
2023-10-09T10:57:46,841 INFO [main]
org.apache.druid.sql.calcite.BaseCalciteQueryTest - - [6.0,21.0]
```
which would need some preprocessing to get it into the `yaml` file.
I think an alternate way of fixing these kind of issues could be to create a
full new testcase file file with the expectations and everything....not sure
how easy/hard would that be
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
