Iceberg Connector ORC support#2042
Merged
electrum merged 7 commits intotrinodb:masterfrom Jan 19, 2020
Merged
Conversation
lxynov
force-pushed
the
icebergOrc
branch
3 times, most recently
from
b9db21f to
8a2f0ae
Compare
lxynov
force-pushed
the
icebergOrc
branch
2 times, most recently
from
c82ef7e to
774e172
Compare
electrum
reviewed
Dec 27, 2019
Member
electrum
left a comment
electrum
left a comment
There was a problem hiding this comment.
A few comments. Overall looks good.
presto-orc/src/main/java/io/prestosql/orc/metadata/OrcMetadataWriter.java
Outdated
Show resolved
Hide resolved
presto-orc/src/main/java/io/prestosql/orc/metadata/OrcType.java
Outdated
Show resolved
Hide resolved
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/IcebergConfig.java
Outdated
Show resolved
Hide resolved
Member
Author
There was a problem hiding this comment.
Actually, ICEBERG_WRITER_OPEN_ERROR and ICEBERG_UNSUPPORTED_FORMAT are used in IcebergFileWriterFactory; ICEBERG_FILESYSTEM_ERROR is used in IcebergPageSourceProvider
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/IcebergFileWriterFactory.java
Outdated
Show resolved
Hide resolved
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/IcebergSessionProperties.java
Outdated
Show resolved
Hide resolved
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/TypeConverter.java
Outdated
Show resolved
Hide resolved
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/TypeConverter.java
Outdated
Show resolved
Hide resolved
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/TypeConverter.java
Outdated
Show resolved
Hide resolved
presto-iceberg/src/main/java/io/prestosql/plugin/iceberg/TypeConverter.java
Outdated
Show resolved
Hide resolved
In this way the callers can construct ORC types themselves and include attributes in them.
Column metrics of data files might be uncollected, in which case nulls would be returned by methods in `DataFile`.
Member
Author
|
@electrum thanks a lot for the review! I've updated the PR |
Member
|
Thanks! |
zhenxiao
pushed a commit
to prestodb/presto
that referenced
this pull request
Jul 15, 2021
Cherry-pick of trinodb/trino#1067, trinodb/trino#2042, trinodb/trino#4055, trinodb/trino#1629, trinodb/trino#3483 Co-authored-by: Parth Brahmbhatt <pbrahmbhatt@netflix.com> Co-authored-by: David Phillips <david@acz.org> Co-authored-by: Xingyuan Lin <linxingyuan1102@gmail.com> Co-authored-by: Dain Sundstrom <dain@iq80.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is ready for review.
Some notes:
TestIcebergSmokepasses with the ORC format.TIMEandTIME WITH TIME ZONEare not supported. The current syntax is that an Iceberg table is also a Hive table (see HiveTableOperations). So types unsupported by Hive are not supported by Iceberg Connector neither.Handle data files with no column metricsis related because ORC files' column metrics collection is not supported yet. This commit ensures that partition tables don't break.Supersedes #1290
Umbrella issue: #1324
cc: @electrum @wagnermarkd @phd3