Support tencent cloud COS storage in datafusion-cli
#9734
Conversation
| @@ -388,7 +411,7 @@ Note that the `endpoint` format of oss needs to be: `https://{bucket}.{oss-regio | |||
| CREATE EXTERNAL TABLE test | |||
| STORED AS PARQUET | |||
| OPTIONS( | |||
| 'service_account_path' '/tmp/gcs.json', | |||
| 'gcp.service_account_path' '/tmp/gcs.json', | |||
There was a problem hiding this comment.
is it related to cos onboard?
There was a problem hiding this comment.
No, add the missing prefix gcp and aws to related external table options.
There was a problem hiding this comment.
Thanks @harveyyue for your contribution.
Please add more details to the PR description, this will makes easier to other people what is the PR without going though the code.
Other than that it looks good, I'll leave it for other reviewers to have a second look.
| @@ -415,6 +424,15 @@ pub(crate) async fn get_object_store( | |||
| let builder = get_oss_object_store_builder(url, options)?; | |||
| Arc::new(builder.build()?) | |||
| } | |||
| "cos" => { | |||
| let Some(options) = table_options.extensions.get::<AwsOptions>() else { | |||
There was a problem hiding this comment.
In follow up we need to refactor this part as for different schemas its too much of the common code
There was a problem hiding this comment.
Yes, agree with you.
There was a problem hiding this comment.
It would be great to file a ticket to do so 🙏
There was a problem hiding this comment.
Looks good to me. Thank you @harveyyue and @comphead for the review
I took the liberty of fixing the clippy CI test and merging up from main and pushed the commits to your branch
| @@ -415,6 +424,15 @@ pub(crate) async fn get_object_store( | |||
| let builder = get_oss_object_store_builder(url, options)?; | |||
| Arc::new(builder.build()?) | |||
| } | |||
| "cos" => { | |||
| let Some(options) = table_options.extensions.get::<AwsOptions>() else { | |||
There was a problem hiding this comment.
It would be great to file a ticket to do so 🙏
alamb
changed the title
datafusion-cli
|
Thanks again @harveyyue and @comphead |
Which issue does this PR close?
NA
Rationale for this change
What changes are included in this PR?
Support
cos://schema as below:Are these changes tested?
yes, unit test
Are there any user-facing changes?
no