-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Feature][ClickhouseFile] Support avro file writer #8361
base: dev
Are you sure you want to change the base?
Conversation
b4e5f4f
to
8f37e37
Compare
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.
Please add test cases and update docs
command.add("\"" + readerOption.getFileFieldsDelimiter() + "\""); | ||
command.add(" --input-format Avro"); |
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.
@caicancai Can it be made optional here? For example, supporting CSV and AVRO, etc
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.
There are many bugs in csv format writing. From my experience, if you want to use csv, you must write a special parse for csv, such as rowconvertTocsv. I am not sure whether you still need to keep csv.
cc @Hisoka-X
@hailin0 I am not familiar with e2e testing. This may take some time. Do I really need to add e2e testing? |
Or upload a test screenshot, you need to prove that the modification is functional |
Purpose of this pull request
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
release-note
.