SEZ9 commented on PR #12397: URL: https://github.com/apache/seatunnel/pull/12397#issuecomment-6567893206
Thanks for the detailed follow-up on head `5a5f78a717f19f84bbf04d4bb53f9903c3a7e2da`. On the earlier points, your description matches what I was looking for: - **F1** – Catching the `ArrayStoreException` in the shared helper and reporting index, declared type and expected/actual runtime classes without the payload addresses the core concern. Field/table identifiers would be useful enrichment, not a blocker for this PR. - **F2** – A comment recording the physical Long/Decimal fallback and why logical TIME / TIMESTAMP_TZ metadata cannot be recovered from an array element is an acceptable resolution, given that path has no production caller in the translation layer. - **F3 / F4** – Documenting allocation ownership plus the parser `MapType.class` and API `LocalTimeType[].class` limitations is what I was after. The class name can stay as-is. - **F5 / F7** – I still need to read the incompatible-changes entry on this head to confirm the NPE failure mode for sink writers iterating `Map[]` elements without a null check is explicit and that the module reference follows the sibling entries' convention. - **F6** – Describing the supported array shapes separately from the FakeSource construction limitation is the right structure; I'll check whether the 'Previously ...' / 'This array-conversion fix does not ...' prose is still present on the reference pages. I'll verify these against the diff on this head and close the findings there; nothing further is needed from you unless that review turns up a wording tweak. <!-- streview-comment:2211 --> -- 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]
