-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Hive: Push Iceberg table property values to HMS table properties #2123
Conversation
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
Outdated
Show resolved
Hide resolved
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java
Outdated
Show resolved
Hide resolved
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java
Outdated
Show resolved
Hide resolved
Build failure is due to the flaky Spark tests:
|
hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveMetastore.java
Show resolved
Hide resolved
25e457a
to
6d630dc
Compare
What do you think about changing the title @marton-bod? I think the title below describes the change better:
Thanks, |
Overall I agree, I think that describes the crux of the change better. I went with the original title because we also relax the initial set of properties we push down from HMS to Iceberg in this patch. However, until we have a pre/postAlter hook on the HiveMetaHook, this is mostly a one-way street indeed. |
9eb76fb
to
e51d971
Compare
Thanks @marton-bod for the change! |
Iceberg table properties are the canonical source of truth HMS table properties should be maintained as much as possible to be in sync with the Iceberg table, but it can only happen on a best effort basis This PR makes the following changes: Ensures that all Iceberg table properties are propagated to the HMS table during HiveTableOperations commit All HMS table properties are pushed down to Iceberg as well during table creation (except for metadata location and spec props) Refactors the various property check assertions scattered throughout various test cases into a single property-focused unit test case What is left out and should be done in the future: Push property changes occurring via Hive DDL (ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because the HiveMetaHook interface only contains a preAlterTable method, but no commitAlterTable method. We'll need to extend this interface and include the change in an upcoming Hive upstream release. Author: Marton Bod <marton.bod@gmail.com> PR: apache/iceberg#2123 Backport Reason: To accomdate(I) for fix apache/iceberg#2328
Iceberg table properties are the canonical source of truth HMS table properties should be maintained as much as possible to be in sync with the Iceberg table, but it can only happen on a best effort basis This PR makes the following changes: Ensures that all Iceberg table properties are propagated to the HMS table during HiveTableOperations commit All HMS table properties are pushed down to Iceberg as well during table creation (except for metadata location and spec props) Refactors the various property check assertions scattered throughout various test cases into a single property-focused unit test case What is left out and should be done in the future: Push property changes occurring via Hive DDL (ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because the HiveMetaHook interface only contains a preAlterTable method, but no commitAlterTable method. We'll need to extend this interface and include the change in an upcoming Hive upstream release. Author: Marton Bod <marton.bod@gmail.com> PR: apache/iceberg#2123 Backport Reason: To accomdate(I) for fix apache/iceberg#2328
Iceberg table properties are the canonical source of truth HMS table properties should be maintained as much as possible to be in sync with the Iceberg table, but it can only happen on a best effort basis This PR makes the following changes: Ensures that all Iceberg table properties are propagated to the HMS table during HiveTableOperations commit All HMS table properties are pushed down to Iceberg as well during table creation (except for metadata location and spec props) Refactors the various property check assertions scattered throughout various test cases into a single property-focused unit test case What is left out and should be done in the future: Push property changes occurring via Hive DDL (ALTER TABLE SET TBLPROPERTIES) down to Iceberg as well. Currently this can't be done reliably because the HiveMetaHook interface only contains a preAlterTable method, but no commitAlterTable method. We'll need to extend this interface and include the change in an upcoming Hive upstream release. Author: Marton Bod <marton.bod@gmail.com> PR: apache/iceberg#2123 Backport Reason: To accomdate(I) for fix apache/iceberg#2328
As agreed with the community:
This PR makes the following changes:
HiveTableOperations
commitWhat is left out and should be done in the future:
ALTER TABLE SET TBLPROPERTIES
) down to Iceberg as well. Currently this can't be done reliably because theHiveMetaHook
interface only contains apreAlterTable
method, but nocommitAlterTable
method. We'll need to extend this interface and include the change in an upcoming Hive upstream release.