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



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/functions.scala
##########
@@ -2841,6 +2841,31 @@ object functions {
   // DateTime functions
   
//////////////////////////////////////////////////////////////////////////////////////////////
 
+  /**
+   * Creates a datetime interval
+   *
+   * @param years Number of years
+   * @param months Number of months
+   * @param weeks Number of weeks
+   * @param days Number of days
+   * @param hours Number of hours
+   * @param mins Number of mins
+   * @param secs Number of secs
+   * @return A datetime interval
+   * @group datetime_funcs
+   * @since 3.2.0
+   */
+  def make_interval(
+      years: Column = lit(0),

Review comment:
       @HyukjinKwon - thank you for the code review.  The default arguments 
allow for this nice syntax:
   
   ```scala
   df.withColumn("plus_2_hours", col("first_datetime") + make_interval(hours = 
lit(2)))
   ```
   
   Without the default arguments, we'll need to invoke the method with a lot of 
arguments:
   
   ```scala
   val twoHourInterval = make_interval(lit(0), lit(0), lit(0), lit(0), lit(2), 
lit(0), lit(0))
   df.withColumn("plus_2_hours", col("first_datetime") + twoHourInterval)
   ```
   
   We could overload the method and allow for both syntaxes (using a little 
trick with the last argument that's optional).
   
   ```scala
   def make_interval(
       years: Column,
       months: Column,
       weeks: Column,
       days: Column,
       hours: Column,
       mins: Column,
       secs: Column,
       failOnError: Boolean): Column = withExpr {
     MakeInterval(
       years.expr, months.expr, weeks.expr, days.expr, hours.expr, mins.expr, 
secs.expr, failOnError)
   }
   ```
   
   This would allow Scala users to either manually list 8 arguments (e.g. 
`make_interval(lit(0), lit(0), lit(0), lit(0), lit(2), lit(0), lit(0), true)` 
or use the clean default syntax (e.g. `make_interval(hours = lit(2))`).  Would 
this give the Java users a usable interface?
   
   I'm also fine removing the default values if that's what's necessary to make 
this method compatible with Java.  @MaxGekk - open to your thoughts too cause I 
know you were originally in favor [of the arguments with default values 
approach](https://issues.apache.org/jira/browse/SPARK-33995?focusedCommentId=17258718&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17258718).




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



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

Reply via email to