sharanggupta commented on code in PR #1381: URL: https://github.com/apache/arrow-java/pull/1381#discussion_r4345712403
########## vector/src/test/java/org/apache/arrow/vector/TestDenseUnionVector.java: ########## Review Comment: Good idea. Done in f41e9a8c: I replaced the two DenseUnionVector-only tests with a parameterized test in `TestSplitAndTransfer`, where the transfer tests for the other vector kinds already live. For 18 kinds, it transfers a populated vector (or splits and transfers it), transfers the result again, and compares it with the original using `VectorEqualsVisitor`. The kinds are the fixed-width, variable-width and view vectors, fixed-size binary, the list, list-view and fixed-size list vectors, map, struct, union and dense union. To answer the other question: without the fix, only the DenseUnionVector cases fail (`expected [5, true, 7] but was [null, null, null]`). The other 17 kinds already keep their values through a second transfer, so DenseUnionVector was the only one missing state there. Factoring the other per-vector behavioural tests into shared parameterized tests looks worthwhile, but it touches many test classes, so I've kept it out of this PR. I can open an issue for it if that's useful. -- 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]
