rok commented on code in PR #12865:
URL: https://github.com/apache/arrow/pull/12865#discussion_r2208218739
##########
cpp/src/arrow/compute/kernels/temporal_internal.h:
##########
@@ -128,22 +162,31 @@ struct ZonedLocalizer {
template <typename Duration>
struct TimestampFormatter {
const char* format;
- const time_zone* tz;
+ const ArrowTimeZone tz_;
std::ostringstream bufstream;
- explicit TimestampFormatter(const std::string& format, const time_zone* tz,
+ explicit TimestampFormatter(const std::string& format, const ArrowTimeZone
tz,
const std::locale& locale)
- : format(format.c_str()), tz(tz) {
+ : format(format.c_str()), tz_(tz) {
bufstream.imbue(locale);
// Propagate errors as C++ exceptions (to get an actual error message)
bufstream.exceptions(std::ios::failbit | std::ios::badbit);
}
Result<std::string> operator()(int64_t arg) {
bufstream.str("");
- const auto zt = zoned_time<Duration>{tz,
sys_time<Duration>(Duration{arg})};
+ const auto visitor = overloads{
Review Comment:
Would this really make sense to do in light of:
```cpp
const auto visitor = overloads{
[&](const time_zone* tz) {
return zoned_time<Duration>{tz, sys_time<Duration>(Duration{arg})};
},
[&](const OffsetZone tz) {
return zoned_time<Duration, const OffsetZone*>{
&tz, sys_time<Duration>(Duration{arg})};
}};
auto zt = std::visit(visitor, tz_);
arrow_vendored::date::to_stream(bufstream, format, zt);
```
throwing:
```cpp
static assertion failed due to requirement '__all<false>::value':
`std::visit` requires the visitor to have a single return type.
```
I did move `system_time` out of the visitor:
https://github.com/apache/arrow/pull/12865/commits/fb0b4c9caf792a4bc56dad5682abdc3d82763dfd
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
