MaxGekk commented on issue #23811: [SPARK-26902][SQL] Support java.time.Instant 
as an external type of TimestampType
URL: https://github.com/apache/spark/pull/23811#issuecomment-466998703
 
 
   > I still have a preference for keeping it simple and returning one type, 
being opinionated.
   
   This is what I propose in this PR - return `java.sql.Timestamp` by default 
but an user can ask `java.time.Instant` instead by setting the SQL config 
`spark.sql.catalyst.timestampType` to `Instant`.
   
   > even though this is only a major version upgrade. Right?
   
   If we stick on `Timestamp` value for the SQL config, this won't break 
anything but definitely breaks users apps if we make `Instant` as the default 
value for the config.
   
   ... but if we will brave enough and make `java.time.Instant` as default 
"external" type for Catalyst's `TimestampType`, migration on user side should 
not be so hard - `Timestamp.from(instant)`.
   
   > I'd even be OK with defaulting to instant with this as a safety-valve, to 
push people to better timestamp implementations. The Java 8 class has been out 
for years.
   
   I would prefer this way too but I am just afraid to break user's apps even 
in major version - 3.0.
   
   @cloud-fan Could you look at the PR as well, please.  
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to