New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
COPY-3586: Support COPY from external location(S3) [PATCH-1] #4170
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/databend/databend/4uPkvAjbfdYGD3qzxeUgc9aikWBJ [Deployment for be63b03 canceled] |
Thanks for the contribution! Please review the labels and make any necessary changes. |
let table = self | ||
.ctx | ||
.get_table(&self.plan.db_name, &self.plan.tbl_name) | ||
.await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can verify db_name
and tbl_name
before accessing file in s3? If wrong db_name/tbl_name is given we can fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, I'm fresh on this project -- databend, if some naive comments posted, I'm supposed to offend you guys. Be free to give correct replies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is under Draft status, database
and table
privileges check hasn't finished yet, will add them soon.
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have some recent deadline about this feature? If not, I'd like to write some code for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For COPY INTO TABLE FROM s3 external location
feature, I will finish it this week in this PR :)
Well, would like to recommend some issues here:
https://github.com/datafuselabs/databend/issues?q=is%3Aissue+is%3Aopen+label%3A%22C-good+first+issue%22
This PR should merge with #4203, but we can review it now. |
Ping @BohuTANG, please give me your fork's |
Hello, This feature is not available at the moment(only works with minio), it needs to wait for OpenDAL to finish: Since there are too many files involved, I will merge it first as the PATCH-1. |
]); | ||
|
||
let local = Operator::new( | ||
fs::Backend::build() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-reminder: Implement apache/opendal#30 so that users can test easier.
StageType::External => { | ||
match plan.stage_info.file_format_options.format { | ||
// CSV. | ||
StageFileFormatType::Csv => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we support parquet
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will add the parquet support after the csv works all well :D
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
Summary about this PR:
Remove:
Syntax:
Where
For example:
Changelog
Related Issues
Fixes #3586
Test Plan
Unit Tests
Stateless Tests