Skip to content

Conversation

@xxchan
Copy link
Collaborator

@xxchan xxchan commented Mar 20, 2025

refactor: refactor incremental scan

As pointed out at https://github.com/risingwavelabs/iceberg-rust/pull/13/files#r2004760818, the rushed original version is problematic. It didn't handle concurrency well.

So in this PR I refactored the implementation to share code path with normal full table scan as much as possible.

xxchan added 3 commits March 20, 2025 16:44
Signed-off-by: xxchan <[email protected]>
Signed-off-by: xxchan <[email protected]>
@xxchan xxchan requested a review from Copilot March 20, 2025 09:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the incremental scan implementation to share more common code with the full table scan, addressing concurrency issues and removing legacy paths.

  • Added an early error for delete file processing when using incremental scan
  • Replaced the custom incremental scan branch with a unified manifest file context builder
  • Introduced a filter function for manifest entries in the scan processing logic
Comments suppressed due to low confidence (1)

crates/iceberg/src/scan.rs:246

  • Consider enhancing the error message to include guidance on how to resolve the issue (e.g., instructions to disable delete_file_processing_enabled for incremental scans) to aid future debugging.
if self.delete_file_processing_enabled {

Copy link
Collaborator

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

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

LGTM

@xxchan xxchan merged commit d0fd9be into risingwavelabs:dev_rebase_main_20250307 Mar 21, 2025
11 checks passed
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.

2 participants