[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303252258 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +580,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => Review comment: Thanks, @mgaido91 ! 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303230689 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +580,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => Review comment: I also agreed to that. Could you give us the pointer to that PR(or some example)? Actually, I tried to reproduce that kind of issue within the scope of this PR. Until now, I didn't succeed. Maybe, we had better a test case for that to make it sure. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303215164 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"${ev.value} = $c == Double.NEGATIVE_INFINITY ? Double.NEGATIVE_INFINITY : " + Review comment: Ya. I'm worrying the same situation. $c is not safe sometime. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303215260 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,32 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"""${ev.value} = \"$c\" == \"Double.NEGATIVE_INFINITY\" ? """ + Review comment: Did you check the generate code manually? 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303215164 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"${ev.value} = $c == Double.NEGATIVE_INFINITY ? Double.NEGATIVE_INFINITY : " + Review comment: Ya. I'm worrying the same situation. $c is not safe in general. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303177402 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"${ev.value} = $c == Double.NEGATIVE_INFINITY ? Double.NEGATIVE_INFINITY : " + Review comment: ~Hmm. This still might fail in some case. `($c)` will be better?~ Let me think about this more. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303177402 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"${ev.value} = $c == Double.NEGATIVE_INFINITY ? Double.NEGATIVE_INFINITY : " + Review comment: ~Hmm. This still might fail in some case. `($c)` will be better?~ Let me think about this. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303177402 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,31 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"${ev.value} = $c == Double.NEGATIVE_INFINITY ? Double.NEGATIVE_INFINITY : " + Review comment: Hmm. This still might fail in some case. `($c)` will be better? 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r303091953 ## File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala ## @@ -199,6 +199,12 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { checkConsistencyBetweenInterpretedAndCodegen(Sinh, DoubleType) } + test("asinh") { +testUnary(Asinh, (x: Double) => math.log(x + math.sqrt(x * x + 1.0))) +checkConsistencyBetweenInterpretedAndCodegen(Asinh, DoubleType) +checkEvaluation(Asinh(Double.NegativeInfinity), Double.NegativeInfinity) Review comment: +1 for @mgaido91 's advice. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302839725 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,32 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + """, + since = "3.0.0") +case class Asinh(child: Expression) + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { + override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { +defineCodeGen(ctx, ev, c => + s"""${ev.value} = \"$c\" == \"Double.NEGATIVE_INFINITY\" ? """ + Review comment: @Tonix517 . This will fail like the following. ``` spark-sql> CREATE TABLE i AS SELECT double('-Infinity') a; spark-sql> SELECT asinh(a) FROM i; NaN ``` `codegen` part is difficult for new developers. You need to check the generated java code by your PR manually. For example, the following is the generated code by this PR. ``` /* 035 */ project_value_0 = project_value_0 = "inputadapter_value_0" == "Double.NEGATIVE_INFINITY" ? java.lang.Double.NEGATIVE_INFINITY : java.lang.Math.log(inputadapter_value_0 + java.lang.Math.sqrt(inputadapter_value_0 * inputadapter_value_0 + 1.0));; ``` 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302793243 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -596,10 +596,14 @@ case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH" """, since = "3.0.0") case class Asinh(child: Expression) - extends UnaryMathExpression((x: Double) => math.log(x + math.sqrt(x * x + 1.0)), "ASINH") { + extends UnaryMathExpression((x: Double) => x match { +case Double.NegativeInfinity => Double.NegativeInfinity +case _ => math.log(x + math.sqrt(x * x + 1.0)) }, "ASINH") { override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { -defineCodeGen(ctx, ev, c => - s"${ev.value} = java.lang.Math.log($c + java.lang.Math.sqrt($c * $c + 1.0));") +defineCodeGen(ctx, ev, c => c match { + case "Double.NEGATIVE_INFINITY" => s"${ev.value} = java.lang.Double.NEGATIVE_INFINITY;" Review comment: Ur, this looks wrong to me. You need to handle them inside the generated Java code. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302729675 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -287,6 +287,30 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS") """) case class Cosh(child: Expression) extends UnaryMathExpression(math.cosh, "COSH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic cosine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(1); + 0.0 + > SELECT _FUNC_(0); + NaN Review comment: This one returns `NaN` while the scope of SPARK-27923 is `Spark SQL is NULL`. `NaN` is different from `NULL` semantically. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302335063 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,28 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 Review comment: The following is different because PostgreSQL returns `-Infinity`. ``` spark-sql> SELECT asinh(double('-Infinity')); NaN Time taken: 0.025 seconds, Fetched 1 row(s) ``` For `Infinity` and `NaN`, Spark and PostgreSQL returns the same result. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302334959 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -287,6 +287,30 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS") """) case class Cosh(child: Expression) extends UnaryMathExpression(math.cosh, "COSH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic cosine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(1); + 0.0 + > SELECT _FUNC_(0); + NaN Review comment: Nope~ I'm doing the corner case checking to identify the difference. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302335063 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -557,6 +581,28 @@ case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN") """) case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic sine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 Review comment: The following is different because PostgreSQL returns `-Infinity`. ``` spark-sql> SELECT asinh(double('-Infinity')); NaN Time taken: 0.025 seconds, Fetched 1 row(s) ``` 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302332704 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -287,6 +287,30 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS") """) case class Cosh(child: Expression) extends UnaryMathExpression(math.cosh, "COSH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic cosine of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(1); + 0.0 + > SELECT _FUNC_(0); + NaN Review comment: This is the difference from PostgreSQL. ``` postgres=# SELECT acosh(0); ERROR: input is out of range ``` 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r302332758 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ## @@ -617,6 +663,30 @@ case class Cot(child: Expression) """) case class Tanh(child: Expression) extends UnaryMathExpression(math.tanh, "TANH") +@ExpressionDescription( + usage = """ +_FUNC_(expr) - Returns inverse hyperbolic tangent of `expr`. + """, + arguments = """ +Arguments: + * expr - hyperbolic angle + """, + examples = """ +Examples: + > SELECT _FUNC_(0); + 0.0 + > SELECT _FUNC_(2); + NaN Review comment: ``` postgres=# SELECT atanh(2); ERROR: input is out of range ``` 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r300175622 ## File path: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala ## @@ -244,6 +254,11 @@ class MathExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { checkConsistencyBetweenInterpretedAndCodegen(Tanh, DoubleType) } +test("atanh") { Review comment: indentation? 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r300130806 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala ## @@ -263,11 +264,13 @@ object FunctionRegistry { expression[Signum]("signum"), expression[Sin]("sin"), expression[Sinh]("sinh"), +expression[Asinh]("asinh"), expression[StringToMap]("str_to_map"), expression[Sqrt]("sqrt"), expression[Tan]("tan"), expression[Cot]("cot"), expression[Tanh]("tanh"), +expression[Atanh]("atanh"), Review comment: Move this to line 228 (below `Atan2`). 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r300130600 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala ## @@ -263,11 +264,13 @@ object FunctionRegistry { expression[Signum]("signum"), expression[Sin]("sin"), expression[Sinh]("sinh"), +expression[Asinh]("asinh"), Review comment: Move this to line 225 (below `Asin`). 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r300129816 ## File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala ## @@ -232,6 +232,7 @@ object FunctionRegistry { expression[Ceil]("ceiling"), expression[Cos]("cos"), expression[Cosh]("cosh"), +expression[Acosh]("acosh"), Review comment: Please move this to line 225 (below `Acos`). 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL
dongjoon-hyun commented on a change in pull request #25041: [SPARK-28133][SQL] Add acosh/asinh/atanh functions to SQL URL: https://github.com/apache/spark/pull/25041#discussion_r300129056 ## File path: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ## @@ -1559,6 +1559,15 @@ object functions { */ def cosh(columnName: String): Column = cosh(Column(columnName)) + /** +* @param e hyperbolic angle +* @return inverse hyperbolic sine of the given value +* +* @group math_funcs +* @since 3.0.0 +*/ + def acosh(e: Column): Column = withExpr { Acosh(e.expr) } Review comment: Got it. Thank you for the clean ups. 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 With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org