You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "brijrajk (via GitHub)" <gi...@apache.org> on 2026/06/22 16:47:52 UTC
[PR] [GLUTEN-12260][VL] Fix CheckOverflowTransformer passing wrong child dataType [gluten]
brijrajk opened a new pull request, #12332:
URL: https://github.com/apache/gluten/pull/12332
## What changes are proposed in this pull request?
Fixes #12260
**Root cause**
`CheckOverflowTransformer` was passing `original.child.dataType` — Spark's declared type on the raw `BinaryArithmetic` — to `createCheckOverflowExprNode` as the `childResultType` argument.
`DecimalArithmeticExpressionTransformer` overrides `dataType` to return `resultType`, the type computed by `DecimalArithmeticUtil.getResultType()` after `rescaleLiteral` adjusts the operands. When the literal is integer-valued (e.g. `5.0`, `BigDecimal.isValidLong == true`), `rescaleLiteral` rescales it, changing the result type. The transformer's actual output type therefore differs from Spark's declared type on the original expression.
`VeloxTransformerApi.createCheckOverflowExprNode` compares `childResultType` with the target `DecimalType` to decide whether to insert a cast:
```scala
if (childResultType.equals(dataType)) {
childNode // no cast needed
} else {
ExpressionBuilder.makeCast(typeNode, childNode, nullOnOverflow) // insert cast
}
```
With the wrong `childResultType`, the cast is generated with the wrong source type, making the Substrait plan invalid. Velox's SimpleFunction type validation rejects it, and `ColumnarPartialProjectRule` falls back the entire `Project` to JVM.
**Fix**
Pass `child.dataType` (the transformer's actual output type) instead of `original.child.dataType`. For transformers that do not override `dataType` (returning `original.dataType` by default) the behaviour is unchanged.
**Files changed**
- `UnaryExpressionTransformer.scala` — `original.child.dataType` → `child.dataType` in `CheckOverflowTransformer.doTransform`
- `DecimalArithmeticValidateSuite.scala` (new) — regression test that reproduces the exact trigger conditions from the issue report and asserts `ProjectExecTransformer` is used
---
## How was this patch tested?
New regression test `DecimalArithmeticValidateSuite` in `backends-velox`:
- Creates two BIGINT temp views
- Runs `SELECT a.val, (a.val - COALESCE(SUM(b.val), 0) / 5.0) / (COALESCE(SUM(b.val), 0) / 5.0) AS growth_rate FROM t1 CROSS JOIN t2 GROUP BY a.val` (the exact reproducer from #12260)
- Asserts `ProjectExecTransformer` is present in the executed plan (would fail before this fix)
Verified locally: 1/1 test passes.
---
## Was this patch authored or co-authored using generative AI tooling?
`Generated-by: Claude Code (claude-sonnet-4-6)`
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-12260][VL] Fix CheckOverflowTransformer passing wrong child dataType [gluten]
Posted by "brijrajk (via GitHub)" <gi...@apache.org>.
brijrajk commented on PR #12332:
URL: https://github.com/apache/gluten/pull/12332#issuecomment-4770751475
Closing as duplicate of #12261 which has the same fix and is already in active review.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-12260][VL] Fix CheckOverflowTransformer passing wrong child dataType [gluten]
Posted by "brijrajk (via GitHub)" <gi...@apache.org>.
brijrajk closed pull request #12332: [GLUTEN-12260][VL] Fix CheckOverflowTransformer passing wrong child dataType
URL: https://github.com/apache/gluten/pull/12332
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-12260][VL] Fix CheckOverflowTransformer passing wrong child dataType [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12332:
URL: https://github.com/apache/gluten/pull/12332#issuecomment-4770721541
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org