xuanyuanking commented on a change in pull request #27537: 
[SPARK-30668][SQL][FOLLOWUP] Raise exception instead of silent change for new 
TimestampFormatter
URL: https://github.com/apache/spark/pull/27537#discussion_r383085952
 
 

 ##########
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
 ##########
 @@ -787,6 +788,10 @@ abstract class ToTimestamp
               formatter.parse(
 
 Review comment:
   > can we create a util function to do it?
   
   The function demo you provided is clearer, but since we need to change both 
sides of code generation and normal mode, the current abstract might be better?
   
   > and do we need to fix more places?
   
   Actually, in this PR, we tracked all the place which used the original 
config `LEGACY_TIME_PARSER_ENABLED ` and do the fix in UT. For every case that 
`legacy` mode have a different result with `corrected` mode, I added an 
explicit check in the UT. (The only exception is 
https://github.com/apache/spark/pull/27537/files#diff-ab881d9b6bceac0a14656dee940917f4R720,
 which use different format pattern in legacy and corrected mode, so I think 
this shouldn't be the cases we want to fix here.)
   cc @MaxGekk for double-checking.
   
   

----------------------------------------------------------------
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.
 
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