ORC-1198: Add a new PhysicalFsWriter constructor with FSDataOutputStream parameter
#1153
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@dongjoon-hyun Could you help me to approve this pr? |
Member
|
Thank you for making a PR, @liujiawinds . Sure, I clicked |
| } | ||
| } | ||
|
|
||
| public PhysicalFsWriter(FileSystem fs, |
Member
There was a problem hiding this comment.
Please move this before the original constructor.
| Path path, | ||
| OrcFile.WriterOptions opts, | ||
| WriterEncryptionVariant[] encryption | ||
| ) throws IOException { |
Member
|
It seems that |
Member
|
Also cc @williamhyun because this is for Apache Flink integration. |
Contributor
Author
|
@dongjoon-hyun Do I need to do something about the C++ module error? |
Member
|
No~ Not at all. I can merge your PR after manual review and testing. |
Member
|
It's rerunning now. |
dongjoon-hyun
approved these changes
Jun 10, 2022
Member
|
To help Apache Flink community use Apache ORC more easier, I strategically set the milestone |
dongjoon-hyun
changed the title
PhysicalFsWriter with FSDataOutputStream parameter
dongjoon-hyun
changed the title
PhysicalFsWriter with FSDataOutputStream parameterPhysicalFsWriter constructor with FSDataOutputStream parameter
dongjoon-hyun
pushed a commit
that referenced
this pull request
Jun 10, 2022
…Stream` parameter ### What changes were proposed in this pull request? Add stream parameter constructor for PhysicalFsWriter. ### Why are the changes needed? PhysicalFsWriter implementation works on the basis of a Path, but Flink's bulk writer based on stream. In order to integrate with flink more elegantly, I think a constructor with stream parameter should be added to PhysicalFsWriter ### How was this patch tested? It won't change behavior of PhysicalFsWriter and passed all existed test cases. Closes #1153 from liujiawinds/main. Authored-by: Jia Liu <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 0565bfe) Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun
pushed a commit
that referenced
this pull request
Jun 10, 2022
…Stream` parameter ### What changes were proposed in this pull request? Add stream parameter constructor for PhysicalFsWriter. ### Why are the changes needed? PhysicalFsWriter implementation works on the basis of a Path, but Flink's bulk writer based on stream. In order to integrate with flink more elegantly, I think a constructor with stream parameter should be added to PhysicalFsWriter ### How was this patch tested? It won't change behavior of PhysicalFsWriter and passed all existed test cases. Closes #1153 from liujiawinds/main. Authored-by: Jia Liu <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 0565bfe) Signed-off-by: Dongjoon Hyun <[email protected]>
Member
|
Merged to main/1.8/1.7. Welcome to the Apache ORC community, @liujiawinds . |
cxzl25
pushed a commit
to cxzl25/orc
that referenced
this pull request
Jan 11, 2024
…Stream` parameter ### What changes were proposed in this pull request? Add stream parameter constructor for PhysicalFsWriter. ### Why are the changes needed? PhysicalFsWriter implementation works on the basis of a Path, but Flink's bulk writer based on stream. In order to integrate with flink more elegantly, I think a constructor with stream parameter should be added to PhysicalFsWriter ### How was this patch tested? It won't change behavior of PhysicalFsWriter and passed all existed test cases. Closes apache#1153 from liujiawinds/main. Authored-by: Jia Liu <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Add stream parameter constructor for PhysicalFsWriter.
Why are the changes needed?
PhysicalFsWriter implementation works on the basis of a Path, but Flink's bulk writer based on stream.
In order to integrate with flink more elegantly, I think a constructor with stream parameter should be added to PhysicalFsWriter
How was this patch tested?
It won't change behavior of PhysicalFsWriter and passed all existed test cases.