dongjoon-hyun commented on code in PR #1587:
URL: https://github.com/apache/orc/pull/1587#discussion_r1298940081
##########
c++/src/Timezone.cc:
##########
@@ -675,6 +675,13 @@ namespace orc {
if (itr != timezoneCache.end()) {
return *(itr->second).get();
}
+ if (!std::filesystem::exists(std::filesystem::path(filename))) {
+ std::stringstream ss;
+ ss << "Time zone file " << filename << " does not exist."
+ << " Please install IANA time zone database and set TZDIR env"
+ << " if it is not installed at /usr/share/zoneinfo";
Review Comment:
Yes, it's not applied to `Windows OS` because the path style is invalid.
This PR might lead a user to try to check the location of invalid path
information by mapping from `/usr/share/zoneinfo` to `C:\\usr\share\zoneinfo`.
Although the user ends up to conclude that the condition statement (`if it is
not installed at /usr/share/zoneinfo`) is `false`, this message might confuse
him by suggesting him to install it into `C:\\usr\share\zoneinfo`.
When the error message happens, I'm wondering if we don't need the message
(`if it is not installed at /usr/share/zoneinfo`) at all.
The first two lines look sufficient to me. WDYT?
```c++
ss << "Time zone file " << filename << " does not exist."
<< " Please install IANA time zone database and set TZDIR env.";
```
--
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]
