You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/08/24 16:49:17 UTC
[GitHub] [arrow] pitrou opened a new pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
pitrou opened a new pull request #10988:
URL: https://github.com/apache/arrow/pull/10988
The `arrow_vendored::date` library represents year numbers as a C short
and may silently wraparound its value
(but also throw an exception if the year has the value -32768).
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-908503085
@jorisvandenbossche Would you like to review some C++ code?
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052963
@jorisvandenbossche Yes, see https://issues.apache.org/jira/browse/ARROW-13738
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-908503085
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052240
I didn't yet look at the code, but one note: the `strftime` compute kernel has the same problem (at least for timestamp type, since it doesn't yet support date32/64 formatting. Checking the limits encoded in this PR for timestamp, using one out of range crashes with `strftime`).
That can certainly be for another JIRA, but wondering a bit if it might be possible to share some of those checks (the printing should also work if compute is not available I suppose, so cannot directly rely on it)
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052240
I didn't yet look at the code, but one note: the `strftime` compute kernel has the same problem (at least for timestamp type, since it doesn't yet support date32/64 formatting. Checking the limits encoded in this PR for timestamp, using one out of range crashes with `strftime`).
That can certainly be for another JIRA, but wondering a bit if it might be possible to share some of those checks (the printing should also work if compute is not available I suppose, so cannot directly rely on it)
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#discussion_r699104892
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
Hmm... nice idea, but it unfortunately overflows for nanoseconds.
```
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc: In instantiation of 'void arrow::{anonymous}::ArrayPrinter::FormatDateTime(const char*, int64_t, bool) [with Unit = std::chrono::duration<long int, std::ratio<1, 1000000000> >; int64_t = long int]':
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:505:71: required from here
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:483:59: in 'constexpr' expansion of 'std::chrono::duration_cast<std::chrono::duration<long int, std::ratio<1, 1000000000> >, int, std::ratio<86400, 1> >(std::chrono::duration<int, std::ratio<86400, 1> >(-12687428))'
/usr/include/c++/9/chrono:200:21: in 'constexpr' expansion of 'std::chrono::__duration_cast_impl<std::chrono::duration<long int, std::ratio<1, 1000000000> >, std::ratio<86400000000000, 1>, long int, false, true>::__cast<int, std::ratio<86400, 1> >((* & __d))'
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:483:20: error: overflow in constant expression [-fpermissive]
483 | constexpr Unit kMin = std::chrono::duration_cast<Unit>(arrow_vendored::date::days{-12687428});
| ^~~~
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#discussion_r699116150
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
Ok, it works by templating a bit differently.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou edited a comment on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou edited a comment on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052963
@jorisvandenbossche Yes, see https://issues.apache.org/jira/browse/ARROW-13738 and also https://issues.apache.org/jira/browse/ARROW-13736
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-910351226
I will merge this, feel free to post any comments afterwards, though.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #10988:
URL: https://github.com/apache/arrow/pull/10988
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052240
I didn't yet look at the code, but one note: the `strftime` compute kernel has the same problem (at least for timestamp type, since it doesn't yet support date32/64 formatting. Checking the limits encoded in this PR for timestamp, using one out of range crashes with `strftime`).
That can certainly be for another JIRA, but wondering a bit if it might be possible to share some of those checks (the printing should also work if compute is not available I suppose, so cannot directly rely on it)
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#discussion_r698674002
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
The range check could possibly even just be here:
```suggestion
constexpr Unit kMin = duration_cast<Unit>(kMinDay);
constexpr Unit kMax = duration_cast<Unit>(kMaxDay);
if (Unit{value} >= kMin && Unit{value} <= kMax) {
(*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
} else {
WriteOutOfRange(value);
}
```
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -167,29 +165,81 @@ class ArrayPrinter : public PrettyPrinter {
}
template <typename T>
- enable_if_date<typename T::TypeClass, Status> WriteDataValues(const T& array) {
+ enable_if_time<typename T::TypeClass, Status> WriteDataValues(const T& array) {
const auto data = array.raw_values();
- using unit = typename std::conditional<std::is_same<T, Date32Array>::value,
- arrow_vendored::date::days,
- std::chrono::milliseconds>::type;
- WriteValues(array, [&](int64_t i) { FormatDateTime<unit>("%F", data[i], true); });
+ const auto& type = checked_cast<const TimeType&>(*array.type());
+ WriteValues(array,
+ [&](int64_t i) { FormatDateTime(type.unit(), "%T", data[i], false); });
return Status::OK();
}
- template <typename T>
- enable_if_time<typename T::TypeClass, Status> WriteDataValues(const T& array) {
+ // NOTE about bounds checking on temporal data:
+ //
+ // While we catch exceptions in FormatDateTime(), some out-of-bound values
+ // would result in successful but erroneous printing because of silent
+ // integer wraparound in the `arrow_vendored::date` library (particularly
+ // because it represents year values as a C `short` internally).
+ //
+ // To avoid such misprinting, we must therefore check the bounds explicitly.
+ // The bounds correspond to start of year -32767 and end of year 32767,
+ // respectively (-32768 is an invalid year value in `arrow_vendored::date`).
+ Status WriteDataValues(const Date32Array& array) {
const auto data = array.raw_values();
- const auto type = static_cast<const TimeType*>(array.type().get());
- WriteValues(array,
- [&](int64_t i) { FormatDateTime(type->unit(), "%T", data[i], false); });
+ WriteValues(array, [&](int64_t i) {
+ const int32_t v = data[i];
+ if (v >= -12687428 && v <= 11248737) {
Review comment:
Nit: please extract named constants like
```suggestion
if (v >= kMinimumDay && v <= kMaximumDay) {
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#discussion_r698674002
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
The range check could possibly even just be here:
```suggestion
constexpr Unit kMin = duration_cast<Unit>(kMinDay);
constexpr Unit kMax = duration_cast<Unit>(kMaxDay);
if (Unit{value} >= kMin && Unit{value} <= kMax) {
(*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
} else {
WriteOutOfRange(value);
}
```
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -167,29 +165,81 @@ class ArrayPrinter : public PrettyPrinter {
}
template <typename T>
- enable_if_date<typename T::TypeClass, Status> WriteDataValues(const T& array) {
+ enable_if_time<typename T::TypeClass, Status> WriteDataValues(const T& array) {
const auto data = array.raw_values();
- using unit = typename std::conditional<std::is_same<T, Date32Array>::value,
- arrow_vendored::date::days,
- std::chrono::milliseconds>::type;
- WriteValues(array, [&](int64_t i) { FormatDateTime<unit>("%F", data[i], true); });
+ const auto& type = checked_cast<const TimeType&>(*array.type());
+ WriteValues(array,
+ [&](int64_t i) { FormatDateTime(type.unit(), "%T", data[i], false); });
return Status::OK();
}
- template <typename T>
- enable_if_time<typename T::TypeClass, Status> WriteDataValues(const T& array) {
+ // NOTE about bounds checking on temporal data:
+ //
+ // While we catch exceptions in FormatDateTime(), some out-of-bound values
+ // would result in successful but erroneous printing because of silent
+ // integer wraparound in the `arrow_vendored::date` library (particularly
+ // because it represents year values as a C `short` internally).
+ //
+ // To avoid such misprinting, we must therefore check the bounds explicitly.
+ // The bounds correspond to start of year -32767 and end of year 32767,
+ // respectively (-32768 is an invalid year value in `arrow_vendored::date`).
+ Status WriteDataValues(const Date32Array& array) {
const auto data = array.raw_values();
- const auto type = static_cast<const TimeType*>(array.type().get());
- WriteValues(array,
- [&](int64_t i) { FormatDateTime(type->unit(), "%T", data[i], false); });
+ WriteValues(array, [&](int64_t i) {
+ const int32_t v = data[i];
+ if (v >= -12687428 && v <= 11248737) {
Review comment:
Nit: please extract named constants like
```suggestion
if (v >= kMinimumDay && v <= kMaximumDay) {
```
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou edited a comment on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou edited a comment on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052963
@jorisvandenbossche Yes, see https://issues.apache.org/jira/browse/ARROW-13738 and also https://issues.apache.org/jira/browse/ARROW-13736
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-904810084
https://issues.apache.org/jira/browse/ARROW-12011
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#discussion_r699104892
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
Hmm... nice idea, but it unfortunately overflows for nanoseconds.
```
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc: In instantiation of 'void arrow::{anonymous}::ArrayPrinter::FormatDateTime(const char*, int64_t, bool) [with Unit = std::chrono::duration<long int, std::ratio<1, 1000000000> >; int64_t = long int]':
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:505:71: required from here
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:483:59: in 'constexpr' expansion of 'std::chrono::duration_cast<std::chrono::duration<long int, std::ratio<1, 1000000000> >, int, std::ratio<86400, 1> >(std::chrono::duration<int, std::ratio<86400, 1> >(-12687428))'
/usr/include/c++/9/chrono:200:21: in 'constexpr' expansion of 'std::chrono::__duration_cast_impl<std::chrono::duration<long int, std::ratio<1, 1000000000> >, std::ratio<86400000000000, 1>, long int, false, true>::__cast<int, std::ratio<86400, 1> >((* & __d))'
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:483:20: error: overflow in constant expression [-fpermissive]
483 | constexpr Unit kMin = std::chrono::duration_cast<Unit>(arrow_vendored::date::days{-12687428});
| ^~~~
```
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
Ok, it works by templating a bit differently.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052963
@jorisvandenbossche Yes, see https://issues.apache.org/jira/browse/ARROW-13738
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#discussion_r699104892
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
Hmm... nice idea, but it unfortunately overflows for nanoseconds.
```
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc: In instantiation of 'void arrow::{anonymous}::ArrayPrinter::FormatDateTime(const char*, int64_t, bool) [with Unit = std::chrono::duration<long int, std::ratio<1, 1000000000> >; int64_t = long int]':
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:505:71: required from here
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:483:59: in 'constexpr' expansion of 'std::chrono::duration_cast<std::chrono::duration<long int, std::ratio<1, 1000000000> >, int, std::ratio<86400, 1> >(std::chrono::duration<int, std::ratio<86400, 1> >(-12687428))'
/usr/include/c++/9/chrono:200:21: in 'constexpr' expansion of 'std::chrono::__duration_cast_impl<std::chrono::duration<long int, std::ratio<1, 1000000000> >, std::ratio<86400000000000, 1>, long int, false, true>::__cast<int, std::ratio<86400, 1> >((* & __d))'
/home/antoine/arrow/dev/cpp/src/arrow/pretty_print.cc:483:20: error: overflow in constant expression [-fpermissive]
483 | constexpr Unit kMin = std::chrono::duration_cast<Unit>(arrow_vendored::date::days{-12687428});
| ^~~~
```
##########
File path: cpp/src/arrow/pretty_print.cc
##########
@@ -421,10 +471,14 @@ class ArrayPrinter : public PrettyPrinter {
private:
template <typename Unit>
void FormatDateTime(const char* fmt, int64_t value, bool add_epoch) {
- if (add_epoch) {
- (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
- } else {
- (*sink_) << arrow_vendored::date::format(fmt, Unit{value});
+ try {
+ if (add_epoch) {
+ (*sink_) << arrow_vendored::date::format(fmt, epoch_ + Unit{value});
Review comment:
Ok, it works by templating a bit differently.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou edited a comment on pull request #10988: ARROW-12011: [C++] Fix crashes and incorrect results when printing extreme date values
Posted by GitBox <gi...@apache.org>.
pitrou edited a comment on pull request #10988:
URL: https://github.com/apache/arrow/pull/10988#issuecomment-909052963
@jorisvandenbossche Yes, see https://issues.apache.org/jira/browse/ARROW-13738 and also https://issues.apache.org/jira/browse/ARROW-13736
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org