feat(encryption) [15/N] write encrypted parquet data files by aarushigupta132 · Pull Request #2701 · apache/iceberg-rust

aarushigupta132 · 2026-06-23T23:34:56Z

Which issue does this PR close?

Working towards #2034.

What changes are included in this PR?

Adds support for writing Iceberg data files with Parquet Modular Encryption: ParquetWriterBuilder::with_encryption_manager opts a writer into encryption, generating a per-file DEK from the table's EncryptionManager and recording its key metadata on the resulting DataFile.

Are these changes tested?

Yes, unit tests cover the encrypted write path for ParquetWriter and RollingFileWriter.

xanderbailey

Thanks carrying the flame on the encryption work. Have made a couple of comments to get us started.

aarushigupta132 · 2026-07-15T23:51:09Z

    type R = ParquetWriter;

    async fn build(&self, output_file: OutputFile) -> Result<Self::R> {
+        let key_metadata = self


Confirmed that in rolling_writer as well, we build this again with new dek for each new parquet file

aarushigupta132 · 2026-07-15T23:53:02Z

    /// When writing into an existing Iceberg table, prefer
    /// [`Self::from_table_properties`], which derives `WriterProperties` from
    /// the table's `write.parquet.*` properties.


curious to know if we should deprecate new and new_with_match_mode in favor of from_table_properties. Or update the comment to prefer the latter in all cases?

I would add a comment in the doc here that encrypted writing is not supported with this constructor. I would honestly be in favour of removing this constructor since it's not used in this crate and nor should it be. I don't think we should do that in this PR but it would be a good follow-up IMO.

also would like to know @blackmwk's recommendation here 🙏

xanderbailey

Few more comments and a test for the writer would be great. Maybe also something that shows datafusion writer works but that can be a follow up

xanderbailey · 2026-07-16T13:55:18Z

    /// When writing into an existing Iceberg table, prefer
    /// [`Self::from_table_properties`], which derives `WriterProperties` from
    /// the table's `write.parquet.*` properties.


I would add a comment in the doc here that encrypted writing is not supported with this constructor. I would honestly be in favour of removing this constructor since it's not used in this crate and nor should it be. I don't think we should do that in this PR but it would be a good follow-up IMO.

mbutrovich

First pass, thanks @aarushigupta132!

xanderbailey · 2026-07-28T18:08:53Z

+            table.metadata().current_schema(),
+        )?;
+
+        // The planner encrypted the file.


Not sure what this means

blackmwk

Thanks @aarushigupta132 , just finished first round of review.

blackmwk · 2026-07-30T09:49:23Z

            .build();
-        Self::new_with_match_mode(props, schema, FieldMatchMode::Id)
+
+        let data_encryption_key_size = table_props


I prefer to make this part of TableProperties, e.g. add a data_entryption_key_size method

blackmwk · 2026-07-30T09:51:34Z

    }
+
+    #[tokio::test]
+    async fn test_iceberg_write_exec_encrypted() -> Result<()> {


This only tested write, not read. I perfer to use sqllogictest to set table properties, and test read and write encrypted parquet files.

I believe a complete read-write test would require removing this check (as this would be a commit)
I would like to follow-up with more round trip tests, whenever we remove this gate.
Would you recommend doing all the mentioned in this pr or should i just remove this datafusion test here and add in a follow up pr?

In this case you could crate an issue to track adding integration tests to for encryption. This test is too difficult to maintain, and I think the ParquetWriter ut is enough for this pr.

blackmwk · 2026-07-31T09:49:49Z

+///
+/// Returns `None` when the table is not configured for encryption.
+/// Returns an error when `encryption.data-key-length` is not a valid AES key length.
+pub(crate) fn data_encryption_key_size(props: &TableProperties) -> Result<Option<AesKeySize>> {


In fact, I mean make this part of TableProperties struct, which feels more natural.

blackmwk · 2026-07-31T10:11:13Z


+    /// Generates a `StandardKeyMetadata` carrying a fresh random DEK of
+    /// `key_size` together with a fresh random AAD prefix.
+    pub fn generate(key_size: AesKeySize) -> Self {


Make this crate private. StandKeyMetadata should be generated by EncryptionManager

blackmwk · 2026-07-31T10:13:42Z

    }
+
+    #[tokio::test]
+    async fn test_iceberg_write_exec_encrypted() -> Result<()> {


In this case you could crate an issue to track adding integration tests to for encryption. This test is too difficult to maintain, and I think the ParquetWriter ut is enough for this pr.

blackmwk · 2026-08-06T09:43:53Z

+    })?;
+
+    Ok(writer_properties
+        .clone()


nit: Do we really need this clone if we already have into_builder?

aarushigupta132 · 2026-08-12T12:38:30Z

thank you @blackmwk for all the reviews! apologies i was a bit preoccupied last week but it is resolved now, happy to iterate with full speed

aarushigupta132 · 2026-08-17T09:05:53Z

@blackmwk gentle ping here 🙏

blackmwk

Thanks @aarushigupta132 for this pr!

aarushigupta132 changed the title ~~Feat/encrypted parquet data files~~ feat(encryption): write encrypted parquet data files Jun 23, 2026

aarushigupta132 marked this pull request as ready for review June 23, 2026 23:45

xanderbailey reviewed Jun 25, 2026

View reviewed changes

Comment thread crates/iceberg/src/writer/file_writer/parquet_writer.rs Outdated

Comment thread crates/iceberg/src/writer/file_writer/parquet_writer.rs Outdated

aarushigupta132 changed the title ~~feat(encryption): write encrypted parquet data files~~ feat(encryption) [15/N] write encrypted parquet data files Jul 15, 2026

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch 2 times, most recently from 58df8f8 to 0783948 Compare July 15, 2026 16:55

aarushigupta132 closed this Jul 15, 2026

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch from 0783948 to b80ebbf Compare July 15, 2026 17:38

aarushigupta132 reopened this Jul 15, 2026

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch 3 times, most recently from cc5fca7 to 74c54db Compare July 15, 2026 23:46

aarushigupta132 commented Jul 15, 2026

View reviewed changes

xanderbailey reviewed Jul 16, 2026

View reviewed changes

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch 2 times, most recently from 5a0ea6f to 9832f50 Compare July 19, 2026 21:53

feat(encryption): write encrypted parquet data files

2ba0884

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch from 9832f50 to 2ba0884 Compare July 19, 2026 21:59

aarushigupta132 added 2 commits July 20, 2026 11:22

Merge branch 'main' into feat/encrypted-parquet-data-files

9e993d1

Merge branch 'main' into feat/encrypted-parquet-data-files

0e7e710

xanderbailey mentioned this pull request Jul 27, 2026

Update purge encryption test to use PME #2917

Closed

Merge branch 'main' into feat/encrypted-parquet-data-files

f27adda

mbutrovich reviewed Jul 28, 2026

View reviewed changes

Comment thread crates/iceberg/src/writer/file_writer/parquet_writer.rs Outdated

Comment thread crates/iceberg/src/writer/file_writer/parquet_writer.rs Outdated

mbutrovich mentioned this pull request Jul 28, 2026

[EPIC] Deletion vector read support (deletion-vector-v1 Puffin blobs) #2792

Open

3 tasks

xanderbailey reviewed Jul 28, 2026

View reviewed changes

blackmwk reviewed Jul 30, 2026

View reviewed changes

refactor(encryption): address review comments

71a2183

blackmwk reviewed Jul 31, 2026

View reviewed changes

Merge branch 'main' into feat/encrypted-parquet-data-files

6a6fcf8

refactor(encryption): route DEK generation through EncryptionManager

3817bf0

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch from 6f56a91 to 57f6604 Compare August 5, 2026 23:10

blackmwk reviewed Aug 6, 2026

View reviewed changes

refactor(encryption): opt into encryption via with_encryption_manager

6526e7c

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch from 833a201 to bb90c29 Compare August 10, 2026 00:44

aarushigupta132 added 2 commits August 10, 2026 01:48

Merge branch 'main' into feat/encrypted-parquet-data-files

b86d466

refactor(encryption): reword encryption doc comments

d76ad77

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch from ab6ce0b to d76ad77 Compare August 10, 2026 01:55

blackmwk reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/iceberg/src/writer/file_writer/parquet_writer.rs Outdated

blackmwk reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/iceberg/src/test_utils.rs Outdated

refactor(encryption): gate encryption on the encryption manager alone

1d691ac

aarushigupta132 added 2 commits August 12, 2026 13:39

Merge branch 'main' into feat/encrypted-parquet-data-files

d217638

refactor(encryption): drop redundant doc lines

6015e0a

aarushigupta132 force-pushed the feat/encrypted-parquet-data-files branch from 1d8daea to 6015e0a Compare August 12, 2026 13:11

Merge branch 'main' into feat/encrypted-parquet-data-files

4289209

blackmwk approved these changes Aug 18, 2026

View reviewed changes

blackmwk merged commit 4908b0b into apache:main Aug 18, 2026
21 checks passed

xanderbailey mentioned this pull request Aug 18, 2026

feature(encryption): [17/N] Enable encrypted writes #3018

Merged

Uh oh!

Conversation

aarushigupta132 commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

What changes are included in this PR?

Are these changes tested?

Uh oh!

xanderbailey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xanderbailey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mbutrovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blackmwk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aarushigupta132 commented Aug 12, 2026

Uh oh!

aarushigupta132 commented Aug 17, 2026

Uh oh!

blackmwk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aarushigupta132 commented Jun 23, 2026 •

edited

Loading