LeonxLJX commented on issue #19633: URL: https://github.com/apache/hudi/issues/19633#issuecomment-5535584540
I'd like to claim this one as well — it's adjacent to #19632 and the fix is the same shape of work (claiming via @LeonxLJX). **Fix shape.** `convertValueToInternal` should wrap arrays in `new GenericArrayData(...)` before handing them back to the expression evaluator. Two things I'd note while I'm there: 1. `GenericArrayData` has three constructor shapes — `Seq[T]`, `Array[T]`, and `ArrayData` — and the JVM erases the element type, so the safe call is to box once into a `Seq[Any]` rather than relying on a `Array[Object]`. The existing maps path (`ArrayBasedMapData`) does the same kind of boxing for the same reason. 2. The same line is also the place that wraps binary (`BinaryType` → `Array[Byte]`), and `ArrayData` over a `byte[]` works for `size` / `getStruct` / `getBinary` reads, so the pinned test for `binary` should be flip-safe with the same change. If it isn't, I'd split the binary path into its own wrapping rather than collapse them. **Alternative I considered and rejected.** Adding a `case _ => new GenericArrayData(Array(...))` catch-all upstream of `convertValueToInternal` would centralise the wrapping, but it shifts the boxing into the hot loop and makes the per-type cases in `convertValueToInternal` redundant, so I'd rather keep the wrapping in the one place that already does per-type conversion. I'll keep the PR scoped to flipping the array pinned assertion and let the binary one ride on whichever side it falls. Will reference #19632's PR so the two land in a reviewable order. -- 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]
