You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by parthchandra <gi...@git.apache.org> on 2015/09/21 18:45:20 UTC
[GitHub] drill pull request: Drill 2908: Int96 support
GitHub user parthchandra opened a pull request:
https://github.com/apache/drill/pull/162
Drill 2908: Int96 support
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/parthchandra/incubator-drill DRILL-2908
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/162.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #162
----
commit a9eb4314ff56497fd2f892d79ee930231261f1d5
Author: Jason Altekruse <al...@gmail.com>
Date: 2015-04-14T23:27:59Z
DRILL-2908:Enable reading the Int 96 type from parquet files.
column chunk metadata can be out of order from the column ordering in the schema, even though it exposes both as a list, making them seem like they should correspond, so we have to make our own map between the column names and indexes in the list.
Support for varbinary reading and int96 reading in the new reader.
Support the second version page header, the java library will only dictionary encode fix length byte arrays when the writer version is set to 2.0
Looks to be working in the vectorized reader, need a test case.
Fixed complex reader, was using the wrong field to figure out the length to read.
Conflicts:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableFixedByteAlignedReaders.java
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetToDrillTypeConverter.java
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetGroupConverter.java
UDF for reading impala timestamps from varbinary
Fix for reading fixed binary and int96 columns in the vectorized parquet reader.
Conflicts:
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/NullableFixedByteAlignedReaders.java
Fix for a bug reading fixed binary and int 96 data out of parquet when the data is plain encoded.
commit c02614ce1e2dc3d7782aa617493deda8f8a269dd
Author: Parth Chandra <pa...@apache.org>
Date: 2015-09-15T22:27:17Z
DRILL-2908: Fix Parquet for var length vectors where encoding changes across pages. Add unit tests. Add option to make parquet page size and disctionary page size configurable at session level.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 2908: Int96 support
Posted by adeneche <gi...@git.apache.org>.
Github user adeneche commented on a diff in the pull request:
https://github.com/apache/drill/pull/162#discussion_r40223624
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java ---
@@ -59,6 +63,50 @@ protected void readField(long recordsToReadInThisPass) {
}
}
+ static class DictionaryFixedBinaryReader extends FixedByteAlignedReader {
+
+ VarBinaryVector castedVector;
+
+ DictionaryFixedBinaryReader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
+ ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, VarBinaryVector v,
+ SchemaElement schemaElement) throws ExecutionSetupException {
+ super(parentReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, v, schemaElement);
+ castedVector = v;
+ }
+
+ // this method is called by its superclass during a read loop
+ @Override
+ protected void readField(long recordsToReadInThisPass) {
+
+ recordsReadInThisIteration = Math.min(pageReader.currentPageCount
+ - pageReader.valuesRead, recordsToReadInThisPass - valuesReadInCurrentPass);
+
+ if (usingDictionary) {
+ VarBinaryVector.Mutator mutator = castedVector.getMutator();
+ Binary currDictValToWrite = null;
+ for (int i = 0; i < recordsReadInThisIteration; i++){
+ currDictValToWrite = pageReader.dictionaryValueReader.readBytes();
+ mutator.setSafe(valuesReadInCurrentPass + i, currDictValToWrite.toByteBuffer(), 0,
+ currDictValToWrite.length());
+ }
+ // Set the write Index. The next page that gets read might be a page that does not use dictionary encoding
+ // and we will go into the else condition below. The readField method of the parent class requires the
+ // writer index to be set correctly.
+ int writerIndex = castedVector.getBuffer().writerIndex();
+ castedVector.getBuffer().setIndex(0, writerIndex + (int)readLength);
--- End diff --
I'm probably missing it, but I couldn't find where `readLength` is set if we don't call `super.readField()`.
Does it mean writeIndex is unchanged when reading from the dictionary ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 2908: Int96 support
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/162#discussion_r40498185
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java ---
@@ -59,6 +63,50 @@ protected void readField(long recordsToReadInThisPass) {
}
}
+ static class DictionaryFixedBinaryReader extends FixedByteAlignedReader {
+
+ VarBinaryVector castedVector;
+
+ DictionaryFixedBinaryReader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
+ ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, VarBinaryVector v,
+ SchemaElement schemaElement) throws ExecutionSetupException {
+ super(parentReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, v, schemaElement);
+ castedVector = v;
+ }
+
+ // this method is called by its superclass during a read loop
+ @Override
+ protected void readField(long recordsToReadInThisPass) {
+
+ recordsReadInThisIteration = Math.min(pageReader.currentPageCount
+ - pageReader.valuesRead, recordsToReadInThisPass - valuesReadInCurrentPass);
+
+ if (usingDictionary) {
+ VarBinaryVector.Mutator mutator = castedVector.getMutator();
+ Binary currDictValToWrite = null;
+ for (int i = 0; i < recordsReadInThisIteration; i++){
+ currDictValToWrite = pageReader.dictionaryValueReader.readBytes();
+ mutator.setSafe(valuesReadInCurrentPass + i, currDictValToWrite.toByteBuffer(), 0,
+ currDictValToWrite.length());
+ }
+ // Set the write Index. The next page that gets read might be a page that does not use dictionary encoding
+ // and we will go into the else condition below. The readField method of the parent class requires the
+ // writer index to be set correctly.
+ int writerIndex = castedVector.getBuffer().writerIndex();
+ castedVector.getBuffer().setIndex(0, writerIndex + (int)readLength);
--- End diff --
You're right. For DictionaryFixedBinaryReader, the readField code does not call the super.readField method and the readLength is set in the parent class' method. Updated the patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 2908: Int96 support
Posted by adeneche <gi...@git.apache.org>.
Github user adeneche commented on the pull request:
https://github.com/apache/drill/pull/162#issuecomment-143516073
+1, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 2908: Int96 support
Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:
https://github.com/apache/drill/pull/162#discussion_r40274099
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java ---
@@ -59,6 +63,50 @@ protected void readField(long recordsToReadInThisPass) {
}
}
+ static class DictionaryFixedBinaryReader extends FixedByteAlignedReader {
+
+ VarBinaryVector castedVector;
+
+ DictionaryFixedBinaryReader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
+ ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, VarBinaryVector v,
+ SchemaElement schemaElement) throws ExecutionSetupException {
+ super(parentReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, v, schemaElement);
+ castedVector = v;
+ }
+
+ // this method is called by its superclass during a read loop
+ @Override
+ protected void readField(long recordsToReadInThisPass) {
+
+ recordsReadInThisIteration = Math.min(pageReader.currentPageCount
+ - pageReader.valuesRead, recordsToReadInThisPass - valuesReadInCurrentPass);
+
+ if (usingDictionary) {
+ VarBinaryVector.Mutator mutator = castedVector.getMutator();
+ Binary currDictValToWrite = null;
+ for (int i = 0; i < recordsReadInThisIteration; i++){
+ currDictValToWrite = pageReader.dictionaryValueReader.readBytes();
+ mutator.setSafe(valuesReadInCurrentPass + i, currDictValToWrite.toByteBuffer(), 0,
+ currDictValToWrite.length());
+ }
+ // Set the write Index. The next page that gets read might be a page that does not use dictionary encoding
+ // and we will go into the else condition below. The readField method of the parent class requires the
+ // writer index to be set correctly.
+ int writerIndex = castedVector.getBuffer().writerIndex();
+ castedVector.getBuffer().setIndex(0, writerIndex + (int)readLength);
--- End diff --
It's set in the parent class NullableColumnReader::processPages()
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 2908: Int96 support
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/162
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: Drill 2908: Int96 support
Posted by adeneche <gi...@git.apache.org>.
Github user adeneche commented on a diff in the pull request:
https://github.com/apache/drill/pull/162#discussion_r40283917
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/ParquetFixedWidthDictionaryReaders.java ---
@@ -59,6 +63,50 @@ protected void readField(long recordsToReadInThisPass) {
}
}
+ static class DictionaryFixedBinaryReader extends FixedByteAlignedReader {
+
+ VarBinaryVector castedVector;
+
+ DictionaryFixedBinaryReader(ParquetRecordReader parentReader, int allocateSize, ColumnDescriptor descriptor,
+ ColumnChunkMetaData columnChunkMetaData, boolean fixedLength, VarBinaryVector v,
+ SchemaElement schemaElement) throws ExecutionSetupException {
+ super(parentReader, allocateSize, descriptor, columnChunkMetaData, fixedLength, v, schemaElement);
+ castedVector = v;
+ }
+
+ // this method is called by its superclass during a read loop
+ @Override
+ protected void readField(long recordsToReadInThisPass) {
+
+ recordsReadInThisIteration = Math.min(pageReader.currentPageCount
+ - pageReader.valuesRead, recordsToReadInThisPass - valuesReadInCurrentPass);
+
+ if (usingDictionary) {
+ VarBinaryVector.Mutator mutator = castedVector.getMutator();
+ Binary currDictValToWrite = null;
+ for (int i = 0; i < recordsReadInThisIteration; i++){
+ currDictValToWrite = pageReader.dictionaryValueReader.readBytes();
+ mutator.setSafe(valuesReadInCurrentPass + i, currDictValToWrite.toByteBuffer(), 0,
+ currDictValToWrite.length());
+ }
+ // Set the write Index. The next page that gets read might be a page that does not use dictionary encoding
+ // and we will go into the else condition below. The readField method of the parent class requires the
+ // writer index to be set correctly.
+ int writerIndex = castedVector.getBuffer().writerIndex();
+ castedVector.getBuffer().setIndex(0, writerIndex + (int)readLength);
--- End diff --
`NullableColumnReader` is not one of the parent classes of `ParquetFixedWidthDictionaryReaders `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---