Skip to content

[AURON #2434] Support full-data-file Iceberg changelog deletes - #2435

Open
weimingdiit wants to merge 3 commits into
apache:masterfrom
weimingdiit:feat/iceberg-changelog-full-file-delete
Open

[AURON #2434] Support full-data-file Iceberg changelog deletes#2435
weimingdiit wants to merge 3 commits into
apache:masterfrom
weimingdiit:feat/iceberg-changelog-full-file-delete

Conversation

@weimingdiit

Copy link
Copy Markdown
Contributor

Which issue does this PR close?
Closes #2434

Rationale for this change
Auron native Iceberg changelog scan currently supports insert-only changelog tasks.

Full-data-file delete changelog tasks can be executed natively without row-level delete-file handling. Auron can scan the deleted data file directly and materialize changelog metadata from the Iceberg changelog task.

Supporting this case improves native Iceberg changelog scan coverage while keeping more complex delete and update semantics on Spark's reader.

What changes are included in this PR?
Allows native Iceberg changelog scan to accept DeletedDataFileScanTask when:

  • the changelog operation is DELETE
  • existingDeletes() is empty

Keeps existing native support for AddedRowsScanTask when:

  • the changelog operation is INSERT
  • deletes() is empty

Keeps fallback behavior for unsupported changelog tasks, including row-level deletes, position/equality deletes, update changelog tasks, mixed file formats, and delete tasks with existing delete files.

Adds coverage for:

  • full-data-file delete changelog native scan
  • a changelog range that contains both insert tasks and full-data-file delete tasks

Are there any user-facing changes?
No user-facing API changes. More Iceberg changelog scans can now be executed natively by Auron.

How was this patch tested?
UT.

Allow native Iceberg changelog scan to handle DeletedDataFileScanTask
when the task represents a DELETE operation and has no existing delete
files.

Keep existing support for insert changelog tasks and continue falling
back for row-level deletes, position/equality deletes, update changelog
tasks, mixed file formats, and delete tasks with existing deletes.

Add tests for full-data-file delete changelog scans and mixed
insert/delete changelog ranges.

Signed-off-by: weimingdiit <weimingdiit@gmail.com>

@merrily01 merrily01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@weimingdiit Thanks for the contribution. The implementation looks correct to me, just a few comments inline.

Signed-off-by: weimingdiit <weimingdiit@gmail.com>
Signed-off-by: weimingdiit <weimingdiit@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The native-acceptance guard for DeletedDataFileScanTask does not enforce the PR’s stated operation == DELETE constraint, risking incorrect native execution for non-DELETE changelog tasks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR expands Auron’s native Iceberg changelog scan support to include full-data-file delete changelog tasks (while keeping row-level delete semantics on Spark’s reader), improving native coverage for simple DELETE changelog ranges.

Changes:

  • Extend native changelog planning to accept DeletedDataFileScanTask when it can be handled by scanning the data file directly.
  • Refactor changelog task handling to a unified “native changelog data-file task” representation used for native scan task construction.
  • Add integration tests covering full-data-file delete changelog scans and mixed insert + full-data-file delete ranges.
File summaries
File Description
thirdparty/auron-iceberg/src/main/scala/org/apache/spark/sql/auron/iceberg/IcebergScanSupport.scala Adds native handling for full-data-file delete changelog tasks by mapping supported changelog tasks into native scan tasks.
thirdparty/auron-iceberg/src/test/scala/org/apache/auron/iceberg/AuronIcebergIntegrationSuite.scala Adds integration coverage for native execution of delete-only and mixed insert+delete changelog ranges.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment on lines +560 to +573
private def toNativeChangelogDataFileTask(
task: ChangelogScanTask): Option[NativeChangelogDataFileTask] = {
task match {
case added: AddedRowsScanTask
if added.operation() == ChangelogOperation.INSERT &&
deletesEmpty(added.deletes()) =>
Some(NativeChangelogDataFileTask(added.file(), added.start(), added.length(), added))
case deleted: DeletedDataFileScanTask if deletesEmpty(deleted.existingDeletes()) =>
Some(
NativeChangelogDataFileTask(deleted.file(), deleted.start(), deleted.length(), deleted))
case _ =>
None
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No change requested, just evidence on this one.

The changelog task type that would actually be unsafe to run natively is DeletedRowsScanTask, the merge-on-read sibling that carries addedDeletes(). In Iceberg 1.10.1 it declares (javadoc stripped):

public interface DeletedRowsScanTask extends ChangelogScanTask, ContentScanTask<DataFile> {
  List<DeleteFile> addedDeletes();

  List<DeleteFile> existingDeletes();

  @Override
  default ChangelogOperation operation() {
    return ChangelogOperation.DELETE;
  }
}

It extends ChangelogScanTask, not DeletedDataFileScanTask, so the type match on this line is what keeps it off the native path. Its operation() is hard-coded DELETE as well, so an operation() check would not have excluded it either. Only the type match does.

For the _change_type angle: the metadata case at IcebergScanSupport.scala:620 reads the value straight off the task (requiredChangelogTask(name).operation().name()) rather than off the Scala type, so a subtype that overrode operation() would stamp its own value instead of a mismatched one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that matches my reading.

The native path is intentionally limited by the concrete task type here. DeletedRowsScanTask is a separate ChangelogScanTask implementation, not a DeletedDataFileScanTask, so it will not match this case and will continue to fall back. Keeping or removing the operation() == DELETE check does not affect that, since DeletedRowsScanTask also hard-codes operation() to DELETE.

For _change_type, I agree the value should come from the task operation itself. The metadata materialization path already uses requiredChangelogTask(name).operation().name(), so if Iceberg ever provides a supported changelog task with a different operation value, the metadata column will reflect the task value rather than the Scala match type.

No additional code change from my side.

Comment on lines +745 to +749
test("iceberg native scan supports mixed insert and full-data-file delete changelog scan") {
withTable("local.db.t_changelog_mixed_delete") {
withTempView("t_changelog_mixed_delete_changes") {
sql("""
|create table local.db.t_changelog_delete (id int, v string)
|create table local.db.t_changelog_mixed_delete (id int, v string, p int)

@slfan1989 slfan1989 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the contribution! The task-type guard correctly limits native execution to AddedRowsScanTask and DeletedDataFileScanTask, while the empty-delete checks preserve the fallback boundary for row-level delete semantics. The changelog metadata and mixed-format fallback behavior are also retained. The changes look good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support full-data-file delete Iceberg changelog tasks in native scan

5 participants