fix: Generic object store access implementation #2709
Conversation
Generic store access is now simply `Arc<dyn ObjStoreAccess>`. This also fixes BSON and JSON globbing issue. ``` > select * from 'file_*.bson'; ┌─────────┐ │ column1 │ │ ── │ │ Int64 │ ╞═════════╡ │ 1 │ │ 2 │ │ 3 │ └─────────┘ ``` Signed-off-by: Vaibhav <vrongmeal@gmail.com>
| let (source_url, storage_options) = table_location_and_opts(ctx, args, &mut opts)?; | ||
|
|
||
| let url = match source_url { | ||
| DatasourceUrl::File(path) => DatasourceUrl::File(resolve_path(&path)?), | ||
| DatasourceUrl::Url(_) => source_url, | ||
| }; | ||
|
|
||
| let store_access = GenericStoreAccess::new_from_location_and_opts( | ||
| url.to_string().as_str(), | ||
| storage_options, | ||
| )?; | ||
| let store_access = storage_options_into_store_access(&source_url, &storage_options) | ||
| .map_err(ExtensionError::access)?; |
There was a problem hiding this comment.
sort of feels like we should just combine the table_location_and_opts with storage_options_into_store_access into one function as neither is particularly useful without the other.
Also, can we just implement the appropriate From<> for the storage_options_into_store_access error type?
There was a problem hiding this comment.
sort of feels like we should just combine the table_location_and_opts with storage_options_into_store_access into one function as neither is particularly useful without the other.
I don't think so. table_location_and_opts is used only for function implementations.
Also, can we just implement the appropriate From<> for the storage_options_into_store_access error type?
Sure! Will do.
There was a problem hiding this comment.
Also, can we just implement the appropriate From<> for the storage_options_into_store_access error type?
So, we can't really do that because this is a trait method returning ExtensionError (which lies in datafusion_ext crate).
| )) | ||
| })?; | ||
|
|
||
| let access = Arc::new(AzureStoreAccess { |
There was a problem hiding this comment.
seems like we might want a constructor for AzureStoreAccess that parse'd the key and the URI rather than needing callers to do this correctly each time.
|
I'm excited for this to land, as there are a number of tests related to #2729 that we can expand and improve upon. |
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Signed-off-by: Vaibhav <vrongmeal@gmail.com>
Generic store access is now simply
Arc<dyn ObjStoreAccess>.This also fixes BSON and JSON globbing issue.