PR to compare with main by corwinjoy · Pull Request #9 · corwinjoy/delta-rs

corwinjoy · 2025-09-04T00:45:44Z

Compare latest df-itegrate with main to see what a PR would look like

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

… constructor Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

This reverts commit a3ca59b

…hanges through. Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

…o examples. Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

…sionContext Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

# Conflicts: # crates/core/src/operations/mod.rs # crates/core/src/operations/update.rs

Copilot

Pull Request Overview

This pull request introduces file format options infrastructure to the Delta Lake Rust library, enabling support for Parquet encryption and other customizable file format behaviors. The changes refactor how writer properties are handled throughout the codebase, transitioning from direct WriterProperties usage to a more flexible WriterPropertiesFactory pattern.

Adds new file format options abstraction and encryption support infrastructure
Refactors all operations to use WriterPropertiesFactory instead of direct WriterProperties
Integrates encryption capabilities into DataFusion operations and file scanning

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/core/src/table/file_format_options.rs	New module providing file format options trait and WriterPropertiesFactory abstraction
crates/core/src/operations/encryption.rs	New encryption configuration module for table-level encryption support
python/src/lib.rs	Updates Python bindings to use new constructor signature with None parameter
crates/core/src/operations/write/writer.rs	Major refactor replacing WriterProperties with WriterPropertiesFactory pattern
crates/core/src/operations/optimize.rs	Enhanced optimize operations with encryption support and new factory pattern
crates/deltalake/examples/	New example files demonstrating encryption capabilities

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

python/src/merge.rs

crates/core/src/table/file_format_options.rs

Copilot · 2025-09-04T00:48:37Z

crates/core/src/operations/write/writer.rs

+                )
+                .await?;


The asynchronous call in the constructor pattern is unusual. Consider whether the factory creation should be synchronous or if this indicates a design issue where async operations are being performed during object construction.

@adamreeve maybe copilot has a point here? I guess this is coming from create_writer_properties now being async. Do we need to change this?

Hmm, I agree it's not ideal to have to change PartitionWriterConfig::try_new to be async, but git grep 'async fn try_' finds a bunch of other async constructors so I don't think this is that unusual. And I can't see a way to avoid this.

crates/core/src/operations/optimize.rs

crates/core/src/writer/utils.rs

# Conflicts: # crates/core/src/delta_datafusion/mod.rs # crates/core/src/operations/mod.rs # crates/deltalake/Cargo.toml

adamreeve

This is looking good thanks Corwin. I've left some comments on possible improvements, but I think this is something we could create a draft PR from to try to solicit feedback and start a discussion.

adamreeve · 2025-09-04T23:27:13Z

crates/core/src/delta_datafusion/table_provider.rs

+        // We have to set the encryption factory on the ParquetSource based on the Parquet options,
+        // as this is usually handled by the ParquetFormat type in DataFusion,
+        // which is not used in delta-rs.


If we pass a FileFormatOptions instead of TableParquetOptions, we could maybe add a new method to FileFormatOptions to allow customizing the ParquetSource creation using and avoid needing to set the encryption factory here.

Although I don't feel like this code is too bad but it does leak some knowledge of Parquet encryption outside of the FileFormatOptions implementation.

adamreeve · 2025-09-04T23:28:07Z

crates/core/src/delta_datafusion/table_provider.rs

    ) -> Result<Arc<dyn ExecutionPlan>> {
        register_store(self.log_store(), session.runtime_env().clone());
+        if let Some(format_options) = &self.file_format_options {
+            format_options.update_session(session)?;


It might be nice to consolidate this session update with the register_store method above.

adamreeve · 2025-09-04T23:33:30Z

crates/core/src/operations/optimize.rs

                            let meta = ObjectMeta::try_from(file).unwrap();
+                            let file_format_options = file_format_options.clone();
                            async move {
+                                let decrypt: Option<FileDecryptionProperties> =


We might be able to tidy this up by adding a method to get ArrowReaderOptions, or alternatively changing this to use DataFusion for reading like for Z-Order compaction.

adamreeve · 2025-09-04T23:37:12Z

crates/core/src/operations/optimize.rs


-            let writer_properties = this.writer_properties.unwrap_or_else(|| {
-                WriterProperties::builder()
-                    .set_compression(Compression::ZSTD(ZstdLevel::try_new(4).unwrap()))


For completeness, we probably want a way for FileFormatOptions / WriterPropertiesFactory to use different compression or other properties for files written during compaction.

I guess users can just modify the options passed when the set up the DeltaOps, so maybe that's enough and we don't need to change the API to support this when reusing the same options for different ops.

adamreeve · 2025-09-04T23:38:37Z

crates/core/src/table/file_format_options.rs

+}
+
+#[async_trait]
+pub trait WriterPropertiesFactory: Send + Sync + std::fmt::Debug + 'static {


I wonder if we could move these methods into FileFormatOptions rather than need a separate trait.

I guess this might be needed for backwards compatibility with setting writer properties though.

adamreeve · 2025-09-04T23:39:38Z

crates/core/src/table/file_format_options.rs

+// More advanced factory with KMS support
+#[cfg(feature = "datafusion")]
+#[derive(Clone, Debug)]
+pub struct KMSWriterPropertiesFactory {


It might make sense to move things around a bit so this lives in the encryption module alongside TableEncryption. encryption is also nested under operations but could maybe be next to file_format_options under the table module?

KmsFileFormatOptions is defined in the example, but maybe it should be in the same module as this type too.

adamreeve · 2025-09-04T23:45:00Z

crates/core/src/table/file_format_options.rs

+}
+
+#[cfg(feature = "datafusion")]
+pub fn state_with_parquet_options(


I still think we should avoid setting table specific options on the state if possible (registering encryption factory is different as the factory also needs to be specified by id in the table options). I will take a look at the cases where this is used and see if we can remove the need for this.

Made a PR for this here: #11

crates/core/src/table/file_format_options.rs

adamreeve · 2025-09-04T23:49:37Z

crates/deltalake/examples/basic_operations_kms_encryption.rs

+    Ok(())
+}
+
+async fn round_trip_test() -> Result<(), deltalake::errors::DeltaTableError> {


As well as having an example, it would be good to have unit tests for each type of operation that check that the table files are all encrypted after the operation is complete.

corwinjoy and others added 30 commits August 13, 2025 15:27

Begin adding table_parquet_options to DeltaTable and builder.

648d4c1

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Create a new trait for operations that write to a DeltaTable

a3ca59b

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Add writer properties to DeltaOps

fff516c

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Add TableParquetOptions to LoadBuilder. Fix broken tests + example.

acf35cd

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Move table_parquet_options out of DeltaTableConfig, update DeltaTable…

666f0ba

… constructor Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Revert "Create a new trait for operations that write to a DeltaTable"

5abdbf7

This reverts commit a3ca59b

Add table_parquet_options to DeltaTable::new_with_state and cascade c…

77acb20

…hanges through. Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Add crypto example

cdbb408

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Backout moved functions from trait. Add basic_operations_encryption t…

fc9a460

…o examples. Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Get basic roundtrip running and document more on SessionState vs. Ses…

47dd02a

…sionContext Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Update comment

4795b00

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Where it makes sense, apply table options to the session state

62fc852

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Remove dead code.

1fc9fd8

Signed-off-by: Corwin Joy <corwin.joy@gmail.com>

Pass parquet options through to DeltaScanBuilder per Adam

f8e6531

Get update operation working.

cf560a8

Get delete_from_table working.

9c458d3

Get basic merge working.

7c35313

Pretty-print results from read_table

1628345

Consolidate function to add parquet options to state.

302c5af

cargo fmt

1de1899

Add use of cfg to reflect datafusion dependency

27356aa

Add test call to optimize operation

1721863

Fix optimize logic to use decryptor

bf255ed

Minor tidy up of optimize op

81c37cf

Add support for optimize z-order.

45abaeb

Merge branch 'main' into crypto_config2

3981851

# Conflicts: # crates/core/src/operations/mod.rs # crates/core/src/operations/update.rs

cargo fmt

30c1cc5

Add comment on wrong_key variable

14ea49c

Add initial definition for WriterPropertiesFactory

78d3f08

Switch raw WriterProperties to WriterPropertiesFactory

d9d433e

corwinjoy and others added 22 commits August 27, 2025 16:50

Got first runthrough working with FileFormatOptions trait.

3bc01f5

Begin streamlining code

8f820f6

Streamline table_parquet_options calls

ccbe91f

Streamline basic_operations_encryption.rs

b953f9b

cargo fmt

9c65275

remove unnecessary clone

287385b

rename table_parquet_options.rs to file_format_options.rs

eb35496

Make suggested changes from clippy

4c747e6

Make file_formation_options docs consistent

967be87

Refactor Option<Arc<dyn FileFormatOptions>> as FileFormatRef

6baab66

cargo fmt

aae93f1

Fix missing changes for FileFormatRef refactor

fba1039

Expand the FileFormatRef refactor to better handle function calls.

50e4216

Redo FileFormatRef refactor to simplify

5f00048

Use DataFusion branch with encryption factory

64e347f

Use EncryptionFactory from DataFusion fork

562bd78

WIP Creating example that uses KMS encryption

9e4356b

Get read operation working with KMS

bd7e73c

WIP supporting more operations with a KMS

3970cee

Switch to GR DataFusion fork

d36aec3

Fix compact with KMS

e4c349e

Tidy up TableEncryption

c9200de

corwinjoy requested review from adamreeve and Copilot September 4, 2025 00:47

Copilot AI reviewed Sep 4, 2025

View reviewed changes

corwinjoy added 3 commits September 3, 2025 17:58

Merge branch 'main' into df-integrate

01f5140

# Conflicts: # crates/core/src/delta_datafusion/mod.rs # crates/core/src/operations/mod.rs # crates/deltalake/Cargo.toml

Fix broken merge due to scan functions being moved to table_provider.rs

086ee3a

Migrate changes from basic_operations_kms_encryption.rs

3e93463

adamreeve reviewed Sep 4, 2025

View reviewed changes

adamreeve mentioned this pull request Sep 16, 2025

Advanced file options #14

Draft

Conversation

corwinjoy commented Sep 4, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 4, 2025

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!

Uh oh!

adamreeve left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants