bojana-db commented on code in PR #58242:
URL: https://github.com/apache/spark/pull/58242#discussion_r3950949156


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala:
##########
@@ -762,6 +762,142 @@ case class Factorial(child: Expression)
     copy(child = newChild)
 }
 
+@ExpressionDescription(
+  usage = "_FUNC_(expr1, expr2) - Returns the greatest common divisor of 
`expr1` and `expr2`.",
+  arguments = """
+    Arguments:
+      * expr1 - The first value. An expression that evaluates to an integral 
number.
+      * expr2 - The second value. An expression that evaluates to an integral 
number.
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_(24, 36);
+       12
+      > SELECT _FUNC_(-24, 36);
+       12
+      > SELECT _FUNC_(0, 0);
+       0
+  """,
+  since = "4.4.0",
+  group = "math_funcs")
+case class Gcd(
+    left: Expression,
+    right: Expression,
+    ansiEnabled: Boolean = SQLConf.get.ansiEnabled)
+  extends BinaryExpression with ImplicitCastInputTypes with 
SupportQueryContext {
+  override def nullIntolerant: Boolean = true
+
+  def this(left: Expression, right: Expression) =
+    this(left, right, ansiEnabled = SQLConf.get.ansiEnabled)
+
+  override def inputTypes: Seq[DataType] = Seq(LongType, LongType)
+
+  override def dataType: DataType = LongType
+
+  // The result overflows for inputs whose divisor is -Long.MinValue, which is 
null in ANSI mode.
+  override def nullable: Boolean = true
+
+  override def initQueryContext(): Option[QueryContext] = if (ansiEnabled) {
+    Some(origin.context)
+  } else {
+    None
+  }
+
+  protected override def nullSafeEval(left: Any, right: Any): Any = {
+    MathUtils.gcd(
+      left.asInstanceOf[Long], right.asInstanceOf[Long], ansiEnabled, 
getContextOrNull())
+  }
+
+  override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
+    val errorContext = getContextOrNullCode(ctx, ansiEnabled)
+    val result = ctx.freshName("gcd")
+    nullSafeCodeGen(ctx, ev, (leftValue, rightValue) => {
+      s"""
+        java.lang.Long $result = 
org.apache.spark.sql.catalyst.util.MathUtils.gcd(
+          $leftValue, $rightValue, $ansiEnabled, $errorContext);
+        if ($result == null) {
+          ${ev.isNull} = true;
+        } else {
+          ${ev.value} = $result;
+        }
+      """
+    })
+  }
+
+  override def prettyName: String = "gcd"

Review Comment:
   This is redundant. Single word function names are correctly handled by the 
base class. (Same for `lcm`)



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala:
##########
@@ -762,6 +762,142 @@ case class Factorial(child: Expression)
     copy(child = newChild)
 }
 
+@ExpressionDescription(
+  usage = "_FUNC_(expr1, expr2) - Returns the greatest common divisor of 
`expr1` and `expr2`.",
+  arguments = """
+    Arguments:
+      * expr1 - The first value. An expression that evaluates to an integral 
number.
+      * expr2 - The second value. An expression that evaluates to an integral 
number.
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_(24, 36);
+       12
+      > SELECT _FUNC_(-24, 36);
+       12
+      > SELECT _FUNC_(0, 0);
+       0
+  """,
+  since = "4.4.0",
+  group = "math_funcs")
+case class Gcd(
+    left: Expression,
+    right: Expression,
+    ansiEnabled: Boolean = SQLConf.get.ansiEnabled)
+  extends BinaryExpression with ImplicitCastInputTypes with 
SupportQueryContext {
+  override def nullIntolerant: Boolean = true
+
+  def this(left: Expression, right: Expression) =
+    this(left, right, ansiEnabled = SQLConf.get.ansiEnabled)
+
+  override def inputTypes: Seq[DataType] = Seq(LongType, LongType)
+
+  override def dataType: DataType = LongType
+
+  // The result overflows for inputs whose divisor is -Long.MinValue, which is 
null in ANSI mode.

Review Comment:
   ```suggestion
     // The result overflows for inputs whose divisor is -Long.MinValue, which 
is null in non-ANSI mode.
   ```



##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathExpressionsSuite.scala:
##########
@@ -488,6 +488,68 @@ class MathExpressionsSuite extends SparkFunSuite with 
ExpressionEvalHelper {
     checkConsistencyBetweenInterpretedAndCodegen(Factorial.apply _, 
IntegerType)
   }
 
+  test("gcd") {
+    checkEvaluation(Gcd(Literal(24L), Literal(36L)), 12L)
+    checkEvaluation(Gcd(Literal(36L), Literal(24L)), 12L)
+    checkEvaluation(Gcd(Literal(17L), Literal(5L)), 1L)
+    // The result is never negative, whichever inputs are.

Review Comment:
   ```suggestion
       // The result is non-negative regardless of the inputs' signs.
   ```
   (Same for other occurrences.)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to