Skip to content

Minor: use FileScanConfig builder API in some tests #14938

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

Merged
merged 3 commits into from
Mar 3, 2025

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Feb 28, 2025

Which issue does this PR close?

Rationale for this change

I noticed some additional cleanup that was possible while reviewing #14685

The FileScanConfig builder API is both more concise but also requires fewer changes when / if FileScanConfig is changed

What changes are included in this PR?

Change some proto tests to use the more concise API

Are these changes tested?

Yes, by CI

Are there any user-facing changes?

No, this is entirely test code

@github-actions github-actions bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) proto Related to proto crate labels Feb 28, 2025
@alamb alamb force-pushed the alamb/try_no_statistics branch from ddc14bf to 40fcbe1 Compare February 28, 2025 15:27
@github-actions github-actions bot removed core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Feb 28, 2025
file_source: source,
};
let file_source = ParquetSource::default().with_statistics(statistics.clone());
let scan_config = FileScanConfig::new(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this also makes the non-default fields more clear

@alamb alamb marked this pull request as ready for review February 28, 2025 15:31
Copy link
Contributor

@berkaysynnada berkaysynnada left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @alamb

@alamb alamb merged commit 787adf0 into apache:main Mar 3, 2025
24 checks passed
@alamb
Copy link
Contributor Author

alamb commented Mar 3, 2025

Thank you for the review @berkaysynnada

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proto Related to proto crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants