Implement CatalogProviderList in FFI #18657
Conversation
| #[allow(non_camel_case_types)] | ||
| pub struct FFI_CatalogProviderList { | ||
| /// Register a catalog | ||
| pub register_catalog: unsafe extern "C" fn( |
There was a problem hiding this comment.
| pub register_catalog: unsafe extern "C" fn( | |
| pub register_catalog_provider: unsafe extern "C" fn( |
There was a problem hiding this comment.
I think this one should remain as is because it mirrors the CatalogProviderList trait function naming.
| pub register_catalog: unsafe extern "C" fn( | ||
| &Self, | ||
| name: RString, | ||
| catalog: &FFI_CatalogProvider, |
There was a problem hiding this comment.
| catalog: &FFI_CatalogProvider, | |
| catalog_provider: &FFI_CatalogProvider, |
There was a problem hiding this comment.
Same, mirrors the CatalogProviderList definition.
| ) -> ROption<FFI_CatalogProvider>, | ||
|
|
||
| /// List of existing catalogs | ||
| pub catalog_names: unsafe extern "C" fn(&Self) -> RVec<RString>, |
There was a problem hiding this comment.
| pub catalog_names: unsafe extern "C" fn(&Self) -> RVec<RString>, | |
| pub catalog_provider_names: unsafe extern "C" fn(&Self) -> RVec<RString>, |
| pub catalog_names: unsafe extern "C" fn(&Self) -> RVec<RString>, | ||
|
|
||
| /// Access a catalog | ||
| pub catalog: |
There was a problem hiding this comment.
| pub catalog: | |
| pub catalog_provider: |
| names.into_iter().map(|s| s.into()).collect() | ||
| } | ||
|
|
||
| unsafe extern "C" fn register_catalog_fn_wrapper( |
There was a problem hiding this comment.
| unsafe extern "C" fn register_catalog_fn_wrapper( | |
| unsafe extern "C" fn register_catalog_provider_fn_wrapper( |
| names.into_iter().map(|s| s.into()).collect() | ||
| } | ||
|
|
||
| unsafe extern "C" fn register_catalog_fn_wrapper( |
There was a problem hiding this comment.
Or:
| unsafe extern "C" fn register_catalog_fn_wrapper( | |
| unsafe extern "C" fn register_provider_fn_wrapper( |
|
@martin-g Thank you for the review! Most of those are copy+paste things I didn't catch. I didn't change the recommendations where you were suggesting something that differs from the underlying trait. Maybe we should update the naming there, but honestly feels a bit pedantic. I think the function names here should mirror those in the underlying trait. |
2 participants
Which issue does this PR close?
None
Rationale for this change
This is continuation of work in the FFI crate to expose useful traits. We currently have FFI catalog, schema, and table providers. The next layer up in the heirarchy is the catalog provider list.
What changes are included in this PR?
Are these changes tested?
Yes, included in PR.
Are there any user-facing changes?
This is only new addition to the FFI crate. No existing code is modified except making one data member pub(crate)