[GitHub] [spark] zsxwing commented on a change in pull request #24619: [SPARK-27735][SS] Parsing interval string should be case-insensitive in SS

[GitBox <gi...@apache.org>]

zsxwing commented on a change in pull request #24619: [SPARK-27735][SS] Parsing interval string should be case-insensitive in SS
URL: https://github.com/apache/spark/pull/24619#discussion_r284825671
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
 ##########
 @@ -695,8 +695,14 @@ class Dataset[T] private[sql](
   // defined on a derived column cannot referenced elsewhere in the plan.
   def withWatermark(eventTime: String, delayThreshold: String): Dataset[T] = withTypedPlan {
     val parsedDelay =
-      Option(CalendarInterval.fromString("interval " + delayThreshold))
-        .getOrElse(throw new AnalysisException(s"Unable to parse time delay '$delayThreshold'"))
+      try {
+        CalendarInterval.fromCaseInsensitiveString(delayThreshold)
+      } catch {
+        case e: IllegalArgumentException =>
+          throw new AnalysisException(
+            s"Unable to parse time delay '$delayThreshold'",
+            cause = Some(e))
 
 Review comment:
   > I'm wondering if we need to propagate `cause` here because previously we didn't.
   
   Previously it was just a `null` return value and there was no `cause`. Since we now get a `cause`, it's better to propagate it so that the error will be clearer.
   
   > And, for the other instance in this PR, the exception messages are not the same.
   
   This is something we need to pay to unify all usages. At least, the exception type doesn't change.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org