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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to