DRILL-4332: Makes vector comparison order stable in test framework In the test framework, a vector is a map of <String, Object>. When comparing actual values with baseline, the comparison is made column by column, but a HashMap key ordering is not guaranteed, and the ordering actually changed between Java7 and Java8 in Oracle/OpenJDK.
Replacing HashMap with TreeMap which has a guaranteed ordering by design. Small update by jason during merge, fixed test failure on JDK 7 due to map key ordering, just replaced two more uses of HashMap with TreeMap. Closes #389 Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/447b093c Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/447b093c Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/447b093c Branch: refs/heads/master Commit: 447b093cd2b05bfeae001844a7e3573935e84389 Parents: 80316f3 Author: Laurent Goujon <[email protected]> Authored: Tue Feb 23 14:14:37 2016 -0800 Committer: Jason Altekruse <[email protected]> Committed: Mon Mar 7 19:49:37 2016 -0800 ---------------------------------------------------------------------- .../java/org/apache/drill/DrillTestWrapper.java | 59 ++++++++++---------- .../org/apache/drill/TestFrameworkTest.java | 2 +- 2 files changed, 30 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/447b093c/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java b/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java index be017dc..41d3ff6 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java +++ b/exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java @@ -29,6 +29,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; import org.apache.drill.common.expression.SchemaPath; import org.apache.drill.common.types.TypeProtos; @@ -157,7 +158,6 @@ public class DrillTestWrapper { } private void compareMergedVectors(Map<String, List<Object>> expectedRecords, Map<String, List<Object>> actualRecords) throws Exception { - for (String s : actualRecords.keySet()) { assertNotNull("Unexpected extra column " + s + " returned by query.", expectedRecords.get(s)); assertEquals("Incorrect number of rows returned by query.", expectedRecords.get(s).size(), actualRecords.get(s).size()); @@ -209,7 +209,7 @@ public class DrillTestWrapper { private Map<String, HyperVectorValueIterator> addToHyperVectorMap(List<QueryDataBatch> records, RecordBatchLoader loader, BatchSchema schema) throws SchemaChangeException, UnsupportedEncodingException { // TODO - this does not handle schema changes - Map<String, HyperVectorValueIterator> combinedVectors = new HashMap<>(); + Map<String, HyperVectorValueIterator> combinedVectors = new TreeMap<>(); long totalRecords = 0; QueryDataBatch batch; @@ -255,7 +255,7 @@ public class DrillTestWrapper { private Map<String, List<Object>> addToCombinedVectorResults(List<QueryDataBatch> records, RecordBatchLoader loader, BatchSchema schema) throws SchemaChangeException, UnsupportedEncodingException { // TODO - this does not handle schema changes - Map<String, List<Object>> combinedVectors = new HashMap<>(); + Map<String, List<Object>> combinedVectors = new TreeMap<>(); long totalRecords = 0; QueryDataBatch batch; @@ -398,38 +398,37 @@ public class DrillTestWrapper { Map<String, List<Object>> expectedSuperVectors; try { - BaseTestQuery.test(testOptionSettingQueries); - actual = BaseTestQuery.testRunAndReturn(queryType, query); + BaseTestQuery.test(testOptionSettingQueries); + actual = BaseTestQuery.testRunAndReturn(queryType, query); - checkNumBatches(actual); + checkNumBatches(actual); - // To avoid extra work for test writers, types can optionally be inferred from the test query - addTypeInfoIfMissing(actual.get(0), testBuilder); + // To avoid extra work for test writers, types can optionally be inferred from the test query + addTypeInfoIfMissing(actual.get(0), testBuilder); - actualSuperVectors = addToCombinedVectorResults(actual, loader, schema); + actualSuperVectors = addToCombinedVectorResults(actual, loader, schema); - // If baseline data was not provided to the test builder directly, we must run a query for the baseline, this includes - // the cases where the baseline is stored in a file. - if (baselineRecords == null) { - BaseTestQuery.test(baselineOptionSettingQueries); - expected = BaseTestQuery.testRunAndReturn(baselineQueryType, testBuilder.getValidationQuery()); - expectedSuperVectors = addToCombinedVectorResults(expected, loader, schema); - } else { - // data is built in the TestBuilder in a row major format as it is provided by the user - // translate it here to vectorized, the representation expected by the ordered comparison - expectedSuperVectors = new HashMap<>(); - expected = new ArrayList<>(); - for (String s : baselineRecords.get(0).keySet()) { - expectedSuperVectors.put(s, new ArrayList<>()); - } - for (Map<String, Object> m : baselineRecords) { - for (String s : m.keySet()) { - expectedSuperVectors.get(s).add(m.get(s)); + // If baseline data was not provided to the test builder directly, we must run a query for the baseline, this includes + // the cases where the baseline is stored in a file. + if (baselineRecords == null) { + BaseTestQuery.test(baselineOptionSettingQueries); + expected = BaseTestQuery.testRunAndReturn(baselineQueryType, testBuilder.getValidationQuery()); + expectedSuperVectors = addToCombinedVectorResults(expected, loader, schema); + } else { + // data is built in the TestBuilder in a row major format as it is provided by the user + // translate it here to vectorized, the representation expected by the ordered comparison + expectedSuperVectors = new TreeMap<>(); + expected = new ArrayList<>(); + for (String s : baselineRecords.get(0).keySet()) { + expectedSuperVectors.put(s, new ArrayList<>()); + } + for (Map<String, Object> m : baselineRecords) { + for (String s : m.keySet()) { + expectedSuperVectors.get(s).add(m.get(s)); + } } } - } - - compareMergedVectors(expectedSuperVectors, actualSuperVectors); + compareMergedVectors(expectedSuperVectors, actualSuperVectors); } catch (Exception e) { throw new Exception(e.getMessage() + "\nFor query: " + query , e); } finally { @@ -510,7 +509,7 @@ public class DrillTestWrapper { logger.debug("reading batch with " + loader.getRecordCount() + " rows, total read so far " + totalRecords); totalRecords += loader.getRecordCount(); for (int j = 0; j < loader.getRecordCount(); j++) { - HashMap<String, Object> record = new HashMap<>(); + Map<String, Object> record = new TreeMap<>(); for (VectorWrapper<?> w : loader) { Object obj = w.getValueVector().getAccessor().getObject(j); if (obj != null) { http://git-wip-us.apache.org/repos/asf/drill/blob/447b093c/exec/java-exec/src/test/java/org/apache/drill/TestFrameworkTest.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestFrameworkTest.java b/exec/java-exec/src/test/java/org/apache/drill/TestFrameworkTest.java index 09e4d9a..a302b71 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/TestFrameworkTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/TestFrameworkTest.java @@ -321,7 +321,7 @@ public class TestFrameworkTest extends BaseTestQuery{ .build().run(); } catch (Exception ex) { assertThat(ex.getMessage(), CoreMatchers.containsString( - "at position 0 column '`first_name`' mismatched values, expected: Jewel(String) but received Peggy(String)")); + "at position 0 column '`employee_id`' mismatched values, expected: 12(String) but received 16(String)")); // this indicates successful completion of the test return; }
