Skip to content

Commit c915c31

Browse files
authored
fix(datafusion): reject non-append insert operations (#2714)
## Which issue does this PR close? - Closes #2711. ## What changes are included in this PR? Reject DataFusion `InsertOp::Overwrite` and `InsertOp::Replace` before constructing an Iceberg write plan. Previously, `IcebergTableProvider::insert_into` ignored the requested insert operation while the commit executor always used `fast_append`. Unsupported overwrite or replace requests could therefore be silently executed with append semantics. This patch keeps the existing append path unchanged and returns `DataFusionError::NotImplemented` for unsupported insert operations. It preserves the existing `_insert_op` parameter name to avoid changing the generated public API snapshot. ## Are these changes tested? Yes. Added unit tests verifying that: - `InsertOp::Overwrite` returns `DataFusionError::NotImplemented`. - `InsertOp::Replace` returns `DataFusionError::NotImplemented`. - Existing append-insert behavior remains unchanged. Local verification: - `cargo test -p iceberg-datafusion test_catalog_backed_provider_rejects -- --nocapture` - `cargo fmt --all -- --check` - `git diff --check`
1 parent 4215bd3 commit c915c31

1 file changed

Lines changed: 42 additions & 0 deletions

File tree

  • crates/integrations/datafusion/src/table

crates/integrations/datafusion/src/table/mod.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,12 @@ impl TableProvider for IcebergTableProvider {
161161
input: Arc<dyn ExecutionPlan>,
162162
_insert_op: InsertOp,
163163
) -> DFResult<Arc<dyn ExecutionPlan>> {
164+
if _insert_op != InsertOp::Append {
165+
return Err(DataFusionError::NotImplemented(format!(
166+
"IcebergTableProvider supports only append inserts, got {_insert_op}"
167+
)));
168+
}
169+
164170
// Load fresh table metadata from catalog
165171
let table = self
166172
.catalog
@@ -710,6 +716,42 @@ mod tests {
710716
false
711717
}
712718

719+
#[tokio::test]
720+
async fn test_catalog_backed_provider_rejects_non_append_op() {
721+
use datafusion::physical_plan::empty::EmptyExec;
722+
723+
let (catalog, namespace, table_name, _temp_dir) = get_test_catalog_and_table().await;
724+
let provider = IcebergTableProvider::try_new(catalog, namespace, table_name)
725+
.await
726+
.unwrap();
727+
let ctx = SessionContext::new();
728+
729+
for (insert_op, expected_message) in [
730+
(
731+
InsertOp::Overwrite,
732+
"IcebergTableProvider supports only append inserts, got Insert Overwrite",
733+
),
734+
(
735+
InsertOp::Replace,
736+
"IcebergTableProvider supports only append inserts, got Replace Into",
737+
),
738+
] {
739+
let input = Arc::new(EmptyExec::new(provider.schema())) as Arc<dyn ExecutionPlan>;
740+
let error = provider
741+
.insert_into(&ctx.state(), input, insert_op)
742+
.await
743+
.expect_err("non-append inserts should be rejected");
744+
745+
assert!(
746+
matches!(
747+
error,
748+
DataFusionError::NotImplemented(ref message) if message == expected_message
749+
),
750+
"unexpected error: {error}"
751+
);
752+
}
753+
}
754+
713755
#[tokio::test]
714756
async fn test_insert_plan_fanout_enabled_no_sort() {
715757
use datafusion::datasource::TableProvider;

0 commit comments

Comments
 (0)