datafusion: no way to set snapshot summary properties on the `insert_into` commit path
- Dominant language
- Rust
- Stars
- 1.4k
- Forks
- 567
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 93
Description
The transaction API supports user snapshot properties on an append — `FastAppendAction::set_snapshot_properties` (`crates/iceberg/src/transaction/append.rs:70`), carried into the snapshot summary and validated against reserved keys (#2744, #2725 show this surface is maintained). The DataFusion integration cannot reach it: `IcebergTableProvider::insert_into` (`crates/integrations/datafusion/src/table/mod.rs:153`) builds `IcebergWriteExec` + `IcebergCommitExec` (`table/mod.rs:221-226`), and the commit node creates the transaction internally —
```rust
// crates/integrations/datafusion/src/physical_plan/commit.rs:243-252
let tx = Transaction::new(&table);
let action = tx.fast_append().add_data_files(data_files);
let _updated_table = action
.apply(tx)
.map_err(to_datafusion_error)?
.commit(catalog.as_ref())
.await?;
```
— with no hook between building the action and committing it, so the properties parameter that exists one layer down is unreachable from an `INSERT INTO`.
(Line references are against main @ a500a2e7; the same shape ships in 0.10.x.)
## Why this matters
Facts about a write are naturally snapshot summary properties: source scan counts, rows dropped by an ingest filter, cast-failure tallies, a recipe/job id — anything audit- or lineage-shaped that describes *this append* rather than the table. An embedder who wants them today has to reimplement the integration's whole write path beside the integration — the same `RollingFileWriterBuilder` + `DataFileWriterBuilder` calls as `physical_plan/write.rs:239`, the same `fast_append` as `commit.rs:244` — solely to pass one `HashMap` the transaction API already accepts. That is what we ended up doing, and the duplicated path has to track every upstream improvement to the real one by hand.
## Proposal
The smallest API that closes the gap: a builder-style setter on the provider, threaded through to the commit node —
```rust
let provider = IcebergTableProvider::try_new(...)
.await?
.with_snapshot_properties(props); // HashMap
```
`IcebergCommitExec` gains the field and applies it:
```rust
let action = tx.fast_append()
.add_data_files(data_files)
.set_snapshot_properties(self.snapshot_properties.clone());
```
Granularity note: properties set this way are per-provider, which is per-statement for embedders that register a provider per statement, and that is the audit use case. A session/statement-option spelling (e.g. `iceberg.snapshot-property.*`) could layer on later for SQL-level use, but the provider hook alone removes the need to fork the write path.
Reserved-key validation stays where it is — the action already owns it, so an invalid property fails the commit exactly as it does through the transaction API directly.
Happy to send a PR if the shape sounds right.
Contributor guide
Research direction
Start with IcebergTableProvider::insert_into in crates/integrations/datafusion/src/table/mod.rs and the commit flow in crates/integrations/datafusion/src/physical_plan/commit.rs, then compare FastAppendAction::set_snapshot_properties in crates/iceberg/src/transaction/append.rs. Trace the provider value through IcebergCommitExec and verify that an INSERT INTO commit accepts the properties while preserving the existing reserved-key validation behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design, databases
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100