You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/01/05 18:55:50 UTC

[GitHub] [spark] MrPowers commented on a change in pull request #31000: [SPARK-33975][SQL] Include add_hours function

MrPowers commented on a change in pull request #31000:
URL: https://github.com/apache/spark/pull/31000#discussion_r552128725



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -1445,6 +1445,40 @@ case class ToUTCTimestamp(left: Expression, right: Expression) extends UTCTimest
   override val prettyName: String = "to_utc_timestamp"
 }
 
+
+
+case class AddHours(startTime: Expression, numHours: Expression, timeZoneId: Option[String] = None)
+  extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant
+    with TimeZoneAwareExpression {

Review comment:
       I read the scala-style-guide and wasn't able to decipher the best practice for this specific scenario.  I opened [a scala-style-guide issue to clarify](https://github.com/databricks/scala-style-guide/issues/81).  Do you think this is better?
   
   ```scala
     extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant
     with TimeZoneAwareExpression {
   ```
   
   I'm glad you made the comment because it's really important to manage whitespace consistently throughout the codebase.




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



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