SEZ9 commented on PR #12650:
URL: https://github.com/apache/seatunnel/pull/12650#issuecomment-6356385983
Thanks for the thorough follow-up.
**Sync:** Noted that `6fad753c1` merges from `df94fb07d` with `cc54bb433` as
an ancestor, and that none of the newer `dev` commits touch the Arrow reader or
its test, so nothing else to rebase for here.
**Map/Struct across batches:** `testStructAndMapCrossRecordBatches` on the
same `{2, 0, 3}` plan, with every value asserted by absolute row and
`getReadRowCount` checked, is exactly the coverage I was hoping for. From your
description it should close the complex-vector gap and the `getReadRowCount`
point; I'll confirm against the diff once the new commit is up. The mutation
result you describe (NPE in the reader with the half-fix, single-batch tests
still green) is encouraging.
Your three notes all make sense. Driving the map through `UnionMapWriter`
despite the standing `// todo check map` is good to know (feel free to soften
that comment if you touch the class again, not required), pinning `convertRow`
returning a field-name keyed `Map` is a plus, and dropping the redundant
`setValueCount` after `setRowCount` should settle the style point once I see it
in the diff.
Agreed on leaving out a `getValueCount()` guard for short vectors; that
stays a separate malformed-payload discussion.
Two small remaining asks:
1. **Null cell at a boundary.** The new tests cover complex types and the
empty batch, but I don't see a nullable column mentioned. Could you confirm
whether any row after the empty batch carries a null cell (scalar or complex)?
If not, one null in a post-boundary row would close that branch cheaply.
2. **Method-level Javadoc on `convertSeatunnelRow()`.** Optional: a short
Javadoc stating that `rowCountInOneBatch` is per-batch and `readRowCount` is
cumulative across batches would settle the ordering-dependency point without
further code changes.
Once CI finishes, just post what you see and I'll take another pass.
<!-- streview-comment:1966 -->
--
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]
