Skip to content

fix(spark): make export_instants descending order work#19172

Merged
wombatu-kun merged 1 commit into
apache:masterfrom
yihua:fix-export-instants-desc
Jul 5, 2026
Merged

fix(spark): make export_instants descending order work#19172
wombatu-kun merged 1 commit into
apache:masterfrom
yihua:fix-export-instants-desc

Conversation

@yihua

@yihua yihua commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Describe the issue this Pull Request addresses

export_instants(..., desc => true) throws UnsupportedOperationException on every invocation. ExportInstantsProcedure builds its instant and file-status lists via asJava over immutable Scala collections, producing read-only java.util.Lists. The desc branch then reverses them in place with Collections.reverse, which calls List.set and fails on a read-only list. So descending export has never worked.

Summary and Changelog

  • Wrap both nonArchivedInstants and archivedStatuses in a mutable new java.util.ArrayList(...) so Collections.reverse succeeds. Ascending export is unaffected.
  • Add TestExportInstantsProcedure coverage: ascending export, the descending-order regression case (which fails without this fix), action filtering, and the invalid-local-folder error path.

Impact

Fixes export_instants with desc => true. No change to ascending behavior or other procedures.

Risk Level

low. One-line-per-list change from an immutable wrapper to a mutable copy; the descending regression test exercises the previously-broken path.

Documentation Update

none

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

@github-actions github-actions Bot added the size:M PR with lines of changes in (100, 300] label Jul 3, 2026

@hudi-agent hudi-agent 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.

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for the contribution! This PR fixes export_instants(..., desc => true), which previously always threw UnsupportedOperationException because Collections.reverse was called on read-only lists produced by asJava over immutable Scala collections; it now wraps both the instant and archive-status lists in mutable ArrayLists and adds regression test coverage. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

export_instants(..., desc => true) threw UnsupportedOperationException on
every call: the instant/status lists were built via asJava over immutable
Scala collections (read-only java.util.List), and the desc branch reverses
them in place with Collections.reverse, which calls List.set. Wrap both in
a mutable ArrayList. Adds TestExportInstantsProcedure coverage including the
desc-ordering regression case.
@yihua yihua force-pushed the fix-export-instants-desc branch from f3fee10 to ec33b4d Compare July 4, 2026 18:27

@hudi-agent hudi-agent 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.

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! This PR fixes export_instants(..., desc => true), which previously threw UnsupportedOperationException because Collections.reverse was called on read-only lists produced by asJava over immutable Scala collections. Wrapping both nonArchivedInstants and archivedStatuses in mutable ArrayList copies resolves the issue, and the ascending path is unaffected. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

@hudi-bot

hudi-bot commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@wombatu-kun wombatu-kun merged commit 8347be4 into apache:master Jul 5, 2026
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M PR with lines of changes in (100, 300]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants