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/10/28 07:20:19 UTC
[GitHub] [orc] coderex2522 opened a new pull request, #1299: ORC-1304: [C++] fix the bug that throw ParseError when using SearchArgument with nested struct
coderex2522 opened a new pull request, #1299:
URL: https://github.com/apache/orc/pull/1299
### What changes were proposed in this pull request?
This pr doesn't call readHeader function when the seek location is all zero.
### Why are the changes needed?
When the length of present stream is zero, the seek function pre-call readHeader() function will throw ParseError.
### How was this patch tested?
Add the ByteRle.seekInEmptyPresentStream and RLEv1.seekInEmptyPresentStream UT.
--
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] coderex2522 commented on a diff in pull request #1299: ORC-1304: [C++] Fix bug of seeking over empty PRESENT stream
Posted by GitBox <gi...@apache.org>.
coderex2522 commented on code in PR #1299:
URL: https://github.com/apache/orc/pull/1299#discussion_r1011064465
##########
c++/test/TestPredicatePushdown.cc:
##########
@@ -557,4 +557,103 @@ namespace orc {
TestFirstStripeSelectedWithStripeStats(reader.get(), pos);
}
}
+
+ TEST(TestPredicatePushdown, testSeekOverEmptyPresentStream) {
Review Comment:
I move the ut into the TestReader.cc file because the ParseError exception was not handled during the read operation.
--
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] coderex2522 commented on a diff in pull request #1299: ORC-1304: [C++] Fix bug of seeking over empty PRESENT stream
Posted by GitBox <gi...@apache.org>.
coderex2522 commented on code in PR #1299:
URL: https://github.com/apache/orc/pull/1299#discussion_r1011065161
##########
c++/test/TestWriter.cc:
##########
@@ -2044,103 +2044,5 @@ namespace orc {
testSuppressPresentStream(CompressionKind_SNAPPY);
}
- TEST(WriterTest, testSeekWithNestedType) {
Review Comment:
It just move into TestReadIntent.testSeekOverEmptyPresentStream in the TestReader.cc file.
--
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] wgtmac commented on a diff in pull request #1299: ORC-1304: [C++] Fix bug of seeking over empty PRESENT stream
Posted by GitBox <gi...@apache.org>.
wgtmac commented on code in PR #1299:
URL: https://github.com/apache/orc/pull/1299#discussion_r1010493186
##########
c++/test/TestPredicatePushdown.cc:
##########
@@ -557,4 +557,103 @@ namespace orc {
TestFirstStripeSelectedWithStripeStats(reader.get(), pos);
}
}
+
+ TEST(TestPredicatePushdown, testSeekOverEmptyPresentStream) {
Review Comment:
nit: the bug has nothing to do with predicate pushdown, we'd better move it out of here.
##########
c++/test/TestWriter.cc:
##########
@@ -2044,103 +2044,5 @@ namespace orc {
testSuppressPresentStream(CompressionKind_SNAPPY);
}
- TEST(WriterTest, testSeekWithNestedType) {
Review Comment:
Why is it removed?
--
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 #1299: ORC-1304: [C++] Fix bug of seeking over empty PRESENT stream
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #1299:
URL: https://github.com/apache/orc/pull/1299#issuecomment-1300944963
cc @williamhyun , too
--
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] wgtmac commented on pull request #1299: ORC-1304: [C++] Fix bug of seeking over empty PRESENT stream
Posted by GitBox <gi...@apache.org>.
wgtmac commented on PR #1299:
URL: https://github.com/apache/orc/pull/1299#issuecomment-1299949322
I just committed this. Thanks @coderex2522 @stiga-huang @dongjoon-hyun!
--
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] wgtmac merged pull request #1299: ORC-1304: [C++] Fix bug of seeking over empty PRESENT stream
Posted by GitBox <gi...@apache.org>.
wgtmac merged PR #1299:
URL: https://github.com/apache/orc/pull/1299
--
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] stiga-huang commented on pull request #1299: ORC-1304: [C++] fix the bug that throw ParseError when using SearchArgument with nested struct
Posted by GitBox <gi...@apache.org>.
stiga-huang commented on PR #1299:
URL: https://github.com/apache/orc/pull/1299#issuecomment-1298014776
+1. The fix LGTM. I suggest a more specific commit title, e.g. "ORC-1304: [C++] Fix bug of seeking in empty PRESENT stream".
For the missing e2e tests, we can add them in TestPredicatePushdown.cc.
--
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] coderex2522 commented on pull request #1299: ORC-1304: [C++] fix the bug that throw ParseError when using SearchArgument with nested struct
Posted by GitBox <gi...@apache.org>.
coderex2522 commented on PR #1299:
URL: https://github.com/apache/orc/pull/1299#issuecomment-1298465732
> We need to add e2e test cases to cover at least empty struct/map/list mentioned in the issue.
I add the new UT WriterTest.testSeekWithNestedType to cover the child column of the struct/map/list type has empty present stream.
--
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