Parquet: Use new logical type annotations #891
Conversation
|
@Fokko, you may be interested in this one. |
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Thanks @rdblue for tagging me in this one. Sorry for the late reply, still finding a new routine with the corona stuff. Hope you and your family are safe.
In this PR's we're using the LogicalTypeAnnotation Parquet 1.11.0 class, so I think we can also get rid of the constructor for Parquet 1.10 in: https://github.com/apache/incubator-iceberg/blob/master/parquet/src/main/java/org/apache/iceberg/parquet/ParquetWriter.java#L60-L64
Just to keep it nice and tidy :)
Cheers, Fokko
| } | ||
|
|
||
| @Override | ||
| public Optional<PrimitiveWriter<?>> visit(LogicalTypeAnnotation.JsonLogicalTypeAnnotation jsonLogicalType) { |
There was a problem hiding this comment.
In Parquet, LogicalTypeAnnotation.java, there is LogicalTypeAnnotation.IntLogicalTypeAnnotation. Do we need it here too?
There was a problem hiding this comment.
Methods that aren't implemented here use the superclass implementation, which returns Optional.empty. When that happens, we fall back to the old logic that uses just the primitive type.
| Optional<PrimitiveWriter<?>> writer = logicalType.accept(new LogicalTypeWriterVisitor(desc)); | ||
| if (writer.isPresent()) { | ||
| return writer.get(); | ||
| } |
There was a problem hiding this comment.
When writer.isPresent() == false, we falls back to the previous logic, right?
rdblue
force-pushed
the
use-parquet-logical-type-annotations
branch
from
d173d28 to
286e69a
Compare
|
This all looks good to me. +1 |
rdblue
force-pushed
the
use-parquet-logical-type-annotations
branch
from
7ff9cba to
7b8b638
Compare
|
Thanks for reviewing, everyone! |
…ck-port apache#965 from open source. To be reverted when apache#891 is merged
5 participants
Parquet 1.11.0 added logical type annotations that can be used to store Iceberg's
adjustToUTCvalue like Avro, so that Parquet files correctly encode whether a value is atimestampor atimestamptz.