You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by GitBox <gi...@apache.org> on 2022/06/10 15:46:51 UTC
[GitHub] [orc] liujiawinds opened a new pull request, #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
liujiawinds opened a new pull request, #1153:
URL: https://github.com/apache/orc/pull/1153
### 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on a diff in pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on code in PR #1153:
URL: https://github.com/apache/orc/pull/1153#discussion_r894741742
##########
java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java:
##########
@@ -129,6 +124,18 @@ public PhysicalFsWriter(FileSystem fs,
}
}
+ public PhysicalFsWriter(FileSystem fs,
+ Path path,
+ OrcFile.WriterOptions opts,
+ WriterEncryptionVariant[] encryption
+ ) throws IOException {
Review Comment:
@dongjoon-hyun done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on a diff in pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on code in PR #1153:
URL: https://github.com/apache/orc/pull/1153#discussion_r894741294
##########
java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java:
##########
@@ -129,6 +124,18 @@ public PhysicalFsWriter(FileSystem fs,
}
}
+ public PhysicalFsWriter(FileSystem fs,
Review Comment:
@dongjoon-hyun done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152535292
It seems that `zlib` library build fails in C++ module. This is irrelevant to your PR.
```
CMake Error at zlib_ep-stamp/zlib_ep-download-RELWITHDEBINFO.cmake:49 (message):
Command failed: 1
'/usr/local/bin/cmake' '-Dmake=' '-Dconfig=' '-P' '/home/runner/work/orc/orc/build/zlib_ep-prefix/src/zlib_ep-stamp/zlib_ep-download-RELWITHDEBINFO-impl.cmake'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152508194
@dongjoon-hyun Could you help me to approve this pr?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152525406
Thank you for making a PR, @liujiawinds . Sure, I clicked `Approve to run`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152570467
No~ Not at all. I can merge your PR after manual review and testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152659434
To help Apache Flink community use Apache ORC more easier, I strategically set the milestone `v1.7.5` for this additional constructor patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152540042
Also cc @williamhyun because this is for Apache Flink integration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152644319
It's rerunning now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #1153: ORC-1198: Add a new `PhysicalFsWriter` constructor with `FSDataOutputStream` parameter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152673234
Merged to main/1.8/1.7. Welcome to the Apache ORC community, @liujiawinds .
I assigned ORC-1198 to you (sonice_lj).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1153:
URL: https://github.com/apache/orc/pull/1153#discussion_r894697789
##########
java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java:
##########
@@ -129,6 +124,18 @@ public PhysicalFsWriter(FileSystem fs,
}
}
+ public PhysicalFsWriter(FileSystem fs,
Review Comment:
Please move this before the original constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun closed pull request #1153: ORC-1198: Add a new `PhysicalFsWriter` constructor with `FSDataOutputStream` parameter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #1153: ORC-1198: Add a new `PhysicalFsWriter` constructor with `FSDataOutputStream` parameter
URL: https://github.com/apache/orc/pull/1153
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1153:
URL: https://github.com/apache/orc/pull/1153#discussion_r894702111
##########
java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java:
##########
@@ -129,6 +124,18 @@ public PhysicalFsWriter(FileSystem fs,
}
}
+ public PhysicalFsWriter(FileSystem fs,
+ Path path,
+ OrcFile.WriterOptions opts,
+ WriterEncryptionVariant[] encryption
+ ) throws IOException {
Review Comment:
indentation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a diff in pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on code in PR #1153:
URL: https://github.com/apache/orc/pull/1153#discussion_r894697789
##########
java/core/src/java/org/apache/orc/impl/PhysicalFsWriter.java:
##########
@@ -129,6 +124,18 @@ public PhysicalFsWriter(FileSystem fs,
}
}
+ public PhysicalFsWriter(FileSystem fs,
Review Comment:
Please add this before the original constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] liujiawinds commented on pull request #1153: ORC-1198: Add stream parameter constructor for PhysicalFsWriter
Posted by GitBox <gi...@apache.org>.
liujiawinds commented on PR #1153:
URL: https://github.com/apache/orc/pull/1153#issuecomment-1152569752
@dongjoon-hyun Do I need to do something about the C++ module error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org