-
Notifications
You must be signed in to change notification settings - Fork 29.2k
[SPARK-10364][SQL] Support Parquet logical type TIMESTAMP_MILLIS #15332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
cb61b53
00d2b38
89c5eb5
8c1416c
d7720ab
8f83f89
796b6b1
93a77d1
e2d0182
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| import org.apache.parquet.io.api.Binary; | ||
| import org.apache.parquet.schema.PrimitiveType; | ||
|
|
||
| import org.apache.spark.sql.catalyst.util.DateTimeUtils; | ||
| import org.apache.spark.sql.execution.vectorized.ColumnVector; | ||
| import org.apache.spark.sql.types.DataTypes; | ||
| import org.apache.spark.sql.types.DecimalType; | ||
|
|
@@ -155,9 +156,13 @@ void readBatch(int total, ColumnVector column) throws IOException { | |
| // Read and decode dictionary ids. | ||
| defColumn.readIntegers( | ||
| num, dictionaryIds, column, rowId, maxDefLevel, (VectorizedValuesReader) dataColumn); | ||
|
|
||
| // Timestamp values encoded as INT64 can't be lazily decoded as we need to post process | ||
| // the values to add microseconds precision. | ||
| if (column.hasDictionary() || (rowId == 0 && | ||
| (descriptor.getType() == PrimitiveType.PrimitiveTypeName.INT32 || | ||
| descriptor.getType() == PrimitiveType.PrimitiveTypeName.INT64 || | ||
| (descriptor.getType() == PrimitiveType.PrimitiveTypeName.INT64 && | ||
| column.dataType() != DataTypes.TimestampType) || | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this check?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to convert the time back to micros and in case of lazy decoding, we don't get that chance ? |
||
| descriptor.getType() == PrimitiveType.PrimitiveTypeName.FLOAT || | ||
| descriptor.getType() == PrimitiveType.PrimitiveTypeName.DOUBLE || | ||
| descriptor.getType() == PrimitiveType.PrimitiveTypeName.BINARY))) { | ||
|
|
@@ -250,7 +255,15 @@ private void decodeDictionaryIds(int rowId, int num, ColumnVector column, | |
| column.putLong(i, dictionary.decodeToLong(dictionaryIds.getDictId(i))); | ||
| } | ||
| } | ||
| } else { | ||
| } else if (column.dataType() == DataTypes.TimestampType) { | ||
| for (int i = rowId; i < rowId + num; ++i) { | ||
| if (!column.isNullAt(i)) { | ||
| column.putLong(i, | ||
| DateTimeUtils.fromMillis(dictionary.decodeToLong(dictionaryIds.getDictId(i)))); | ||
| } | ||
| } | ||
| } | ||
| else { | ||
| throw new UnsupportedOperationException("Unimplemented type: " + column.dataType()); | ||
| } | ||
| break; | ||
|
|
@@ -362,7 +375,15 @@ private void readLongBatch(int rowId, int num, ColumnVector column) throws IOExc | |
| if (column.dataType() == DataTypes.LongType || | ||
| DecimalType.is64BitDecimalType(column.dataType())) { | ||
| defColumn.readLongs( | ||
| num, column, rowId, maxDefLevel, (VectorizedValuesReader) dataColumn); | ||
| num, column, rowId, maxDefLevel, (VectorizedValuesReader) dataColumn); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For vectorized reader, I think we should also add
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dilipbiswal Per our offline discussion, I think you should add
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tested the following test case: It will cause an exception:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @viirya Thanks Simon. Very good catch !! I have made the changes.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why this change in indentation? if anything, looks like it should be indented less than the original.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @squito Thank you for reviewing. I have fixed the indentation. |
||
| } else if (column.dataType() == DataTypes.TimestampType) { | ||
| for (int i = 0; i < num; i++) { | ||
| if (defColumn.readInteger() == maxDefLevel) { | ||
| column.putLong(rowId + i, DateTimeUtils.fromMillis(dataColumn.readLong())); | ||
| } else { | ||
| column.putNull(rowId + i); | ||
| } | ||
| } | ||
| } else { | ||
| throw new UnsupportedOperationException("Unsupported conversion to: " + column.dataType()); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: epoch