-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add hidden $path column in Iceberg #11661
Conversation
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Manish Malhotra.
|
45fbbdb
to
ad8d778
Compare
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Manish Malhotra.
|
ad8d778
to
cf6812d
Compare
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Manish Malhotra.
|
cf6812d
to
8f59b0a
Compare
@osscm please do squash the commits. |
b2ce830
to
86df754
Compare
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumns.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergColumnHandle.java
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumns.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumns.java
Outdated
Show resolved
Hide resolved
Thanks @alexjo2144 for the heads up. Here is the source code reference for reserved metadata columns: @alexjo2144 there are already a few reserved column ids. What should we do eventually for dealing with further metadata columns (e.g. : |
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergColumnHandle.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
yes @alexjo2144 @findinpath I took the reference from https://github.com/apache/iceberg/blob/827b6c86108eec7b6de25f61022c7f8b5dda481c/core/src/main/java/org/apache/iceberg/MetadataColumns.java#L34-L42 (thanks to @RussellSpitzer for the discussion) So, for the other hidden columns, we can go further |
In case we'll need extra columns, I think it would probably be a good idea to add them first to iceberg project to avoid eventual clashes with reserved columns which may appear in the next Iceberg releases. |
86df754
to
4bee72a
Compare
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Manish Malhotra.
|
4bee72a
to
b703273
Compare
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Manish Malhotra.
|
b57d7bb
to
71cd5d0
Compare
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Manish Malhotra.
|
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergColumnHandle.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumn.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumn.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumn.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
Outdated
Show resolved
Hide resolved
e7c802f
to
cd45fe2
Compare
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.
Reviewed docs content only. That part looks good to me ;-)
thanks @mosabua for quick review! |
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
Outdated
Show resolved
Hide resolved
903815a
to
6f773b3
Compare
Thanks a lot @findinpath, @alexjo2144 , @ebyhr, @mosabua for all the help and reviews! |
Thanks @findepi for all the help! Is it possible to merge this PR now? thanks |
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumn.java
Outdated
Show resolved
Hide resolved
plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadataColumn.java
Show resolved
Hide resolved
27f4b84
to
a75f17b
Compare
a75f17b
to
16efb54
Compare
Merged, thanks! |
thanks a lot! |
Description
improvement
With this change , it will be possible to select the file path of the data file from the table.
$path
will be a hidden column.Other connector's like Hive and DeltaLake are supporting it.
This is required when user wants to debug the data related issues, and can map the data file as well.
Related issues, pull requests, and links
Documentation
( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
( ) No release notes entries required.
(x) Release notes entries required with the following suggested text: