LeonxLJX commented on issue #19632: URL: https://github.com/apache/hudi/issues/19632#issuecomment-5535584398
I'd like to take this one — the Long-vs-Int narrowing case is straightforward and I want to look at the reversed-operands and the other numeric pairs while I'm in there (claiming via @LeonxLJX). **Where I think the fix lives.** `HoodieProcedureFilterUtils.applyTypeCoercion` is the only place that special-cases anything today, and the right shape for me is to delete the special case entirely and route through Spark's own coercion. Two concrete options: 1. **Use CatalystTypeCoercion.findTightestCommonType on the leaf pair** — that already knows that `LongType` + `IntegerType` widens to `LongType`, and that `Double` + `Decimal(_, 1, 0)` widens to `DoubleType`. It handles both operand orders in one shot, which fixes case (2) for free, and it also fixes case (3) for every numeric pair it already understands. I think this is the cleaner fix and I'd bias toward it. 2. **Hand-roll a symmetric `(BoundReference, Literal) | (Literal, BoundReference)` matcher that widens the smaller type** — fewer moving parts and more obvious to read, but it's still my own narrow-coercion table that has to be extended every time Spark grows a new numeric type. I'd lean (1) and use the existing pinned tests to drive it; happy to switch if a maintainer prefers (2). **Two things I'd flag to the maintainer while I'm in this file.** - `evaluateFilter`'s per-row `Try` swallows the cast overflow and surfaces it as a non-match, which is what makes all three bugs silent. A `Try` on a `Catalyst` expression should probably at least log at debug the first time it fires per session, so silent no-matches are diagnosable without rerunning with a debugger. I'll keep this scoped to the pinned tests unless a maintainer wants the log folded in. - If we go with (1), `findTightestCommonType` returns `Some(...)` even for pairs it doesn't know (e.g. string + numeric), so we still need the `case None => original` fallback to preserve today's behaviour for unrelated types. I'll come back with the PR once I have the pinned assertions flipped. Happy to break this into multiple commits (coercion table, then operand-order, then `findTightestCommonType`) if that's easier to 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
