QuakeWang commented on PR #7598:
URL: https://github.com/apache/paimon/pull/7598#issuecomment-4645504235

   > I think there is still a correctness issue in the projected Arrow bundle 
fast path.
   > 
   > `ArrowBundleRecords.project(int[] projection)` only changes the `rowType`, 
but it keeps the original `VectorSchemaRoot`. This is fine for iterator-based 
consumers, because `ArrowBatchReader` uses the projected `rowType` to map 
fields by name. However, after this PR, `ProjectedFileWriter.writeBundle` 
preserves the projected bundle as an `ArrowBundleRecords` and passes it through 
to the underlying bundle-aware writer.
   > 
   > For real bundle format writers such as Lance/Mosaic/Vortex, `writeBundle` 
does not consume the bundle through `iterator()`. It detects 
`ArrowBundleRecords` and writes `getVectorSchemaRoot()` directly. In that path 
the projection is effectively lost, so a dedicated vector child writer may 
receive the full original root instead of only its projected physical columns. 
This can either write the wrong schema/columns or fail with a schema mismatch, 
depending on the format.
   > 
   > The current `ProjectedFileWriterTest` does not catch this because the mock 
writer iterates over the bundle. I think we need either:
   > 
   > 1. make `ArrowBundleRecords.project(...)` return a truly projected 
`VectorSchemaRoot`, or
   > 2. avoid preserving the `ArrowBundleRecords` fast-path type after 
projection unless the root itself is projected.
   > 
   > Could you add a test that exercises a projected `ArrowBundleRecords` with 
a real `BundleFormatWriter` path, not only an iterator-based mock writer?
   
   @JingsongLi  Thanks for catching this.
   
   I fixed the issue by making `ArrowBundleRecords.project(...)` project the 
underlying `VectorSchemaRoot` as well, instead of only changing the `rowType`.
   
   I also added a test that goes through the `ArrowBundleWriter.writeBundle` 
fast path, so it verifies the projected Arrow root that a real bundle writer 
receives, not just iterator-based consumption.


-- 
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]

Reply via email to