[GLUTEN-12991][CORE] Remove the leftover PromotePrecision shim and its decimal rescale path

[LuciferYang]

What changes are proposed in this pull request?

Spark deleted the PromotePrecision Catalyst expression in SPARK-39316, which landed in 3.4. Gluten kept four byte-identical passthrough stubs under shims/spark{34,35,40,41} so one codebase could serve 3.3 and 3.4+. #12902 removed Spark 3.3, so the class is now Gluten's own and nothing in the repository constructs it: the only withNewChildren on it lives inside rescaleCastForOneSide, which is reachable only from a match on PromotePrecision itself. That makes the three rewrites DecimalArithmeticUtil grew around it identity functions and the two ExpressionConverter arms that matched it dead code.

Removed: the four stub classes; rescaleLiteral, rescaleCastForDecimal, removeCastForDecimal and the five helpers only they called; SparkShims.widerDecimalType with its four overrides, whose only caller was checkIsWiderType inside that rescale path; the PROMOTE_PRECISION name and its Sig; and the PromotePrecision(Cast) arm in ExpressionConverter. genRescaleDecimalTransformer no longer rescales anything, so it is renamed and takes the child types directly.

The CheckOverflow(b: BinaryArithmetic, ...) arm goes for a separate reason. On v3.4.4, v3.5.5, v4.0.2 and v4.1.1, plain CheckOverflow has three construction sites, all in SerializerBuildHelper, where an encoder converts a JVM BigDecimal to Decimal through a StaticInvoke. No rule wraps it around an arithmetic node: decimal overflow is handled by BinaryArithmetic itself via resultDecimalType and checkDecimalOverflow, and table insert and Sum use the CheckOverflowInTableInsert and CheckOverflowInSum variants. The case c: CheckOverflow arm further down still catches the encoder path, so nothing is left uncovered.

Both of those arms sit behind !transformCheckOverflow, which only clickhouse sets. The three DecimalArithmeticUtil rewrites are on the velox and bolt path instead.

getResultType stays, and so does its case other => throw new GlutenNotSupportException. Once the rewrites are gone it computes exactly b.dataType for the four operators it handles, which invites replacing the call with b.dataType outright, and that would be a behaviour change: Spark's Remainder.resultDecimalType and Pmod.resultDecimalType do not throw, so decimal % decimal and pmod would start being offloaded. Dropping Remainder and Pmod from isDecimalArithmetic has the same effect from the other direction, since they would then fall through to the generic arm and be offloaded as a plain modulus or pmod without the _deny_precision_loss name. A new DecimalArithmeticUtilSuite pins that rejection so the next reader who notices the redundancy has a test to trip over.

How was this patch tested?

clean test-compile on Spark 3.4, 3.5, 4.0 and 4.1 with -Pbackends-velox -Pspark-ut -Piceberg -Pdelta, Scala 2.13 throughout. spotless:apply produced no changes.

-Pbackends-clickhouse compiles on 3.5 only, so that is where it was checked. On the other three profiles it does not build at all on main: CHRuleApi.scala and src-delta/main reference Delta 3.3 APIs, and delta.binary.version=33 is set by the spark-3.5 profile alone. -Pbackends-bolt is out for a similar reason, that bolt does not compile on main until #12999 lands.

On Spark 3.5: GlutenDecimalPrecisionSuite, GlutenDecimalExpressionSuite and GlutenArithmeticExpressionSuite in gluten-ut/spark35, DecimalArithmeticUtilSuite in gluten-substrait, plus MathFunctionsValidateSuite and MiscOperatorSuite in backends-velox. GlutenDecimalPrecisionSuite is the closest thing to a regression net here: it runs Spark's own analyzer over nested decimal arithmetic, integral operands and 38-digit saturation, then asserts the transformed expression's type matches. It only asserts the outermost node's type, though, which is why the added GLUTEN-7082 case compares end to end against vanilla rather than checking a plan shape.

Three cases in backends-velox failed, and all three fail the same way on main: decimal to double preserves precision after decimal division, GLUTEN-12356: high-precision decimal to double matches vanilla Spark and cast null type to complex type. The local libvelox.dylib predates #12605 and #12950, and it contains no decimalToFloatHighPrecisionCastEnabled symbol at all, so the first two cannot pass against it. Everything else was green: 143 of 146 in backends-velox, 42 in gluten-ut/spark35, and the new unit case.

The clickhouse half of the diff, the two arms behind !transformCheckOverflow, is compile-only: there is no clickhouse native library on this machine. A Run Gluten Clickhouse CI pass would be worth having before merge.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude claude-opus-5

Related issue: #12991

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

🔵 Needs a closer look

Cross-version conversion changes include a ClickHouse path validated only by compilation, and additional plan-level regression coverage was requested.

Pull request overview

Removes obsolete Spark 3.3 PromotePrecision compatibility code and dead decimal conversion paths now that supported versions begin at Spark 3.4.

Changes:

• Deletes promotion shims, mappings, names, and wider-decimal APIs.
• Removes dead decimal rescaling and arithmetic overflow paths.
• Adds regression coverage for decimal remainder and pmod fallback behavior.

File summaries

File	Summary
shims/spark41/src/main/scala/org/apache/spark/sql/catalyst/expressions/PromotePrecision.scala	Removes obsolete shim.
shims/spark41/src/main/scala/org/apache/gluten/sql/shims/spark41/Spark41Shims.scala	Removes wider-decimal override.
shims/spark40/src/main/scala/org/apache/spark/sql/catalyst/expressions/PromotePrecision.scala	Removes obsolete shim.
shims/spark40/src/main/scala/org/apache/gluten/sql/shims/spark40/Spark40Shims.scala	Removes wider-decimal override.
shims/spark35/src/main/scala/org/apache/spark/sql/catalyst/expressions/PromotePrecision.scala	Removes obsolete shim.
shims/spark35/src/main/scala/org/apache/gluten/sql/shims/spark35/Spark35Shims.scala	Removes wider-decimal override.
shims/spark34/src/main/scala/org/apache/spark/sql/catalyst/expressions/PromotePrecision.scala	Removes obsolete shim.
shims/spark34/src/main/scala/org/apache/gluten/sql/shims/spark34/Spark34Shims.scala	Removes wider-decimal override.
shims/common/src/main/scala/org/apache/gluten/sql/shims/SparkShims.scala	Removes obsolete shim API.
shims/common/src/main/scala/org/apache/gluten/expression/ExpressionNames.scala	Removes promotion expression name.
gluten-substrait/src/test/scala/org/apache/gluten/utils/DecimalArithmeticUtilSuite.scala	Tests decimal remainder and pmod rejection; plan-level coverage was requested.
gluten-substrait/src/main/scala/org/apache/gluten/utils/DecimalArithmeticUtil.scala	Removes dead rescaling helpers.
gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionMappings.scala	Removes obsolete signature.
gluten-substrait/src/main/scala/org/apache/gluten/expression/ExpressionConverter.scala	Simplifies decimal conversion and removes dead cases.
backends-velox/src/test/scala/org/apache/gluten/functions/MathFunctionsValidateSuite.scala	Adds nested decimal arithmetic regression coverage.

Review details

Suppressed comments (1)

gluten-substrait/src/test/scala/org/apache/gluten/utils/DecimalArithmeticUtilSuite.scala:40

• This only exercises DecimalArithmeticUtil directly; it never runs ExpressionConverter with transformCheckOverflow = true. A regression that bypasses getResultType or routes decimal %/pmod through the generic transformer would still pass while offloading these expressions. Please add a Spark/Gluten plan test with non-foldable decimal operands for both operators and assert that they fall back rather than reaching a native plan.

  test("remainder and pmod are admitted but have no result type") {
    Seq(Remainder(left, right), Pmod(left, right)).foreach {
      expr =>
        assert(DecimalArithmeticUtil.isDecimalArithmetic(expr))
        intercept[GlutenNotSupportException] {
          DecimalArithmeticUtil.getResultType(expr, decimalType, decimalType)

• Files reviewed: 15/15 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Run Gluten Clickhouse CI on x86

[LuciferYang]

cc @jackylee-ch FYI

[Copilot]

🔵 Needs a closer look

Add an ExpressionConverter or end-to-end assertion proving decimal %/pmod still fall back when transformCheckOverflow=true.

Review details

Suppressed comments (1)

gluten-substrait/src/test/scala/org/apache/gluten/utils/DecimalArithmeticUtilSuite.scala:39

• Although this asserts that the utility throws, it never exercises ExpressionConverter with transformCheckOverflow=true. A regression that removes Remainder/Pmod from isDecimalArithmetic or routes them through the generic arm would still pass here while decimal %/pmod are offloaded instead of falling back. Please add an end-to-end or converter assertion that these decimal expressions actually fall back on the Velox/Bolt path.

  test("remainder and pmod are admitted but have no result type") {
    Seq(Remainder(left, right), Pmod(left, right)).foreach {
      expr =>
        assert(DecimalArithmeticUtil.isDecimalArithmetic(expr))
        intercept[GlutenNotSupportException] {

• Files reviewed: 15/15 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jackylee-ch]

Basically looks good to me.

Could you also verify whether the issue reported in #7082 still exists with this change?

[rui-mo]

Thanks @LuciferYang. Just one comment.

[rui-mo]

Is this an existing limitation, and could we fix it by following the below result type calculation?

https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/arithmetic.scala#L1070-L1078

[LuciferYang]

Yes, this is a pre-existing limitation rather than something this PR introduces — the throw has been there since the gluten-substrait split, and this change only preserves it (the PR is a no-op removal of the PromotePrecision machinery).

On fixing it via resultDecimalType: that calculation is necessary but not sufficient. Spark's Pmod.resultDecimalType follows the same Remainder rule, so the Gluten side would be small. But Velox's sparksql library registers decimal arithmetic for add/subtract/multiply/divide only (velox/functions/sparksql/DecimalArithmetic.cpp); remainder/pmod are registered for integral and floating types only, not decimal. So if we dropped the throw and emitted a decimal modulus/pmod, Velox has no decimal kernel to resolve it, and it would not offload — the explicit throw is the cleaner fallback point. Supporting it would need the decimal remainder/pmod kernels added in Velox first, then the type calc here.

That is a real enhancement rather than part of this cleanup, so I'd rather keep it out of this PR and open a follow-up. Happy to file one.