wgtmac commented on code in PR #1893:
URL: https://github.com/apache/orc/pull/1893#discussion_r1566617237
##########
c++/test/TestType.cc:
##########
@@ -490,4 +490,28 @@ namespace orc {
*(footer.add_types()) = intType;
checkProtoTypes(footer);
}
+
+ TEST(TestType, testExistSubType) {
Review Comment:
Could you also add a roundtrip test to write and read ORC file without
timestamp type with an invalid TZDIR env set? Something like we did in
`TEST(TestTimezone, testMissingTZDB)` and be careful about the timezone cache.
##########
c++/src/Reader.cc:
##########
@@ -1089,14 +1089,9 @@ namespace orc {
} while (sargsApplier_ && currentStripe_ < lastStripe_);
if (currentStripe_ < lastStripe_) {
- // get writer timezone info from stripe footer to help understand
timestamp values.
- const Timezone& writerTimezone =
- currentStripeFooter_.has_writer_timezone()
- ? getTimezoneByName(currentStripeFooter_.writer_timezone())
- : localTimezone_;
StripeStreamsImpl stripeStreams(*this, currentStripe_,
currentStripeInfo_,
currentStripeFooter_,
currentStripeInfo_.offset(),
- *contents_->stream, writerTimezone,
readerTimezone_);
+ *contents_->stream, readerTimezone_);
Review Comment:
We should also fix here:
https://github.com/apache/orc/blob/main/c%2B%2B/src/Reader.cc#L249-L257
It blindly gets timezone when creating a RowReader.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
