drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jacq...@apache.org
Subject [1/6] drill git commit: DRILL-1821: Fix issue where schema changes for previously defined field.
Date Mon, 08 Dec 2014 15:55:34 GMT
Repository: drill
Updated Branches:
  refs/heads/0.7.0 cd74ccede -> 503a5b2a4
  refs/heads/master 54ce3d375 -> bd3e9138a


DRILL-1821: Fix issue where schema changes for previously defined field.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/d092ae72
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/d092ae72
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/d092ae72

Branch: refs/heads/master
Commit: d092ae72e6d8980a9f9f679e17799e4b0bb804ae
Parents: 54ce3d3
Author: Jacques Nadeau <jacques@apache.org>
Authored: Sun Dec 7 15:55:06 2014 -0800
Committer: Jacques Nadeau <jacques@apache.org>
Committed: Sun Dec 7 22:37:11 2014 -0800

----------------------------------------------------------------------
 .../drill/exec/physical/impl/OutputMutator.java | 36 +++++++++++++++++---
 .../drill/exec/physical/impl/ScanBatch.java     | 16 ++++-----
 .../drill/exec/record/MaterializedField.java    |  9 +----
 .../drill/exec/record/VectorContainer.java      | 10 +-----
 .../java/org/apache/drill/BaseTestQuery.java    |  3 ++
 .../test/java/org/apache/drill/TestBuilder.java |  5 ++-
 .../drill/exec/store/TestOutputMutator.java     |  4 ---
 .../store/parquet/ParquetRecordReaderTest.java  |  5 ---
 .../vector/complex/writer/TestJsonReader.java   | 27 ++++++++-------
 .../vector/complex/writer/expected.json         |  4 +++
 .../vector/complex/writer/schemaChange/f1.json  |  2 ++
 .../vector/complex/writer/schemaChange/f2.json  |  2 ++
 12 files changed, 68 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/OutputMutator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/OutputMutator.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/OutputMutator.java
index 32aae07..d3449ee 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/OutputMutator.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/OutputMutator.java
@@ -25,15 +25,41 @@ import org.apache.drill.exec.exception.SchemaChangeException;
 import org.apache.drill.exec.record.MaterializedField;
 import org.apache.drill.exec.vector.ValueVector;
 
+/**
+ * Interface that allows a record reader to modify the current schema.
+ *
+ * The output mutator interface abstracts ValueVector creation and maintenance away from
any particular RecordReader.
+ * This means, among other things, that a new RecordReader that shares the same column definitions
in a different order
+ * does not generate a Schema change event for downstream consumers.
+ */
 public interface OutputMutator {
-  public <T extends ValueVector> T addField(MaterializedField field, Class<T>
clazz) throws SchemaChangeException ;
+
+  /**
+   * Add a ValueVector for new (or existing) field.
+   *
+   * @param field
+   *          The specification of the newly desired vector.
+   * @param clazz
+   *          The expected ValueVector class. Also allows strongly typed use of this interface.
+   *
+   * @return The existing or new ValueVector associated with the provided field.
+   *
+   * @throws SchemaChangeException
+   *           If the addition of this field is incompatible with this OutputMutator's capabilities.
+   */
+  public <T extends ValueVector> T addField(MaterializedField field, Class<T>
clazz) throws SchemaChangeException;
+
   public void allocate(int recordCount);
+
+  /**
+   * Whether or not the fields added to the OutputMutator generated a new schema event.
+   * @return
+   */
   public boolean isNewSchema();
 
-  /* TODO: This interface is added to support information schema tables,
-   * FixedTables, the way they exist currently.
-   * One to many layers to rip out, address it as a separate JIRA.
+  /**
+   * Allows a scanner to request a set of managed block of memory.
+   * @return A DrillBuf that will be released at the end of the current query (and can be
resized as desired during use).
    */
-  public void addFields(List<ValueVector> vvList);
   public DrillBuf getManagedBuffer();
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
index 6e1f139..23833b6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java
@@ -283,7 +283,12 @@ public class ScanBatch implements RecordBatch {
           throw new SchemaChangeException(String.format("The class that was provided %s does
not correspond to the expected vector type of %s.", clazz.getSimpleName(), v.getClass().getSimpleName()));
         }
         container.add(v);
-        fieldVectorMap.put(field.key(), v);
+
+        ValueVector old = fieldVectorMap.put(field.key(), v);
+        if(old != null){
+          container.remove(old);
+          old.clear();
+        }
 
         // Adding new vectors to the container mark that the schema has changed
         schemaChange = true;
@@ -293,15 +298,6 @@ public class ScanBatch implements RecordBatch {
     }
 
     @Override
-    public void addFields(List<ValueVector> vvList) {
-      for (ValueVector v : vvList) {
-        fieldVectorMap.put(v.getField().key(), v);
-        container.add(v);
-      }
-      schemaChange = true;
-    }
-
-    @Override
     public void allocate(int recordCount) {
       for (ValueVector v : fieldVectorMap.values()) {
         AllocationHelper.allocate(v, recordCount, 50, 10);

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java
index 7f7dcfa..a9f3292 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java
@@ -268,7 +268,6 @@ public class MaterializedField {
       final int prime = 31;
       int result = 1;
       result = prime * result + ((path == null) ? 0 : path.hashCode());
-      result = prime * result + ((type == null) ? 0 : type.hashCode());
       return result;
     }
 
@@ -291,13 +290,7 @@ public class MaterializedField {
       } else if (!path.equals(other.path)) {
         return false;
       }
-      if (type == null) {
-        if (other.type != null) {
-          return false;
-        }
-      } else if (!type.equals(other.type)) {
-        return false;
-      }
+
       return true;
     }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
index 24b8a4b..d50760a 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/VectorContainer.java
@@ -106,15 +106,7 @@ public class VectorContainer extends AbstractMapVector implements Iterable<Vecto
 
   public <T extends ValueVector> T addOrGet(String name, MajorType type, Class<T>
clazz) {
     MaterializedField field = MaterializedField.create(name, type);
-    ValueVector v = TypeHelper.getNewVector(field, this.oContext.getAllocator());
-
-    add(v);
-
-    if (clazz.isAssignableFrom(v.getClass())) {
-      return (T) v;
-    } else {
-      throw new IllegalStateException(String.format("Vector requested [%s] was different
than type stored [%s].  Drill doesn't yet support hetergenous types.", clazz.getSimpleName(),
v.getClass().getSimpleName()));
-    }
+    return addOrGet(field);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
index 0de3938..782fbe7 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
@@ -30,9 +30,11 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.regex.Pattern;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
+
 import org.apache.drill.common.config.DrillConfig;
 import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.common.types.Types;
@@ -206,6 +208,7 @@ public class BaseTestQuery extends ExecTest{
   }
 
   public static void test(String query) throws Exception{
+    query = query.replaceAll(Pattern.quote("${WORKING_PATH}"), TestTools.getWorkingPath());
     String[] queries = query.split(";");
     for (String q : queries) {
       if (q.trim().isEmpty()) {

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java b/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
index e9f7f55..63de112 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java
@@ -18,6 +18,7 @@
 package org.apache.drill;
 
 import com.google.common.base.Joiner;
+
 import org.antlr.runtime.ANTLRStringStream;
 import org.antlr.runtime.CommonTokenStream;
 import org.antlr.runtime.RecognitionException;
@@ -26,6 +27,7 @@ import org.apache.drill.common.expression.parser.ExprLexer;
 import org.apache.drill.common.expression.parser.ExprParser;
 import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.common.types.Types;
+import org.apache.drill.common.util.TestTools;
 import org.apache.drill.exec.memory.BufferAllocator;
 import org.apache.drill.exec.proto.UserBitShared;
 
@@ -34,6 +36,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import static org.junit.Assert.assertEquals;
 
@@ -117,7 +120,7 @@ public class TestBuilder {
   }
 
   public TestBuilder sqlQuery(String query) {
-    this.query = query;
+    this.query = query.replaceAll(Pattern.quote("${WORKING_PATH}"), TestTools.getWorkingPath());
     this.queryType = UserBitShared.QueryType.SQL;
     return this;
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/java/org/apache/drill/exec/store/TestOutputMutator.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/TestOutputMutator.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/TestOutputMutator.java
index 15bae6e..0509b7b 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/TestOutputMutator.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/TestOutputMutator.java
@@ -59,10 +59,6 @@ public class TestOutputMutator implements OutputMutator, Iterable<VectorWrapper<
     fieldVectorMap.put(vector.getField(), vector);
   }
 
-  public void addFields(List<ValueVector> v) {
-    return;
-  }
-
   public Iterator<VectorWrapper<?>> iterator() {
     return container.iterator();
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetRecordReaderTest.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetRecordReaderTest.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetRecordReaderTest.java
index da8abdd..219e66f 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetRecordReaderTest.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetRecordReaderTest.java
@@ -332,11 +332,6 @@ public class ParquetRecordReaderTest extends BaseTestQuery{
     }
 
     @Override
-    public void addFields(List<ValueVector> vv) {
-      return;
-    }
-
-    @Override
     public <T extends ValueVector> T addField(MaterializedField field, Class<T>
clazz) throws SchemaChangeException {
       return null;
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java
index 29d7b3e..4a460a4 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java
@@ -26,19 +26,14 @@ import java.util.List;
 import org.apache.drill.BaseTestQuery;
 import org.apache.drill.common.expression.SchemaPath;
 import org.apache.drill.common.util.FileUtils;
-import org.apache.drill.common.util.TestTools;
 import org.apache.drill.exec.exception.SchemaChangeException;
-import org.apache.drill.exec.memory.BufferAllocator;
-import org.apache.drill.exec.memory.TopLevelAllocator;
 import org.apache.drill.exec.proto.UserBitShared;
 import org.apache.drill.exec.record.RecordBatchLoader;
 import org.apache.drill.exec.record.VectorWrapper;
 import org.apache.drill.exec.rpc.user.QueryResultBatch;
 import org.apache.drill.exec.vector.IntVector;
-import org.apache.drill.exec.vector.NullableIntVector;
 import org.apache.drill.exec.vector.RepeatedBigIntVector;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import com.google.common.base.Charsets;
@@ -47,17 +42,23 @@ import com.google.common.io.Files;
 public class TestJsonReader extends BaseTestQuery {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestJsonReader.class);
 
-  private static BufferAllocator allocator;
   private static final boolean VERBOSE_DEBUG = false;
 
-  @BeforeClass
-  public static void setupAllocator() {
-    allocator = new TopLevelAllocator();
+
+  @Test
+  public void schemaChange() throws Exception {
+    test("select b from dfs.`${WORKING_PATH}/src/test/resources/vector/complex/writer/schemaChange/`");
   }
 
-  @AfterClass
-  public static void destroyAllocator() {
-    allocator.close();
+  @Test
+  @Ignore("DRILL-1824")
+  public void schemaChangeValidate() throws Exception {
+    testBuilder() //
+      .sqlQuery("select b from dfs.`${WORKING_PATH}/src/test/resources/vector/complex/writer/schemaChange/`")
//
+      .unOrdered() //
+      .jsonBaselineFile("/vector/complex/writer/expected.json") //
+      .build()
+      .run();
   }
 
   public void runTestsOnFile(String filename, UserBitShared.QueryType queryType, String[]
queries, long[] rowCounts) throws Exception {

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/resources/vector/complex/writer/expected.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/vector/complex/writer/expected.json b/exec/java-exec/src/test/resources/vector/complex/writer/expected.json
new file mode 100644
index 0000000..1a53036
--- /dev/null
+++ b/exec/java-exec/src/test/resources/vector/complex/writer/expected.json
@@ -0,0 +1,4 @@
+{"b": null}
+{"b": null}
+{"b": null}
+{"b": {"x":1, "y":2}}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f1.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f1.json
b/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f1.json
new file mode 100644
index 0000000..d03af0f
--- /dev/null
+++ b/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f1.json
@@ -0,0 +1,2 @@
+{"a": "foo","b": null}
+{"a": "bar","b": null}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/d092ae72/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f2.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f2.json
b/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f2.json
new file mode 100644
index 0000000..f1f0296
--- /dev/null
+++ b/exec/java-exec/src/test/resources/vector/complex/writer/schemaChange/f2.json
@@ -0,0 +1,2 @@
+{"a": "foo2","b": null}
+{"a": "bar2","b": {"x":1, "y":2}}


Mime
View raw message