SEZ9 commented on PR #12397:
URL: https://github.com/apache/seatunnel/pull/12397#issuecomment-6332216155
Thanks for the update. Build run `39091719501` succeeded for head
`47fccbac68f6`, so CI is green and there is no remaining blocker from my side.
The earlier points (F1-F7) stay optional and non-blocking; if you have time,
either in this PR or a follow-up, they would be nice to have:
- **F1** – a declared-vs-runtime element-class guard in the `reconvertArray`
overloads of `SeaTunnelRowConverter`, so a mismatch surfaces as a contextual
error rather than a bare `ArrayStoreException`. If a test already covers this,
a pointer is enough.
- **F2** – for TIME/TIMESTAMP_TZ element types in `TypeConverterUtils`
(Spark->SeaTunnel arrays), either an explicit guard or a short comment noting
the path is lossy and currently has no production caller.
- **F3/F4** – class-level Javadoc on `SeaTunnelArrayType`, and extending the
`runtimeClass` comment to cover both the parsed array-of-map case and the
mis-declared `LocalTimeType[].class` constants.
- **F5/F7** – in the incompatible-changes entry, spell out the failure mode
(null `Map` elements inside arrays now reach Spark sinks as `null` instead of
`{}`, so writers iterating `Map[]` without a null check may NPE) and name the
component by module path like sibling entries.
- **F6** – trim the release-note style wording from the Nested Arrays
section in the Spark engine docs (and the zh mirror).
None of these are required for merge. Thanks for the work on this!
<!-- streview-comment:1951 -->
--
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]