[CORE] Remove the leftover PromotePrecision shim and its decimal rescale path
- Dominant language
- Scala
- Stars
- 1.6k
- Forks
- 657
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 86
Description
Gluten still ships a hand-written `org.apache.spark.sql.catalyst.expressions.PromotePrecision` in all four shim modules, plus the decimal-arithmetic rewrites built around it. Both are unreachable now that Spark 3.3 support is gone, and one comment in `ExpressionConverter` still says otherwise.
**Why it exists.** Up to Spark 3.3, `DecimalPrecision` wrapped decimal operands in `PromotePrecision(Cast(child, ))` and the operation in `CheckOverflow`. Velox derives a decimal result type from its *input* types, so the widened operands gave the wrong scale, and `DecimalArithmeticUtil` grew three rewrites to undo them: recover a literal's real precision/scale, narrow an integral operand's cast to the minimal decimal type, and strip decimal→decimal widening casts. When 3.4 deleted the expression ([SPARK-39316](https://issues.apache.org/jira/browse/SPARK-39316)), #3360 added a transparent stub in Spark's package so one code path could still serve 3.3 and 3.4; it was then copied to 3.5 (#4822), 4.0 (#9768) and 4.1.
**Why it is no longer needed.** Beyond the node simply never appearing, Spark 3.4+ performs all three adjustments itself — `BinaryArithmetic.resultDecimalType` carries the result type, `DecimalPrecision.decimalAndDecimal` only rewrites `BinaryComparison`, and `nondecimalAndDecimal` casts via `DecimalType.forType` / `fromLiteral`. So Gluten's three rewrites return their input unchanged. The same reasoning retires the `CheckOverflow(BinaryArithmetic, ...)` case: in 3.4–4.1 only the encoder path constructs `CheckOverflow`, while table inserts and `Sum` build the `InTableInsert` / `InSum` variants.
**To remove**
- `DecimalArithmeticUtil`: the three identity rewrites and the private helpers only they reach
- `ExpressionConverter`: the `PromotePrecision` case and pass-through alternative, the unreachable `CheckOverflow(BinaryArithmetic, ...)` case, and [this comment](https://github.com/apache/gluten/blob/5ff78c30606a3d175d486d1dea7a9b6e3f7f14a3/gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala#L691-L693)
- the `promote_precision` signature and name constant — no native backend implements it
- the four shim stubs
- `SparkShims.widerDecimalType` and its overrides, whose only caller goes away above
**Two things to watch**
1. `isDecimalArithmetic` admits `Remainder`/`Pmod`, but result-type derivation [throws for them](https://github.com/apache/gluten/blob/5ff78c30606a3d175d486d1dea7a9b6e3f7f14a3/gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala#L65-L66) — today that throw is the only thing keeping `decimal % decimal` off Velox. Keep an explicit rejection and add a fallback test.
2. This may close #7082, which blames exactly this machinery and suggests keeping `PromotePrecision(Cast(...))`. Worth running its repro on 3.5/4.1 first: if it passes, add it as a test and close that issue.
No behaviour change expected otherwise; the result type Gluten declares already matches Spark's. Follow-up to #12807.
Contributor guide
Assessment
This issue has not been assessed yet.