Skip to content

Commit ff6358b

Browse files
committed
Clarify usage with documentation and naming
1 parent cc6ea44 commit ff6358b

2 files changed

Lines changed: 13 additions & 9 deletions

File tree

datafusion/ffi/src/config/mod.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,15 @@ impl TryFrom<FFI_ConfigOptions> for ConfigOptions {
8181
}
8282

8383
pub trait ExtensionOptionsFFIProvider {
84-
fn ffi_extension<C: ConfigExtension + Clone + Default>(&self) -> Option<C>;
84+
/// Extract a [`ConfigExtension`]. This method should attempt to first extract
85+
/// the extension from the local options when possible. Should that fail, it
86+
/// should attempt to extract the FFI options and then convert them to the
87+
/// desired [`ConfigExtension`].
88+
fn local_or_ffi_extension<C: ConfigExtension + Clone + Default>(&self) -> Option<C>;
8589
}
8690

8791
impl ExtensionOptionsFFIProvider for ConfigOptions {
88-
fn ffi_extension<C: ConfigExtension + Clone + Default>(&self) -> Option<C> {
92+
fn local_or_ffi_extension<C: ConfigExtension + Clone + Default>(&self) -> Option<C> {
8993
self.extensions
9094
.get::<C>()
9195
.map(|v| v.to_owned())
@@ -98,7 +102,7 @@ impl ExtensionOptionsFFIProvider for ConfigOptions {
98102
}
99103

100104
impl ExtensionOptionsFFIProvider for TableOptions {
101-
fn ffi_extension<C: ConfigExtension + Clone + Default>(&self) -> Option<C> {
105+
fn local_or_ffi_extension<C: ConfigExtension + Clone + Default>(&self) -> Option<C> {
102106
self.extensions
103107
.get::<C>()
104108
.map(|v| v.to_owned())

datafusion/ffi/tests/ffi_config.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ mod tests {
4444

4545
// Verify default values are as expected
4646
let returned_config: ExternalConfig = config
47-
.ffi_extension()
47+
.local_or_ffi_extension()
4848
.expect("should have external config extension");
4949
assert_eq!(returned_config, ExternalConfig::default());
5050

5151
config.set("external_config.is_enabled", "false")?;
5252
let returned_config: ExternalConfig = config
53-
.ffi_extension()
53+
.local_or_ffi_extension()
5454
.expect("should have external config extension");
5555
assert!(!returned_config.is_enabled);
5656

@@ -74,14 +74,14 @@ mod tests {
7474

7575
// Verify default values are as expected
7676
let returned_options: ExternalConfig = table_options
77-
.ffi_extension()
77+
.local_or_ffi_extension()
7878
.expect("should have external config extension");
7979

8080
assert_eq!(returned_options, ExternalConfig::default());
8181

8282
table_options.set("external_config.is_enabled", "false")?;
8383
let returned_options: ExternalConfig = table_options
84-
.ffi_extension()
84+
.local_or_ffi_extension()
8585
.expect("should have external config extension");
8686
assert!(!returned_options.is_enabled);
8787

@@ -105,7 +105,7 @@ mod tests {
105105
// Verify default values are as expected
106106
let returned_config: ExternalConfig = config
107107
.options()
108-
.ffi_extension()
108+
.local_or_ffi_extension()
109109
.expect("should have external config extension");
110110
assert_eq!(returned_config, ExternalConfig::default());
111111

@@ -115,7 +115,7 @@ mod tests {
115115
);
116116
let returned_config: ExternalConfig = config
117117
.options()
118-
.ffi_extension()
118+
.local_or_ffi_extension()
119119
.expect("should have external config extension");
120120
assert!(!returned_config.is_enabled);
121121

0 commit comments

Comments
 (0)