Skip to content

Conversation

adriangb
Copy link
Contributor

@github-actions github-actions bot added the core Core DataFusion crate label Jul 17, 2025
@@ -433,7 +433,7 @@ impl ListingTableConfig {
/// `SchemaAdapterFactory` is set, in which case only the `SchemaAdapterFactory` will be used.
///
/// See <https://github.com/apache/datafusion/issues/16800> for details on this transition.
pub fn with_physical_expr_adapter_factory(
pub fn with_expr_adapter_factory(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In writing the example I noticed that this name is very long and probably too verbose.
This tones it down and brings it in line with the field name.

Copy link
Contributor

Choose a reason for hiding this comment

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

I love this explanation -- I have found trying to write out examples of using APIs almost always leads to discoveries of ways to improve them for the better. Thank you!

@@ -101,7 +101,7 @@ pub struct ListingTableConfig {
/// Optional [`SchemaAdapterFactory`] for creating schema adapters
schema_adapter_factory: Option<Arc<dyn SchemaAdapterFactory>>,
/// Optional [`PhysicalExprAdapterFactory`] for creating physical expression adapters
physical_expr_adapter_factory: Option<Arc<dyn PhysicalExprAdapterFactory>>,
expr_adapter_factory: Option<Arc<dyn PhysicalExprAdapterFactory>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

this rename might cause api_change label along with Upgrade Guide changes, I would stick to just example in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This API was added yesterday in #16791

Copy link
Contributor

@comphead comphead left a comment

Choose a reason for hiding this comment

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

lgtm thanks @adriangb

@adriangb adriangb merged commit eabf3b7 into apache:main Jul 17, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants