asugranyes commented on PR #56180: URL: https://github.com/apache/spark/pull/56180#issuecomment-4584792915
> Nice fix - the reasoning about self-sufficient rules is solid. > > > > One edge case: `l + o` can overflow Int when both values are large (e.g. `LIMIT 2000000000 OFFSET 2000000000`). In ANSI mode the existing Add path would surface an overflow error via `ConstantFolding`, but the eager Scala addition wraps silently to a negative literal. Unlikely in practice, but easy to guard: > > > > ``` > > case (IntegerLiteral(l), IntegerLiteral(o)) => > > val sum = l.toLong + o.toLong > > if (sum <= Int.MaxValue) Literal(sum.toInt, IntegerType) > > else Add(le, oe) > > ``` The refinement defers the overflow case to `Add(le, oe)`, but in the scenario this PR targets (`ConstantFolding` excluded), that Add leaves an unplanable node — the same failure mode the PR is fixing. So the overflow case probably needs inline ANSI-aware handling to be consistent. -- 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]
