MaxGekk commented on code in PR #48004:
URL: https://github.com/apache/spark/pull/48004#discussion_r1757527270


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/randomExpressions.scala:
##########
@@ -181,3 +187,239 @@ case class Randn(child: Expression, hideSeed: Boolean = 
false) extends RDG {
 object Randn {
   def apply(seed: Long): Randn = Randn(Literal(seed, LongType))
 }
+
+@ExpressionDescription(
+  usage = """
+    _FUNC_(min, max[, seed]) - Returns a random value with independent and 
identically
+      distributed (i.i.d.) values with the specified range of numbers. The 
random seed is optional.
+      The provided numbers specifying the minimum and maximum values of the 
range must be constant.
+      If both of these numbers are integers, then the result will also be an 
integer. Otherwise if
+      one or both of these are floating-point numbers, then the result will 
also be a floating-point
+      number.
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_(10, 20) > 0 AS result;
+      true
+  """,
+  since = "4.0.0",
+  group = "math_funcs")
+object UniformExpressionBuilder extends ExpressionBuilder {
+  override def build(funcName: String, expressions: Seq[Expression]): 
Expression = {
+    val numArgs = expressions.length

Review Comment:
   nit: could you move it into the default case



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/randomExpressions.scala:
##########
@@ -181,3 +187,239 @@ case class Randn(child: Expression, hideSeed: Boolean = 
false) extends RDG {
 object Randn {
   def apply(seed: Long): Randn = Randn(Literal(seed, LongType))
 }
+
+@ExpressionDescription(
+  usage = """
+    _FUNC_(min, max[, seed]) - Returns a random value with independent and 
identically
+      distributed (i.i.d.) values with the specified range of numbers. The 
random seed is optional.
+      The provided numbers specifying the minimum and maximum values of the 
range must be constant.
+      If both of these numbers are integers, then the result will also be an 
integer. Otherwise if
+      one or both of these are floating-point numbers, then the result will 
also be a floating-point
+      number.
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_(10, 20) > 0 AS result;
+      true
+  """,
+  since = "4.0.0",
+  group = "math_funcs")
+object UniformExpressionBuilder extends ExpressionBuilder {
+  override def build(funcName: String, expressions: Seq[Expression]): 
Expression = {
+    val numArgs = expressions.length
+    expressions match {
+      case Seq(min, max) =>
+        Uniform(min, max)
+      case Seq(min, max, seed) =>
+        Uniform(min, max, seed)
+      case _ =>
+        throw QueryCompilationErrors.wrongNumArgsError(funcName, Seq(2, 3), 
numArgs)
+    }
+  }
+}
+
+case class Uniform(min: Expression, max: Expression)
+  extends RuntimeReplaceable with BinaryLike[Expression] with 
ExpressionWithRandomSeed {
+
+  private var seed: Expression = Literal(Uniform.random.nextLong(), LongType)
+  final override lazy val deterministic: Boolean = false
+  override val nodePatterns: Seq[TreePattern] =
+    Seq(RUNTIME_REPLACEABLE, EXPRESSION_WITH_RANDOM_SEED)
+
+  override val dataType: DataType = {
+    val first = min.dataType
+    val second = max.dataType
+    (min.dataType, max.dataType) match {
+      case _ if !valid(min) || !valid(max) => NullType
+      case (_, LongType) | (LongType, _) if Seq(first, second).forall(integer) 
=> LongType
+      case (_, IntegerType) | (IntegerType, _) if Seq(first, 
second).forall(integer) => IntegerType
+      case (_, ShortType) | (ShortType, _) if Seq(first, 
second).forall(integer) => ShortType
+      case (_, DoubleType) | (DoubleType, _) => DoubleType
+      case (_, FloatType) | (FloatType, _) => FloatType
+      case _ => NullType
+    }
+  }
+
+  private def valid(e: Expression): Boolean = e.dataType match {
+    case _: ShortType | _: IntegerType | _: LongType | _: FloatType | _: 
DoubleType => true
+    case _ => false
+  }
+
+  private def integer(t: DataType): Boolean = t match {
+    case _: ShortType | _: IntegerType | _: LongType => true
+    case _ => false
+  }
+
+  override def checkInputDataTypes(): TypeCheckResult = {
+    var result: TypeCheckResult = TypeCheckResult.TypeCheckSuccess
+    def requiredType = "integer or floating-point"
+    Seq((min, "min", 0),
+      (max, "max", 1),
+      (seed, "seed", 2)).foreach {
+      case (expr: Expression, name: String, index: Int) =>
+        if (!expr.foldable && result == TypeCheckResult.TypeCheckSuccess) {
+          result = DataTypeMismatch(
+            errorSubClass = "NON_FOLDABLE_INPUT",
+            messageParameters = Map(
+              "inputName" -> name,
+              "inputType" -> requiredType,
+              "inputExpr" -> toSQLExpr(expr)))
+        } else if (!valid(expr) && result == TypeCheckResult.TypeCheckSuccess) 
{
+          result = DataTypeMismatch(
+            errorSubClass = "UNEXPECTED_INPUT_TYPE",
+            messageParameters = Map(
+              "paramIndex" -> ordinalNumber(index),
+              "requiredType" -> requiredType,
+              "inputSql" -> toSQLExpr(expr),
+              "inputType" -> toSQLType(expr.dataType)))
+        }
+    }
+    result
+  }
+
+  override def left: Expression = min
+  override def right: Expression = max
+
+  override def seedExpression: Expression = seed
+  override def withNewSeed(newSeed: Long): Expression = {
+    val result = Uniform(min, max)
+    result.seed = Literal(newSeed, LongType)
+    result
+  }
+
+  override def withNewChildrenInternal(newFirst: Expression, newSecond: 
Expression): Expression =
+    Uniform(newFirst, newSecond)
+
+  override def replacement: Expression = {
+    def cast(e: Expression, to: DataType): Expression = if (e.dataType == to) 
e else Cast(e, to)
+    cast(Add(
+      cast(min, DoubleType),
+      Multiply(
+        Subtract(
+          cast(max, DoubleType),
+          cast(min, DoubleType)),
+        Rand(seed))),
+      dataType)
+  }
+}
+
+object Uniform {
+  lazy val random = new Random()

Review Comment:
   Why don't you follow the existing approach for the random expression, and 
don't extend `RDG`?



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/randomExpressions.scala:
##########
@@ -181,3 +187,239 @@ case class Randn(child: Expression, hideSeed: Boolean = 
false) extends RDG {
 object Randn {
   def apply(seed: Long): Randn = Randn(Literal(seed, LongType))
 }
+
+@ExpressionDescription(
+  usage = """
+    _FUNC_(min, max[, seed]) - Returns a random value with independent and 
identically
+      distributed (i.i.d.) values with the specified range of numbers. The 
random seed is optional.
+      The provided numbers specifying the minimum and maximum values of the 
range must be constant.
+      If both of these numbers are integers, then the result will also be an 
integer. Otherwise if
+      one or both of these are floating-point numbers, then the result will 
also be a floating-point
+      number.
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_(10, 20) > 0 AS result;
+      true
+  """,
+  since = "4.0.0",
+  group = "math_funcs")
+object UniformExpressionBuilder extends ExpressionBuilder {
+  override def build(funcName: String, expressions: Seq[Expression]): 
Expression = {
+    val numArgs = expressions.length
+    expressions match {
+      case Seq(min, max) =>
+        Uniform(min, max)
+      case Seq(min, max, seed) =>
+        Uniform(min, max, seed)
+      case _ =>
+        throw QueryCompilationErrors.wrongNumArgsError(funcName, Seq(2, 3), 
numArgs)
+    }
+  }
+}
+
+case class Uniform(min: Expression, max: Expression)
+  extends RuntimeReplaceable with BinaryLike[Expression] with 
ExpressionWithRandomSeed {
+
+  private var seed: Expression = Literal(Uniform.random.nextLong(), LongType)

Review Comment:
   hmm, why do you need `var`. Could you follow the existing approach, and 
define a case class w/ 3 parameters + constructors with 2 params.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/randomExpressions.scala:
##########
@@ -181,3 +187,239 @@ case class Randn(child: Expression, hideSeed: Boolean = 
false) extends RDG {
 object Randn {
   def apply(seed: Long): Randn = Randn(Literal(seed, LongType))
 }
+
+@ExpressionDescription(
+  usage = """
+    _FUNC_(min, max[, seed]) - Returns a random value with independent and 
identically
+      distributed (i.i.d.) values with the specified range of numbers. The 
random seed is optional.
+      The provided numbers specifying the minimum and maximum values of the 
range must be constant.
+      If both of these numbers are integers, then the result will also be an 
integer. Otherwise if
+      one or both of these are floating-point numbers, then the result will 
also be a floating-point
+      number.
+  """,
+  examples = """
+    Examples:
+      > SELECT _FUNC_(10, 20) > 0 AS result;
+      true
+  """,
+  since = "4.0.0",
+  group = "math_funcs")
+object UniformExpressionBuilder extends ExpressionBuilder {

Review Comment:
   Why do you need an expression builder? Can't you just define an expression 
w/ a few constructors? Please, explain 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.

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