SEPURI-SAI-KRISHNA opened a new issue, #12649: URL: https://github.com/apache/seatunnel/issues/12649
### Search before asking - [X] I had searched in the [issues](https://github.com/apache/seatunnel/issues?q=is%3Aissue+label%3A%22bug%22) and found no similar issues. ### What happened An arrow IPC stream may carry more than one record batch, and `ArrowToSeatunnelRowReader` is written to handle that: `readArrow` loops on `loadNextBatch()`, appends each batch's rows to a cumulative list, and accumulates `readRowCount`. The conversion step does not follow the same convention, so **every record batch after the first throws `IndexOutOfBoundsException`**. **What I verified and what I did not.** Verified, on `dev` at `11e4f5554`, by running it: the throw and the exact line it comes from; which batch shapes break and which do not, measured across 11 shapes; that both halves of the fix are required; and that after the fix the rows come back complete and in order, including a multi-column case with an `ARRAY` column and a `TIMESTAMP` column. Not verified: whether a Doris or StarRocks backend actually packs more than one record batch into a single `TScanBatchResult`. That is a backend implementation detail I cannot establish from this repository, and no issue reports this crash. **So this is a report that the multi-batch code path is broken and untested, not a claim that Doris or StarRocks are crashing in production.** ### The defect `ArrowToSeatunnelRowReader.java:133-152`. Two indexing conventions are mixed in one loop: ```java for (int i = 0; i < seatunnelRowBatch.size(); i++) { // cumulative, across ALL batches if (fieldIndex != null) { ... Object fieldValue = convertArrowData( readRowCount + i, minorType, fieldVector, seaTunnelDataType); // indexes the VECTOR ... seatunnelRowBatch.get(readRowCount + i).setField(fieldIndex, fieldValue); // indexes the LIST } } ``` `seatunnelRowBatch` is cumulative, so `readRowCount + i` is right for it. `fieldVector` holds **only the batch currently loaded**, so indexing it by the absolute row number is wrong, and the loop bound is the cumulative size rather than this batch's row count. With a first batch of 3 rows and a second of 4 (`readRowCount=3`, `size()=7`, this batch = 4): | i | vector index `3+i` | list index `3+i` | outcome | | --- | --- | --- | --- | | 0 | 3, in range | 3 | writes the wrong row's value | | 1,2,3 | 4,5,6 out of range | 4,5,6 | arrow returns null, no exception | | 4 | | **7, size is 7** | `IndexOutOfBoundsException` | Measured stack: ``` java.lang.IndexOutOfBoundsException: Index 7 out of bounds for length 7 at java.base/java.util.ArrayList.get(ArrayList.java:459) at ...ArrowToSeatunnelRowReader.convertSeatunnelRow(ArrowToSeatunnelRowReader.java:148) at ...ArrowToSeatunnelRowReader.readArrow(ArrowToSeatunnelRowReader.java:111) ``` Note the throw comes from the `ArrayList`, not from arrow. Arrow answers an out-of-range index with `null` rather than an exception, which matters below. ### Which shapes break, measured | batches | result | | --- | --- | | `{3}`, `{7}` single | correct | | `{1,1}`, `{3,4}`, `{5,2}`, `{2,5}`, `{1,1,1}` | `IndexOutOfBoundsException` | | `{0,4}`, `{0,0,3}` leading empty batches | correct, they are skipped before `readRowCount` moves | | `{4,0}` trailing empty | correct | | `{4,0,2}` empty in the middle | `IndexOutOfBoundsException` | So the rule is: the first non-empty batch is handled correctly, and every later non-empty batch throws. It always throws, so no partially converted rows ever reach the caller on `dev` today. ### Why it has never been caught Both tests in `ArrowToSeatunnelRowReaderTest` call `writer.writeBatch()` exactly once, `testSeatunnelRow` at `:316` and `testConvertArrowSpeed` at `:442`. A single batch makes `readRowCount` zero and `seatunnelRowBatch.size()` equal to this batch's row count, so both conventions coincide and the bug is invisible. ### A trap worth recording The loop bound looks like the whole bug, and it is not. Correcting only the bound and leaving `readRowCount + i` on the vector removes the exception and replaces it with **silent data corruption**, because arrow returns `null` past the end instead of throwing. Measured, same 3+4 stream: ``` expected: <[0, 1, 2, 100, 101, 102, 103]> but was: <[0, 1, 2, 103, null, null, null]> ``` A regression test that asserts only the row count would pass against that. Any test for this needs to assert the values. ### Both halves are required | change | outcome | | --- | --- | | neither (current `dev`) | `IndexOutOfBoundsException` | | loop bound only | no exception, wrong values as above | | vector index only | `IndexOutOfBoundsException` | | both | correct | ### Fix Bound the loop by the current batch's row count, and index the vector from 0 while continuing to index the cumulative list from `readRowCount`: ```java for (int i = 0; i < rowCountInOneBatch; i++) { ... Object fieldValue = convertArrowData(i, minorType, fieldVector, seaTunnelDataType); ... seatunnelRowBatch.get(readRowCount + i).setField(fieldIndex, fieldValue); // unchanged } ``` Single-batch behaviour is unchanged by construction: with one batch `readRowCount` is 0 and `rowCountInOneBatch == seatunnelRowBatch.size()`, so both forms are identical. The existing tests, including the 1,000,000 row `testConvertArrowSpeed`, pass untouched. Verified after the fix: all 11 shapes above return the right values in the right order, and a two-batch stream with `BIGINT`, `VARCHAR`, `ARRAY<INT>` and `TIMESTAMP` columns reads back all rows correctly, which exercises `ListConverter` and `TimeStampMilliConverter` through the changed `rowIndex`. ### Scope `ArrowToSeatunnelRowReader` has two production callers, `DorisValueReader` and `StarRocksBeReadClient`, both passing `TScanBatchResult.getRows()` straight from the backend. As stated above, I make no claim about what those backends emit. ### SeaTunnel Version `dev` at `11e4f5554`, 3.0.0-SNAPSHOT. ### Are you willing to submit PR? - [X] Yes I am willing to submit a PR! -- 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]
