Skip to content

fix(datafusion): reject non-append insert operations#2714

Open
u70b3 wants to merge 1 commit into
apache:mainfrom
u70b3:fix/datafusion-insert-op-validation
Open

fix(datafusion): reject non-append insert operations#2714
u70b3 wants to merge 1 commit into
apache:mainfrom
u70b3:fix/datafusion-insert-op-validation

Conversation

@u70b3

@u70b3 u70b3 commented Jun 25, 2026

Copy link
Copy Markdown

Which issue does this PR close?

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

@CTTY CTTY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just one minor comment

),
"unexpected error: {error}"
);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about merging these 2 tests into 1 test_catalog_backed_provider_rejects_non_append_op

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the comment, already merge two testcase

@u70b3 u70b3 force-pushed the fix/datafusion-insert-op-validation branch from b824bb5 to 4c1f548 Compare June 30, 2026 03:48
@u70b3 u70b3 force-pushed the fix/datafusion-insert-op-validation branch from 1a8f80a to d1e1c26 Compare June 30, 2026 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataFusion non-append InsertOp is silently committed as append

2 participants