[SPARK-40945][SQL][PYTHON] Add truncate function - #57476
[SPARK-40945][SQL][PYTHON] Add truncate function#57476SreeramaYeshwanthGowd wants to merge 6 commits into
Conversation
truncate function|
@gengliangwang Would you have a moment to review this when you get a chance? Thank you! |
uros-b
left a comment
There was a problem hiding this comment.
math.sql - no truncate entries were added, whereas round and bround each carry ~30 golden-file cases spanning all integral types (byte/short/int/long) at positive, zero, and negative scale. A committer reviewing a new built-in math function may reasonably ask for the same SQL-level golden coverage for consistency with its siblings. Note this is a parity gap, not a masked bug: the novel ROUND_DOWN path is already unit-tested (MathExpressionsSuite + the generative DecimalSuite test across all round modes), the DataFrame + selectExpr SQL path is exercised in MathFunctionsSuite, and; unlike round; the ANSI integral-overflow boundary cases (round(127y, -1) etc.) are not applicable here, since truncation toward zero can never increase magnitude and so can never overflow.
|
@uros-b Thanks for the review. Added the truncate entries to math.sql and regenerated the golden output to match round and bround. |
|
Noting that the Base image build check is failing with a pull access denied error on docker.io/library/root:latest, unrelated to this PR. |
| SELECT truncate(525L, -1); | ||
| SELECT truncate(525L, -2); | ||
| SELECT truncate(525L, -3); | ||
|
|
There was a problem hiding this comment.
Every new math.sql case uses a positive input. All twenty are 25 or 525. Truncating toward zero instead of toward negative infinity is the entire reason this function exists, and no golden case exercises it: truncate(-25y, -1) is -20 where floor(-25y, -1) is -30. Mirroring the existing inputs from round was the right instinct, but it mechanically inherited round's all-positive inputs, which for this function drops the interesting half of the behavior.
There was a problem hiding this comment.
The no-overflow property isn't pinned down anywhere. round/bround carry boundary cases like bround(-9223372036854775808L, -1) that throw under ANSI. The mirror-image truncate cases would document in the ANSI golden that truncate never overflows — truncate(-128y, -1) is -120 while round(-128y, -1) overflows tinyint. Two lines, and it captures the most notable property of the new function.
| checkEvaluation(Truncate(Literal.create(null, DoubleType), Literal(2)), null) | ||
| checkEvaluation(Truncate(Literal(1.23), Literal.create(null, IntegerType)), null) | ||
| } | ||
|
|
There was a problem hiding this comment.
The one-argument form is never exercised end-to-end. MathExpressionsSuite builds new Truncate(Literal(...)) directly, which bypasses the function registry, and every SQL query, doctest, and DataFrame call passes an explicit scale. Since the usage string advertises FUNC(expr[, scale]), add SELECT truncate(1234.5678); to math.sql or as a fourth ExpressionDescription example (those are executed by ExpressionInfoSuite), plus a PySpark doctest that omits scale.
| checkConsistencyBetweenInterpretedAndCodegen(Logarithm, DoubleType, DoubleType) | ||
| } | ||
|
|
||
| test("truncate") { |
There was a problem hiding this comment.
Two of the three Scala overloads are untested. MathFunctionsSuite only uses truncate(Column, Column). In particular def truncate(e: Column): Column = truncate(e, 0) is the kind of delegation that can be wrong silently.
| Seq(Row(1234.56), Row(-1234.56)) | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
No decimal column goes through the DataFrame API. MathFunctionsSuite covers Int and Double only, yet Decimal is the path that required the new Decimal code. A decimal case there, and a NaN/Infinity double case since RoundBase special-cases those, would round it out.
| lv += (if (droppedDigits < 0) -1L else 1L) | ||
| } | ||
| case ROUND_DOWN => | ||
| // Truncation toward zero: `lv /= pow10diff` already dropped the fractional part. |
There was a problem hiding this comment.
| // Truncation toward zero: `lv /= pow10diff` already dropped the fractional part. | |
| // Truncation toward zero: `lv /= pow10diff` already dropped the fractional part. |
There was a problem hiding this comment.
The comment in Decimal.scala sits at the case indentation rather than inside the body; it should be indented one more level to read as the case body.
| case class Truncate( | ||
| child: Expression, | ||
| scale: Expression, | ||
| override val ansiEnabled: Boolean = SQLConf.get.ansiEnabled) |
There was a problem hiding this comment.
ansiEnabled is inert for Truncate, it drags in MathUtils.withOverflow wrappers, the *ValueExact codegen variants, and query-context plumbing that can never fire, and it participates in canonicalized. Either drop it and let RoundBase's false default stand, or keep it for symmetry with a one-line comment saying overflow is impossible here.
| child: Expression, | ||
| scale: Expression, | ||
| override val ansiEnabled: Boolean = SQLConf.get.ansiEnabled) | ||
| extends RoundBase(child, scale, BigDecimal.RoundingMode.DOWN, "ROUND_DOWN") { |
There was a problem hiding this comment.
RoundBase.dataType widens decimals by one integral digit because rounding can carry (ceil(9.9, 0) = 10). Truncation never can, so truncate(1234.5678, 2) returns decimal(7,2) where decimal(6,2) would do. Inheriting this is the safe choice, but a comment noting it's deliberate would help the next reader, since it can matter at MAX_PRECISION.
| ------- | ||
| :class:`~pyspark.sql.Column` | ||
| A column for the truncated value. | ||
| Returns a column of the same type as the input. |
There was a problem hiding this comment.
In the PySpark docstring, "Returns a column of the same type as the input" isn't accurate for decimals, whose precision and scale change.
|
|
||
| See Also | ||
| -------- | ||
| :meth:`pyspark.sql.functions.round` |
There was a problem hiding this comment.
The See Also should also probably point at trunc, floor, and ceil
| * constant. | ||
| * @group math_funcs | ||
| * @since 4.3.0 | ||
| * @return |
There was a problem hiding this comment.
here (and above), @return is placed after @since; whereas Spark's convention puts @param and @return before @group and @since.
|
@uros-b Thanks for the very thorough review, addressed all of it. math.sql now has negative inputs for every type and scale, showing truncation toward zero (e.g. truncate(-25y, -1) is -20). Added truncate(127y, -1) and truncate(-128y, -1) to document that truncate never overflows, unlike round. Added truncate(1234.5678) with no scale argument. MathFunctionsSuite now covers the truncate(Column, Int) and truncate(Column) overloads, a decimal column (the type that needed the new Decimal.changePrecision mode), and NaN and Infinity, which are returned unchanged. Fixed the comment indentation in Decimal.scala. Added comments explaining that ansiEnabled is inert for Truncate and that the inherited decimal precision widening is expected. Fixed the PySpark docstring wording for decimal outputs and added trunc, floor, and ceil to See Also. Also reordered the return tag to come before group and since in functions.scala. |
e657fc3 to
6ae0f55
Compare
|
@cloud-fan Would you have a moment to review this when you get a chance? Thank you! |
cloud-fan
left a comment
There was a problem hiding this comment.
0 blocking, 1 non-blocking, 1 nit.
The implementation and coverage look sound; two documentation fixes remain.
Correctness (1)
- sql/api/src/main/scala/org/apache/spark/sql/functions.scala:6078: The ScalaDoc promises that every overload returns the same type as the input, but decimal inputs are retyped by RoundBase according to the requested scale. Please mirror the Python wording here: the type is preserved except that decimal precision and scale may change. -- see inline
Nits: 1 minor item (see inline comments).
Verification
I traced SQL and language entry points through FunctionRegistry to Truncate, then through RoundBase and both Decimal representations. The SQL goldens confirm toward-zero behavior for positive and negative inputs, negative scales, boundary values, and omitted scale.
| * @param e | ||
| * the value to truncate. A column that evaluates to a numeric. | ||
| * @return | ||
| * Returns a column of the same type as the input. |
There was a problem hiding this comment.
Decimal inputs do not necessarily keep the same Spark SQL type: RoundBase derives new precision and scale from the requested scale. Please mirror the Python wording here and in the other two overloads: the type is preserved except that decimal precision and scale may change.
| arguments = """ | ||
| Arguments: | ||
| * expr - The expression to truncate. An expression that evaluates to a numeric. | ||
| * scale - The number of decimal places to keep. An expression that evaluates to an integer, must be a constant, and defaults to 0. A negative value truncates digits to the left of the decimal point. |
There was a problem hiding this comment.
| * scale - The number of decimal places to keep. An expression that evaluates to an integer, must be a constant, and defaults to 0. A negative value truncates digits to the left of the decimal point. | |
| * scale - The number of decimal places to keep. It must be a constant integer expression and defaults to 0. A negative value truncates digits to the left of the decimal point. |
|
@cloud-fan Thanks, updated the return doc on all three Scala overloads to match Python's decimal precision/scale wording, and applied the scale argument wording suggestion. |
cloud-fan
left a comment
There was a problem hiding this comment.
2 addressed, 0 remaining, 0 new to this AI review.
0 blocking, 0 non-blocking, 0 nits.
The implementation, public API wiring, documentation, and coverage are coherent; the prior documentation concerns are addressed and no current defects remain.
Verification
I traced each public entry point through function registration to Truncate, then followed interpreted and generated evaluation through RoundBase and both Decimal representations. I also checked the generated SQL result/analyzer sections for negative inputs, negative scales, the tinyint no-overflow boundary, and omitted scale, and reconciled every prior review thread against current source evidence. Tests were not run as part of this review.
### What changes were proposed in this pull request? Add a built in `truncate(expr[, scale])` scalar function that truncates a numeric value toward zero to `scale` decimal places. `scale` defaults to 0, and a negative `scale` truncates digits to the left of the decimal point. API surface added: - SQL: `truncate(expr)` and `truncate(expr, scale)` - Scala DataFrame: `functions.truncate(col)`, `functions.truncate(col, scale)` - PySpark, classic and Spark Connect: `pyspark.sql.functions.truncate(col, scale=None)` Implementation notes: - `Truncate` is a small `RoundBase` subclass using `RoundingMode.DOWN`, mirroring the existing `Round` (HALF_UP) and `BRound` (HALF_EVEN). It reuses the same input types, result type derivation, and constant scale handling. - `RoundBase` routes `DecimalType` through `Decimal.changePrecision`, which previously supported only the FLOOR, CEILING, HALF_UP, and HALF_EVEN modes. This adds `Decimal.ROUND_DOWN` and the corresponding branches so the DOWN mode is supported. The branches are trivial: on the compact (long backed) path, integer division already truncates toward zero, so no adjustment is needed; the BigDecimal path already uses `setScale(scale, roundMode)`, which supports DOWN natively. On naming: this adds a new function named `truncate` rather than overloading the existing `trunc`. Spark's `trunc` is date only (`trunc(date, fmt)`), and both the date form and a numeric `trunc(numeric, scale)` take two arguments, so they cannot be distinguished by arity the way `ceil`/`floor` overload their scale argument. Introducing a numeric overload of `trunc` would require type based dispatch at parse time, which is a larger and more error prone change. The name `truncate` matches MySQL and Trino, and the behavior (truncation toward zero) matches PostgreSQL `trunc`, BigQuery `TRUNC`, Oracle, and Snowflake. ### Why are the changes needed? Truncation toward zero to a given number of decimal places is a common numeric operation that Spark cannot express directly today. `floor` and `ceil` with a scale round toward negative and positive infinity, which is wrong for negative values, and `round`/`bround` round to nearest. The function is requested in SPARK-40945 and is provided by PostgreSQL, BigQuery, MySQL, Oracle, Snowflake, and Trino, so it also improves parity with the engines Spark users migrate from. ### Does this PR introduce _any_ user-facing change? Yes. It adds a new built in SQL function `truncate` and the corresponding Scala and PySpark DataFrame API entries. No existing behavior changes; the `Decimal` change only adds support for a new rounding mode and does not alter the existing modes. Example: ``` spark-sql> SELECT truncate(1234.5678, 2); 1234.56 spark-sql> SELECT truncate(-1234.5678, 2); -1234.56 spark-sql> SELECT truncate(1234.5678, -2); 1200 ``` ### How was this patch tested? Added catalyst unit tests in `MathExpressionsSuite` covering decimal (both long backed and BigDecimal backed), double, and integral inputs, positive and negative values, positive and negative scale, the default scale, and null propagation. Extended `DecimalSuite` so its existing "respect rounding mode" test also exercises `ROUND_DOWN` by comparing `Decimal.changePrecision` against `BigDecimal.setScale` for the DOWN mode. Added a DataFrame API test in `MathFunctionsSuite`, PySpark doctests, and regenerated `sql-expression-schema.md`. ### Was this patch authored or co-authored using generative AI tooling? No Closes #57476 from SreeramaYeshwanthGowd/add-truncate-function. Authored-by: SreeramaYeshwanthGowd <yeshwanthgowdsreerama@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit ae51635) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
What changes were proposed in this pull request?
Add a built in
truncate(expr[, scale])scalar function that truncates a numeric value toward zero toscaledecimal places.scaledefaults to 0, and a negativescaletruncates digits to the left of the decimal point.API surface added:
truncate(expr)andtruncate(expr, scale)functions.truncate(col),functions.truncate(col, scale)pyspark.sql.functions.truncate(col, scale=None)Implementation notes:
Truncateis a smallRoundBasesubclass usingRoundingMode.DOWN, mirroring the existingRound(HALF_UP) andBRound(HALF_EVEN). It reuses the same input types, result type derivation, and constant scale handling.RoundBaseroutesDecimalTypethroughDecimal.changePrecision, which previously supported only the FLOOR, CEILING, HALF_UP, and HALF_EVEN modes. This addsDecimal.ROUND_DOWNand the corresponding branches so the DOWN mode is supported. The branches are trivial: on the compact (long backed) path, integer division already truncates toward zero, so no adjustment is needed; the BigDecimal path already usessetScale(scale, roundMode), which supports DOWN natively.On naming: this adds a new function named
truncaterather than overloading the existingtrunc. Spark'struncis date only (trunc(date, fmt)), and both the date form and a numerictrunc(numeric, scale)take two arguments, so they cannot be distinguished by arity the wayceil/flooroverload their scale argument. Introducing a numeric overload oftruncwould require type based dispatch at parse time, which is a larger and more error prone change. The nametruncatematches MySQL and Trino, and the behavior (truncation toward zero) matches PostgreSQLtrunc, BigQueryTRUNC, Oracle, and Snowflake.Why are the changes needed?
Truncation toward zero to a given number of decimal places is a common numeric operation that Spark cannot express directly today.
floorandceilwith a scale round toward negative and positive infinity, which is wrong for negative values, andround/broundround to nearest. The function is requested in SPARK-40945 and is provided by PostgreSQL, BigQuery, MySQL, Oracle, Snowflake, and Trino, so it also improves parity with the engines Spark users migrate from.Does this PR introduce any user-facing change?
Yes. It adds a new built in SQL function
truncateand the corresponding Scala and PySpark DataFrame API entries. No existing behavior changes; theDecimalchange only adds support for a new rounding mode and does not alter the existing modes.Example:
How was this patch tested?
Added catalyst unit tests in
MathExpressionsSuitecovering decimal (both long backed and BigDecimal backed), double, and integral inputs, positive and negative values, positive and negative scale, the default scale, and null propagation. ExtendedDecimalSuiteso its existing "respect rounding mode" test also exercisesROUND_DOWNby comparingDecimal.changePrecisionagainstBigDecimal.setScalefor the DOWN mode. Added a DataFrame API test inMathFunctionsSuite, PySpark doctests, and regeneratedsql-expression-schema.md.Was this patch authored or co-authored using generative AI tooling? No