SEZ9 commented on issue #12968: URL: https://github.com/apache/seatunnel/issues/12968#issuecomment-6431978679
Thanks @SEPURI-SAI-KRISHNA, that answers it. On the guards: having `fieldVector.isNull(rowIndex)` as the first statement of the generics `convert` overload in both `StructConverter` and `MapConverter`, before the `getObject`/`getReader` dereference, is exactly what I was after, and matching the shape `ListConverter`/`LargeListConverter` already use means a null cell now behaves the same across all the container converters. I also agree with your point that `MapConverter` does not need an `isEmpty` branch, since an empty map already falls out of the reader yielding no entries and `testEmptyMapCellStaysAnEmptyMap` pins that. Leaving the older `// todo check map` alone to keep the PR on one problem is fine with me. On the state at `a66150a38`: nothing left from me on the change itself. The remaining asks are: 1. Let me know here once the rerun you triggered finishes. If `AmazonDocumentDBIT.testProjection` fails again under Spark 3.3 with `row num: 0`, please post the log link so we can confirm it is unrelated to this change, given that connector has no arrow references and only ran because `connector-common` is upstream of every connector. 2. Once Build is green, I will take a final look at `a66150a38` and line up a write-access review. <!-- streview-comment:2056 --> -- 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]
