SEPURI-SAI-KRISHNA commented on PR #28947:
URL: https://github.com/apache/flink/pull/28947#issuecomment-5289689543
Thanks @dianfu — you're right on both counts, and the first one is reachable.
I reproduced it: with an Object[] backing array,
ArrayObjectArrayConverter#toExternal takes the hasInternalElements fast path
and returns the backing array directly to the caller, which fails with
java.lang.ClassCastException: class [Ljava.lang.Object; cannot be cast to
class [Ljava.lang.Integer;
The GenericArrayData Javadoc states the requirement explicitly too
("Object arrays that contain boxed types ... MUST be boxed arrays"), so the
patch was violating a documented contract. Good catch.
Fixed in 936c053 by allocating the concrete component type, matching the
idiom already used in ArrayObjectArrayConverter and ArrayListConverter:
Object[] array = (Object[]) Array.newInstance(
LogicalTypeUtils.toInternalConversionClass(elementType), size);
Two tests cover it:
- testNullableArrayKeepsConcreteComponentType — pins the component type
for all eight supported element types (Integer[], Long[], Byte[], Short[],
Float[], Double[], Boolean[], StringData[])
- testNullableArraySurvivesExternalConversion — drives the real
DataStructureConverters path and reproduces the ClassCastException above
without the fix
Your second suggestion is adopted as well — toTritonDataType(elementType)
is now called before the loop. One note: it recurses into ArrayType rather than
rejecting it, so it doesn't reject a nested array on its own. I kept the
explicit ArrayType guard alongside it so the nullable path accepts exactly the
same element types as the primitive path.
Also rebased onto master and pushed the fix as a separate commit, so the
delta against your review is isolated in 936c053.
flink-model-triton is green locally at Tests run: 101, Failures: 0,
Errors: 0, Skipped: 0 (checkstyle and spotless included); waiting on Azure for
the new head. PR description updated to match.
--
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]
