jackylee-ch opened a new issue, #12991: URL: https://github.com/apache/gluten/issues/12991
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, <wider type>))` 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. -- 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]
