You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by sm...@apache.org on 2015/04/02 21:53:41 UTC

drill git commit: DRILL-2220: Complex reader unable to read FIXED_LEN_BYTE_ARRAY types in parquet file

Repository: drill
Updated Branches:
  refs/heads/master 168fa904b -> 30ad950e5


DRILL-2220: Complex reader unable to read FIXED_LEN_BYTE_ARRAY types in parquet file


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

Branch: refs/heads/master
Commit: 30ad950e577b8c087fe8cae33fe189f30193c078
Parents: 168fa90
Author: adeneche <ad...@gmail.com>
Authored: Thu Mar 5 12:27:53 2015 -0800
Committer: Steven Phillips <sp...@maprtech.com>
Committed: Thu Apr 2 11:22:41 2015 -0700

----------------------------------------------------------------------
 .../codegen/templates/AbstractFieldWriter.java  |   9 ++-
 .../src/main/codegen/templates/BaseWriter.java  |   4 ++
 .../src/main/codegen/templates/MapWriters.java  |  15 ++++-
 .../parquet2/DrillParquetGroupConverter.java    |  19 +++++-
 .../store/parquet2/TestDrillParquetReader.java  |  61 +++++++++++++++++++
 .../resources/parquet2/decimal28_38.parquet     | Bin 0 -> 708 bytes
 6 files changed, 104 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/30ad950e/exec/java-exec/src/main/codegen/templates/AbstractFieldWriter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/AbstractFieldWriter.java b/exec/java-exec/src/main/codegen/templates/AbstractFieldWriter.java
index 145130b..1b5dad1 100644
--- a/exec/java-exec/src/main/codegen/templates/AbstractFieldWriter.java
+++ b/exec/java-exec/src/main/codegen/templates/AbstractFieldWriter.java
@@ -26,6 +26,7 @@ package org.apache.drill.exec.vector.complex.impl;
 
 <#include "/@includes/vv_imports.ftl" />
 
+/* This class is generated using freemarker and the AbstractFieldWriter.java template */
 @SuppressWarnings("unused")
 abstract class AbstractFieldWriter extends AbstractBaseWriter implements FieldWriter{
   
@@ -82,7 +83,13 @@ abstract class AbstractFieldWriter extends AbstractBaseWriter implements FieldWr
   <#if lowerName == "int" ><#assign lowerName = "integer" /></#if>
   <#assign upperName = minor.class?upper_case />
   <#assign capName = minor.class?cap_first />
-  public ${capName}Writer ${lowerName}(String name){
+  <#if minor.class?starts_with("Decimal") >
+  public ${capName}Writer ${lowerName}(String name, int scale, int precision) {
+    fail("${capName}");
+    return null;
+  }
+  </#if>
+  public ${capName}Writer ${lowerName}(String name) {
     fail("${capName}");
     return null;
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/30ad950e/exec/java-exec/src/main/codegen/templates/BaseWriter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/BaseWriter.java b/exec/java-exec/src/main/codegen/templates/BaseWriter.java
index 979d4ac..2ce4c3c 100644
--- a/exec/java-exec/src/main/codegen/templates/BaseWriter.java
+++ b/exec/java-exec/src/main/codegen/templates/BaseWriter.java
@@ -26,6 +26,7 @@ package org.apache.drill.exec.vector.complex.writer;
 
 <#include "/@includes/vv_imports.ftl" />
 
+/* This class is generated using freemarker and the BaseWriter.java template */
 @SuppressWarnings("unused")
 public interface BaseWriter extends Positionable{
   FieldWriter getParent();
@@ -44,6 +45,9 @@ public interface BaseWriter extends Positionable{
     <#if lowerName == "int" ><#assign lowerName = "integer" /></#if>
     <#assign upperName = minor.class?upper_case />
     <#assign capName = minor.class?cap_first />
+    <#if minor.class?starts_with("Decimal") >
+    ${capName}Writer ${lowerName}(String name, int scale, int precision);
+    </#if>
     ${capName}Writer ${lowerName}(String name);
     </#list></#list>
     

http://git-wip-us.apache.org/repos/asf/drill/blob/30ad950e/exec/java-exec/src/main/codegen/templates/MapWriters.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/codegen/templates/MapWriters.java b/exec/java-exec/src/main/codegen/templates/MapWriters.java
index b8d5365..4dab990 100644
--- a/exec/java-exec/src/main/codegen/templates/MapWriters.java
+++ b/exec/java-exec/src/main/codegen/templates/MapWriters.java
@@ -34,6 +34,7 @@ package org.apache.drill.exec.vector.complex.impl;
 <#include "/@includes/vv_imports.ftl" />
 import java.util.Map;
 
+import org.apache.drill.common.types.TypeProtos.DataMode;
 import org.apache.drill.exec.expr.holders.RepeatedMapHolder;
 import org.apache.drill.exec.vector.AllocationHelper;
 import org.apache.drill.exec.vector.complex.reader.FieldReader;
@@ -164,9 +165,21 @@ public class ${mode}MapWriter extends AbstractFieldWriter{
   <#assign capName = minor.class?cap_first />
   <#assign vectName = capName />
   <#assign vectName = "Nullable${capName}" />
-  
+
+  <#if minor.class?starts_with("Decimal") >
+  public ${minor.class}Writer ${lowerName}(String name){
+    // returns existing writer
+    FieldWriter writer = fields.get(name);
+    assert writer != null;
+    return writer;
+  }
+
+  public ${minor.class}Writer ${lowerName}(String name, int scale, int precision){
+    final MajorType ${upperName}_TYPE = Types.withScaleAndPrecision(MinorType.${upperName}, DataMode.OPTIONAL, scale, precision);
+  <#else>
   private static final MajorType ${upperName}_TYPE = Types.optional(MinorType.${upperName});
   public ${minor.class}Writer ${lowerName}(String name){
+  </#if>
     FieldWriter writer = fields.get(name);
     if(writer == null){
       ${vectName}Vector vector = container.addOrGet(name, ${upperName}_TYPE, ${vectName}Vector.class);

http://git-wip-us.apache.org/repos/asf/drill/blob/30ad950e/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java
index 1ec24e0..389c1f6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java
@@ -69,6 +69,7 @@ import parquet.io.api.PrimitiveConverter;
 import parquet.schema.DecimalMetadata;
 import parquet.schema.GroupType;
 import parquet.schema.MessageType;
+import parquet.schema.OriginalType;
 import parquet.schema.PrimitiveType;
 import parquet.schema.Type;
 import parquet.schema.Type.Repetition;
@@ -216,13 +217,14 @@ public class DrillParquetGroupConverter extends GroupConverter {
             VarCharWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).varChar() : mapWriter.varChar(name);
             return new DrillVarCharConverter(writer, mutator.getManagedBuffer());
           }
+          //TODO not sure if BINARY/DECIMAL is actually supported
           case DECIMAL: {
             DecimalMetadata metadata = type.getDecimalMetadata();
             if (metadata.getPrecision() <= 28) {
-              Decimal28SparseWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).decimal28Sparse() : mapWriter.decimal28Sparse(name);
+              Decimal28SparseWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).decimal28Sparse() : mapWriter.decimal28Sparse(name, metadata.getScale(), metadata.getPrecision());
               return new DrillBinaryToDecimal28Converter(writer, metadata.getPrecision(), metadata.getScale(), mutator.getManagedBuffer());
             } else {
-              Decimal38SparseWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).decimal38Sparse() : mapWriter.decimal38Sparse(name);
+              Decimal38SparseWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).decimal38Sparse() : mapWriter.decimal38Sparse(name, metadata.getScale(), metadata.getPrecision());
               return new DrillBinaryToDecimal38Converter(writer, metadata.getPrecision(), metadata.getScale(), mutator.getManagedBuffer());
             }
           }
@@ -231,6 +233,19 @@ public class DrillParquetGroupConverter extends GroupConverter {
           }
         }
       }
+      case FIXED_LEN_BYTE_ARRAY:
+        if (type.getOriginalType() == OriginalType.DECIMAL) {
+          DecimalMetadata metadata = type.getDecimalMetadata();
+          if (metadata.getPrecision() <= 28) {
+            Decimal28SparseWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).decimal28Sparse() : mapWriter.decimal28Sparse(name, metadata.getScale(), metadata.getPrecision());
+            return new DrillBinaryToDecimal28Converter(writer, metadata.getPrecision(), metadata.getScale(), mutator.getManagedBuffer());
+          } else {
+            Decimal38SparseWriter writer = type.getRepetition() == Repetition.REPEATED ? mapWriter.list(name).decimal38Sparse() : mapWriter.decimal38Sparse(name, metadata.getScale(), metadata.getPrecision());
+            return new DrillBinaryToDecimal38Converter(writer, metadata.getPrecision(), metadata.getScale(), mutator.getManagedBuffer());
+          }
+        } else {
+          throw new UnsupportedOperationException("Unsupported type " + type.getOriginalType());
+        }
       default:
         throw new UnsupportedOperationException("Unsupported type: " + type.getPrimitiveTypeName());
     }

http://git-wip-us.apache.org/repos/asf/drill/blob/30ad950e/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet2/TestDrillParquetReader.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet2/TestDrillParquetReader.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet2/TestDrillParquetReader.java
new file mode 100644
index 0000000..782191f
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet2/TestDrillParquetReader.java
@@ -0,0 +1,61 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.parquet2;
+
+import org.apache.drill.BaseTestQuery;
+import org.junit.Test;
+
+import java.math.BigDecimal;
+
+public class TestDrillParquetReader extends BaseTestQuery {
+
+  private void testColumn(String columnName) throws Exception {
+    testNoResult("alter session set `store.parquet.use_new_reader` = true");
+
+    BigDecimal result = new BigDecimal("1.20000000");
+
+    testBuilder()
+      .ordered()
+      .sqlQuery("select %s from cp.`parquet2/decimal28_38.parquet`", columnName)
+      .baselineColumns(columnName)
+      .baselineValues(result)
+      .go();
+
+    testNoResult("alter session set `store.parquet.use_new_reader` = false");
+  }
+
+  @Test
+  public void testRequiredDecimal28() throws Exception {
+    testColumn("d28_req");
+  }
+
+  @Test
+  public void testRequiredDecimal38() throws Exception {
+    testColumn("d38_req");
+  }
+
+  @Test
+  public void testOptionalDecimal28() throws Exception {
+    testColumn("d28_opt");
+  }
+
+  @Test
+  public void testOptionalDecimal38() throws Exception {
+    testColumn("d38_opt");
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/30ad950e/exec/java-exec/src/test/resources/parquet2/decimal28_38.parquet
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/parquet2/decimal28_38.parquet b/exec/java-exec/src/test/resources/parquet2/decimal28_38.parquet
new file mode 100644
index 0000000..0dbcf97
Binary files /dev/null and b/exec/java-exec/src/test/resources/parquet2/decimal28_38.parquet differ