You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "xanderbailey (via GitHub)" <gi...@apache.org> on 2026/08/18 11:35:57 UTC
[PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
xanderbailey opened a new pull request, #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Closes https://github.com/apache/iceberg-rust/issues/2919
- Working towards: https://github.com/apache/iceberg-rust/issues/2034
## What changes are included in this PR?
Since merging https://github.com/apache/iceberg-rust/pull/2701 we can now write encrypted parquet files which means we now fully support read and write paths. The last step is to remove the error on the commit path to allow users to actually write encrypted tables.
<!--
Provide a summary of the modifications in this PR. List the main changes such as new features, bug fixes, refactoring, or any other updates.
-->
## Are these changes tested?
I've added roundtrip tests for datafusion in a new slt test and purge table catalog tests can now be added since we can commit to an encrypted table.
<!--
Specify what test covers (unit test, integration test, etc.).
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## AI Disclosure
<!--
https://iceberg.apache.org/contribute/#guidelines-for-ai-assisted-contributions
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "blackmwk (via GitHub)" <gi...@apache.org>.
blackmwk commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3820422151
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +328,82 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_catalog(CatalogKind::Glue)]
+#[case::sql_catalog(CatalogKind::Sql)]
+#[case::memory_catalog(CatalogKind::Memory)]
+#[tokio::test]
+async fn test_catalog_purge_encrypted_table(#[case] kind: CatalogKind) -> Result<()> {
Review Comment:
I was thinking about append data tx action, but I think datafusion tests already covered that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "xanderbailey (via GitHub)" <gi...@apache.org>.
xanderbailey commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3821341347
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +326,93 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_plain(CatalogKind::Glue, TableMode::Plain)]
Review Comment:
HMS doesn't work either actually `FeatureUnsupported => Updating a table is not supported yet`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "blackmwk (via GitHub)" <gi...@apache.org>.
blackmwk commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3820417655
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +326,93 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_plain(CatalogKind::Glue, TableMode::Plain)]
Review Comment:
There are some catalog kinds are missing, why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "xanderbailey (via GitHub)" <gi...@apache.org>.
xanderbailey commented on PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#issuecomment-5367567138
Done! Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "blackmwk (via GitHub)" <gi...@apache.org>.
blackmwk merged PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "blackmwk (via GitHub)" <gi...@apache.org>.
blackmwk commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3811530624
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +328,82 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_catalog(CatalogKind::Glue)]
+#[case::sql_catalog(CatalogKind::Sql)]
+#[case::memory_catalog(CatalogKind::Memory)]
+#[tokio::test]
+async fn test_catalog_purge_encrypted_table(#[case] kind: CatalogKind) -> Result<()> {
Review Comment:
We have several methods for updating table, I'm thinking maybe we should make encrypted or not a test case param?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "xanderbailey (via GitHub)" <gi...@apache.org>.
xanderbailey commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3820940156
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +326,93 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_plain(CatalogKind::Glue, TableMode::Plain)]
Review Comment:
Ah sorry I missed Hms and S3Tables, REST purges server-side so this isn't supported yet
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +326,93 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_plain(CatalogKind::Glue, TableMode::Plain)]
Review Comment:
Ah sorry I missed Hms and S3Tables, REST purges server-side so this isn't supported yet for encryption
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "xanderbailey (via GitHub)" <gi...@apache.org>.
xanderbailey commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3821341347
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +326,93 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_plain(CatalogKind::Glue, TableMode::Plain)]
Review Comment:
HMS doesn't work either actually
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "blackmwk (via GitHub)" <gi...@apache.org>.
blackmwk commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3828713073
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +326,93 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_plain(CatalogKind::Glue, TableMode::Plain)]
Review Comment:
Thanks for explaination.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "blackmwk (via GitHub)" <gi...@apache.org>.
blackmwk commented on PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#issuecomment-5367532758
Please help to resolve conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feature(encryption): [17/N] Enable encrypted writes [iceberg-rust]
Posted by "xanderbailey (via GitHub)" <gi...@apache.org>.
xanderbailey commented on code in PR #3018:
URL: https://github.com/apache/iceberg-rust/pull/3018#discussion_r3812124387
##########
crates/catalog/loader/tests/table_suite.rs:
##########
@@ -325,6 +328,82 @@ async fn test_catalog_purge_table(#[case] kind: CatalogKind) -> Result<()> {
Ok(())
}
+#[rstest]
+#[case::glue_catalog(CatalogKind::Glue)]
+#[case::sql_catalog(CatalogKind::Sql)]
+#[case::memory_catalog(CatalogKind::Memory)]
+#[tokio::test]
+async fn test_catalog_purge_encrypted_table(#[case] kind: CatalogKind) -> Result<()> {
Review Comment:
Gave it a go [85bc744](https://github.com/apache/iceberg-rust/pull/3018/commits/85bc744fada8218ce83212fdb230eecc906c9ec5) for the purge test, were there others you think I needed to include in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org