westonpace commented on a change in pull request #12279:
URL: https://github.com/apache/arrow/pull/12279#discussion_r797935652
##########
File path: cpp/src/arrow/scalar.h
##########
@@ -345,8 +345,8 @@ struct ARROW_EXPORT TimestampScalar : public
TemporalScalar<TimestampType> {
using TemporalScalar<TimestampType>::TemporalScalar;
TimestampScalar(typename TemporalScalar<TimestampType>::ValueType value,
- TimeUnit::type unit)
- : TimestampScalar(std::move(value), timestamp(unit)) {}
+ TimeUnit::type unit, std::string tz = "")
Review comment:
I agree that the default behavior here would be to assume a naive
timestamp. I just don't agree that it is a sane default. The user is
explicitly creating a literal so they should know which of the two they want
and I don't know how we can possibly make that decision for them (e.g. a naive
timestamp is no more likely to be the right answer than an aware timestamp).
So I think it would be better to leave off the default and force the user to
tell us what they have.
However, I agree with you that it seems my opinion is against precedent so
I'll drop 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
