[GitHub] [spark] sarutak commented on a change in pull request #34593: [SPARK-37324][SQL] Adds support for decimal rounding mode up, down, half_down

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

sarutak commented on a change in pull request #34593:
URL: https://github.com/apache/spark/pull/34593#discussion_r753885615



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala
##########
@@ -1500,26 +1514,37 @@ abstract class RoundBase(child: Expression, scale: Expression,
 }
 
 /**
- * Round an expression to d decimal places using HALF_UP rounding mode.
- * round(2.5) == 3.0, round(3.5) == 4.0.
+ * Round an expression to d decimal places using given rounding mode, default rounding mode HALF_UP.
+ * round(2.5) == 3.0, round(3.5) == 4.0,
+ * round(3.6, 0, 'half_down') == 4.0, round(3.6, 0, 'down') == 3.0.
  */
 // scalastyle:off line.size.limit
 @ExpressionDescription(
-  usage = "_FUNC_(expr, d) - Returns `expr` rounded to `d` decimal places using HALF_UP rounding mode.",
+  usage = "_FUNC_(expr, d) - Returns `expr` rounded to `d` decimal places using given rounding mode.",
   examples = """
     Examples:
       > SELECT _FUNC_(2.5, 0);
        3
+      > SELECT _FUNC_(2.6, 0, 'down');

Review comment:
       The API document for this function is generated from this example so it's great if we have more examples for other modes and negative numbers.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/functions.scala
##########
@@ -2197,13 +2197,23 @@ object functions {
   def round(e: Column): Column = round(e, 0)
 
   /**
-   * Round the value of `e` to `scale` decimal places with HALF_UP round mode
-   * if `scale` is greater than or equal to 0 or at integral part when `scale` is less than 0.
+   * Returns the value of the column `e` rounded to 0 decimal places with HALF_UP round mode.
    *
    * @group math_funcs
    * @since 1.5.0
    */
-  def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
+  def round(e: Column, scale: Int): Column = round(e, scale, "half_up")
+
+  /**
+   * Round the value of `e` to `scale` decimal places with given round mode, default: HALF_UP
+   * if `scale` is greater than or equal to 0 or at integral part when `scale` is less than 0.
+   *
+   * @group math_funcs
+   * @since 3.3.0
+   */
+  def round(e: Column, scale: Int, mode: String): Column = withExpr {

Review comment:
       Is it better for the types of all the arguments to be `Column` for newly added functions?
   Please see the comment at the head of `functions.scala`.




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

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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