Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: use builder API to create FileScanConfig #3266

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Feb 25, 2025

Description

Note this PR should have no functional change

Similarly to #3265, this is part of the DataFusion 46 upgrade in #3262, which I am trying to split into multiple parts to reduce the size of the diff (so to make it easier to review and debug)

Specifically, ParquetExec builder is non trivially changed as part of the upgrade, and new fields are added to FileScanConfig. This this change updates the API to use the builder style API for FileScanConfig which makes the delta code less verbose as well as making the upgrade easier

Related Issue(s)

Documentation

@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Feb 25, 2025
Copy link

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

output_ordering: vec![],
})
let mut exec_plan_builder = ParquetExecBuilder::new(
FileScanConfig::new(self.log_store.object_store_url(), file_schema)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is to use FileScanConfig::new()` rather than explicitly create the structure manually.

new() fills in the defaults

@alamb alamb force-pushed the alamb/update_filescan_config branch from 0da8007 to c058b5b Compare February 25, 2025 17:54
@alamb alamb marked this pull request as ready for review February 25, 2025 17:57
@alamb alamb force-pushed the alamb/update_filescan_config branch from c058b5b to 4b0cfc1 Compare February 25, 2025 17:59
@alamb alamb changed the title chore: Use Builder API to create FileScanConfig chore: use builder API to create FileScanConfig Feb 25, 2025
Copy link

codecov bot commented Feb 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.11%. Comparing base (4be81fb) to head (3694c23).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3266      +/-   ##
==========================================
- Coverage   72.14%   72.11%   -0.03%     
==========================================
  Files         143      143              
  Lines       45724    45701      -23     
  Branches    45724    45701      -23     
==========================================
- Hits        32986    32959      -27     
+ Misses      10666    10665       -1     
- Partials     2072     2077       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ion-elgreco ion-elgreco force-pushed the alamb/update_filescan_config branch from 4b0cfc1 to 3694c23 Compare February 25, 2025 18:20
@ion-elgreco ion-elgreco added this pull request to the merge queue Feb 25, 2025
@alamb
Copy link
Contributor Author

alamb commented Feb 25, 2025

Thanks @ion-elgreco for the fast reviews 🚤

Merged via the queue into delta-io:main with commit 25fd4a1 Feb 25, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants