drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [4/4] drill git commit: DRILL-4332: Makes vector comparison order stable in test framework
Date Tue, 08 Mar 2016 03:57:22 GMT
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 <laurent@dremio.com>
Authored: Tue Feb 23 14:14:37 2016 -0800
Committer: Jason Altekruse <altekrusejason@gmail.com>
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;
     }


Mime
View raw message