HyukjinKwon commented on code in PR #59408:
URL: https://github.com/apache/spark/pull/59408#discussion_r4276978909
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala:
##########
@@ -1900,6 +1907,100 @@ abstract class RoundBase(child: Expression, scale:
Expression,
}
}
+/**
+ * Scaled floor/ceil return decimals, including for floating-point inputs.
Keep the original
+ * floating-point value until rounding: an implicit decimal cast can round
across a boundary.
+ */
+abstract class RoundToDecimal(
+ child: Expression,
+ scale: Expression,
+ mode: BigDecimal.RoundingMode.Value,
+ modeStr: String)
+ extends RoundBase(child, scale, mode, modeStr) {
+
+ override def inputTypes: Seq[AbstractDataType] =
+ Seq(TypeCollection(DecimalType, FloatType, DoubleType), IntegerType)
+
+ private lazy val decimalInputType: DecimalType = child.dataType match {
+ case dt: DecimalType => dt
+ case dt => DecimalType.forType(dt)
+ }
+
+ override lazy val dataType: DataType = decimalResultType(decimalInputType)
+
+ private lazy val inputLimit: Double =
+ math.pow(10, decimalInputType.precision - decimalInputType.scale)
+
+ // Called by generated code as well as interpreted evaluation.
+ def roundDecimal(decimal: Decimal): Decimal = {
+ val resultType = dataType.asInstanceOf[DecimalType]
+ val p = resultType.precision
+ val s = resultType.scale
+ if (_scale >= 0) {
+ decimal.toPrecision(p, s, mode, nullOnOverflow = !ansiEnabled,
+ context = getContextOrNull())
+ } else {
+ // Apply the result precision after rounding so ANSI mode also governs
carry overflow.
+ val rounded = Decimal(decimal.toBigDecimal.setScale(_scale, mode))
Review Comment:
**Nit (P3):** `decimal.toBigDecimal.setScale(_scale, mode)` always has a
negative scale here, so `Decimal(...)` goes through `Decimal.set(BigDecimal)`,
which evaluates `SqlApiConf.get.allowNegativeScaleOfDecimalEnabled` for
negative scales. Inside a task, `SQLConf.get` usually has no existing conf, so
each call builds a new `ReadOnlySQLConf` (a settings map, two `ConfigReader`s
and their providers) and does a task-property lookup. That now happens on every
row of `floor(x, -k)` / `ceil(x, -k)`, for Decimal, integral and floating-point
inputs, in both interpreted and generated evaluation. The replaced `Decimal(bd,
p, s)` call only reached `checkNegativeScale(0)`, which never reads the conf.
The rounded value is already integral, so normalizing the scale first is
exact. Results, types and the ANSI error / NULL overflow behavior stay the
same, including with the legacy negative-scale flag enabled:
```suggestion
val rounded = Decimal(decimal.toBigDecimal.setScale(_scale,
mode).setScale(0))
```
**Verification:**
- **Regression:** Existing negative-scale floor/ceil coverage still passes
in both ANSI modes and in interpreted and generated evaluation: Decimal carry
at scale -1, 1e-20 at scales -37/-38/-39, small values at scale -1, and Float
9999999 at scale -1.
- **Compatibility:** With the legacy negative-scale decimal setting enabled,
negative-scale floor/ceil returns the same values and result types as with it
disabled.
- **Inspection:** The negative-scale branch passes a scale-0 BigDecimal to
Decimal, so Decimal.set does not evaluate its negative-scale conf check.
--
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]
