[SPARK-14015][SQL] Support TimestampType in vectorized parquet reader #11882
Conversation
|
cc @nongli |
|
Test build #53757 has finished for PR 11882 at commit
|
| case INT96: | ||
| if (column.dataType() == DataTypes.TimestampType) { | ||
| for (int i = rowId; i < rowId + num; ++i) { | ||
| Binary v = dictionary.decodeToBinary(dictionaryIds.getInt(i)); |
There was a problem hiding this comment.
hm -- maybe we can do something a lot cheaper? At the very least maybe we can remove the creation of the this Binary object, since we are turning it immediately into a Long.
There was a problem hiding this comment.
sure, that sounds good
There was a problem hiding this comment.
Added a TODO for converting the dictionary of binaries into long to make it cheaper
|
Test build #53759 has finished for PR 11882 at commit
|
| if (column.dataType() == DataTypes.LongType || | ||
| if (column.dataType() == DataTypes.LongType || column.dataType() == DataTypes.TimestampType || | ||
| DecimalType.is64BitDecimalType(column.dataType())) { | ||
| defColumn.readLongs( |
There was a problem hiding this comment.
hmm. How does this work? readLongs is expecting to read parquet int64 physical types. How is it able to read this other physical type?
There was a problem hiding this comment.
Do we have tests where this type is not dictionary encoded?
There was a problem hiding this comment.
Thanks, fixed. Also added tests in HadoopFsRelationTest that test both the dictionary encoded and non-encoded versions across all supported datatypes.
|
Test build #53829 has finished for PR 11882 at commit
|
|
|
||
| originalTypes[i] = t.getOriginalType(); | ||
|
|
||
| // TODO: Be extremely cautious in what is supported. Expand this. |
There was a problem hiding this comment.
I think we can remove this check now too.
There was a problem hiding this comment.
sure, sounds good.
|
LGTM |
|
Test build #53940 has finished for PR 11882 at commit
|
4 participants
What changes were proposed in this pull request?
This PR adds support for TimestampType in the vectorized parquet reader
How was this patch tested?
VectorizedColumnReaderinitially had a gating condition onprimitiveType.getPrimitiveTypeName() == PrimitiveType.PrimitiveTypeName.INT96)that made us fall back on parquet-mr for handling timestamps. This condition is now removed.ParquetHadoopFsRelationSuite(that tests for all supported hive types -- includingTimestampType) fails when the gating condition is removed ([WIP][SPARK-13994][SQL] Investigate types not supported by vectorized parquet reader #11808) and should now pass with this change. Similarly, theParquetHiveCompatibilitySuite.SPARK-10177 timestamptest that fails when the gating condition is removed, should now pass as well.HadoopFsRelationTestthat test both the dictionary encoded and non-encoded versions across all supported datatypes.