Github user BryanCutler commented on the issue:
https://github.com/apache/spark/pull/18664
I merged your changes @ueshin , but having timezone as an Option this way
makes me a little nervous. It will be easy for people to omit it and in doing
so won't cause an immediate failure, but lead to small discrepancies that might
not be obvious. For instance, I think `ArrowWriter.create(schema: StructType)`
probably needs an Option
[here](https://github.com/apache/spark/pull/18664/files#diff-c7968011d98750cc4065f5b37269ca8eL33)
but it wouldn't cause any failures, it would just use
`DateTimeUtils.defaultTimeZone()` instead.
I think if we are going to use `SESSION_LOCAL_TIMEZONE` in Arrow timestamp
data, then it should be more heavily enforced to not allow anything else. I
have an alternative to your changes that will throw an exception if a
`TimestampType` is used for Arrow conversions without first attaching the
session local time zone. It goes about it a little differently, so please take
a look and if you still prefer using Options, then I can revert it, thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
