diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java index fc76039982a..36615d10e99 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java @@ -120,9 +120,6 @@ import org.junit.Rule; import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; -import javax.annotation.Nullable; - -import static org.junit.Assert.assertEquals; import java.io.IOException; import java.io.PrintStream; import java.util.Arrays; @@ -135,6 +132,10 @@ import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; +import javax.annotation.Nullable; + +import static org.junit.Assert.assertEquals; + /** * A base class for SQL query testing. It sets up query execution environment, provides useful helper methods, * and populates data using {@link CalciteTests#createMockWalker}. @@ -1057,9 +1058,14 @@ public class BaseCalciteQueryTest extends CalciteTestBase } } + /** + * Validates the results with slight loosening in case {@link NullHandling} is not sql compatible. + * + * In case {@link NullHandling#replaceWithDefault()} an expected results of null accepts + * both null and the default value for that column as actual result. + */ public void assertResultsValid(String message, List expected, QueryResults queryResults) { - List results = queryResults.results; int numRows = Math.min(results.size(), expected.size()); for (int row = 0; row < numRows; row++) { @@ -1071,8 +1077,8 @@ public class BaseCalciteQueryTest extends CalciteTestBase Object resultCell = resultRow[i]; Object expectedCell = expectedRow[i]; - if(expectedCell == null) { - if(resultCell == null) { + if (expectedCell == null) { + if (resultCell == null) { continue; } expectedCell = NullHandling.defaultValueForType(queryResults.signature.getColumnType(i).get().getType()); @@ -1082,9 +1088,7 @@ public class BaseCalciteQueryTest extends CalciteTestBase expectedCell, resultCell); } - } - } public void assertResultsEquals(String sql, List expectedResults, List results) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java index 464d10e93cb..cc9465adbf3 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java @@ -20,7 +20,6 @@ package org.apache.druid.sql.calcite; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.google.common.collect.ImmutableMap; @@ -50,7 +49,6 @@ import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Objects; -import java.util.function.Function; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -99,7 +97,6 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest { private WindowQueryTestInputClass input; private ObjectMapper queryJackson; - private Function jacksonToString; public TestCase(String filename) throws Exception { @@ -110,14 +107,6 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest queryJackson = queryFramework().queryJsonMapper(); input = queryJackson.convertValue(objectFromYaml, WindowQueryTestInputClass.class); - jacksonToString = value -> { - try { - return queryJackson.writeValueAsString(value); - } catch (JsonProcessingException e) { - throw new RE(e); - } - }; - } public TestType getType() @@ -131,7 +120,7 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest } @Override - public void verifyResults(QueryResults results) + public void verifyResults(QueryResults results) throws Exception { if (results.exception != null) { throw new RE(results.exception, "Failed to execute because of exception."); @@ -157,8 +146,8 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest // prepend different values so that we are guaranteed that it is // always different - String expected = "e " + jacksonToString.apply(expectedOperator); - String actual = "a " + jacksonToString.apply(actualOperator); + String expected = "e " + queryJackson.writeValueAsString(expectedOperator); + String actual = "a " + queryJackson.writeValueAsString(actualOperator); Assert.assertEquals("Operator Mismatch, index[" + i + "]", expected, actual); } @@ -170,7 +159,7 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest Assert.assertEquals(types[i], results.signature.getColumnType(i).get()); } - maybeDumpActualResults(jacksonToString, results.results); + 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 @@ -181,28 +170,32 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest // 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()); + 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()); } } } } } - assertResultsValid(filename, input.expectedResults, results); } + private void maybeDumpActualResults(List results) throws Exception + { + for (Object[] row : results) { + System.out.println(" - " + queryJackson.writeValueAsString(row)); + } + } } @Test @@ -232,16 +225,6 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest return (WindowOperatorQuery) query; } - private void maybeDumpActualResults( - Function toStrFn, List results - ) - { - if (DUMP_ACTUAL_RESULTS) { - for (Object[] result : results) { - System.out.println(" - " + toStrFn.apply(result)); - } - } - } public static class WindowQueryTestInputClass { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/QueryVerification.java b/sql/src/test/java/org/apache/druid/sql/calcite/QueryVerification.java index 919420b131d..ad96f35ba23 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/QueryVerification.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/QueryVerification.java @@ -28,7 +28,7 @@ public class QueryVerification public interface QueryResultsVerifier { - void verifyResults(QueryTestRunner.QueryResults results); + void verifyResults(QueryTestRunner.QueryResults results) throws Exception; } public static class QueryResultsVerifierFactory implements QueryTestRunner.QueryVerifyStepFactory @@ -47,7 +47,12 @@ public class QueryVerification { return () -> { for (QueryTestRunner.QueryResults queryResults : execStep.results()) { - verifier.verifyResults(queryResults); + try { + verifier.verifyResults(queryResults); + } catch (Exception e) + { + throw new RuntimeException("Exception during verification!", e); + } } }; }