kgyrtkirk commented on code in PR #15086:
URL: https://github.com/apache/druid/pull/15086#discussion_r1350149211
##########
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));
}
- };
+ }
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void windowQueryTest() throws Exception
+ {
+ TestCase testCase = new TestCase(filename);
- assumeThat(input.type, Matchers.not(TestType.failingTest));
+ assumeThat(testCase.getType(), Matchers.not(TestType.failingTest));
- if (input.type == TestType.operatorValidation) {
+ if (testCase.getType() == TestType.operatorValidation) {
testBuilder()
.skipVectorize(true)
- .sql(input.sql)
+ .sql(testCase.getSql())
.queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS,
true,
QueryContexts.ENABLE_DEBUG, true))
- .addCustomVerification(QueryVerification.ofResults(results -> {
- if (results.exception != null) {
- throw new RE(results.exception, "Failed to execute because of
exception.");
- }
-
- Assert.assertEquals(1, results.recordedQueries.size());
- // 2 tests are failing at this moment on this check
Review Comment:
I think this comment was originally misplaced when it was added - I believe
notices like this should be next to the testcase; or somewhere around the
`ignore` ; so that it gets removed when it gets enabled back.
`operatorValidation` is kinda like the "enable" mode for these tests - I
see only one of the old tests
(wikipediaAggregationsMultipleOrderingDesc.sqlTest) to be marked as
`failingTest` ; and that one is reporting an issue because there is a `DESC` in
the query which is not supported..actually that test was never enabled; ever
since it was added
```
dev@windowing-fixes:~/druid$ grep -r failing
sql/src/test/resources/calcite/tests/window/*
sql/src/test/resources/calcite/tests/window/no_grouping.sqlTest:type:
"failingTest"
sql/src/test/resources/calcite/tests/window/wikipediaAggregationsMultipleOrderingDesc.sqlTest:type:
"failingTest"
```
--
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]
