peterxcli opened a new issue, #5190:
URL: https://github.com/apache/datafusion-comet/issues/5190

   ### What is the problem the feature request solves?
   
   Decimal `Divide` is currently serialized with two consecutive, equivalent 
`CheckOverflow` expressions.
   
   After #5171:
   
   - 
[`DecimalPrecision.promote`](https://github.com/apache/datafusion-comet/blob/f3050be3f19f274cc226b27a460ab2e842e2b9f7/spark/src/main/scala/org/apache/spark/sql/comet/DecimalPrecision.scala#L60-L62)
 adds an outer wrapper with `datatype = Divide.dataType` and `failOnError = 
(Divide.evalMode == ANSI)`.
   - 
[`CometDivide.convert`](https://github.com/apache/datafusion-comet/blob/f3050be3f19f274cc226b27a460ab2e842e2b9f7/spark/src/main/scala/org/apache/comet/serde/arithmetic.scala#L276-L286)
 adds an inner wrapper with the same datatype and overflow mode.
   
   The resulting shape is:
   
   ```text
   CheckOverflow(type=T, fail=F)
     CheckOverflow(type=T, fail=F)
       Divide(...)
   ```
   
   The native planner only removes a `CheckOverflow` whose child is a matching 
`WideDecimalBinaryExpr`, or fuses one whose child is a decimal `Cast`; it does 
not collapse nested `CheckOverflow` expressions 
([`planner.rs`](https://github.com/apache/datafusion-comet/blob/f3050be3f19f274cc226b27a460ab2e842e2b9f7/native/core/src/execution/planner.rs#L515-L563)).
   
   On successful array-valued decimal-division batches, both wrappers therefore 
perform the precision scan in 
[`CheckOverflow.evaluate`](https://github.com/apache/datafusion-comet/blob/f3050be3f19f274cc226b27a460ab2e842e2b9f7/native/spark-expr/src/math_funcs/internal/checkoverflow.rs#L105-L135).
   
   ### Describe the potential solution
   
   Stop `DecimalPrecision.promote` from wrapping decimal `Divide`, while 
retaining the specialized wrapper in `CometDivide.convert`. That wrapper is 
required to handle Comet's native decimal-division overflow sentinel correctly.
   
   Acceptance criteria:
   
   - Decimal `Divide` serializes to exactly one `CheckOverflow`.
   - Its datatype equals `Divide.dataType`.
   - `failOnError` is true only for ANSI mode.
   - LEGACY and TRY overflow behavior remains null-producing.
   - ANSI overflow behavior remains error-producing.
   - Existing divide-by-zero behavior is unchanged.
   - Add a proto-level regression covering LEGACY, TRY, and ANSI modes.
   
   ### Additional context
   
   Raised in [review of 
#5171](https://github.com/apache/datafusion-comet/pull/5171#pullrequestreview-4830862439),
 while fixing #5075.
   


-- 
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]

Reply via email to