JingsongLi commented on PR #9702: URL: https://github.com/apache/paimon/pull/9702#issuecomment-5629947276
Thanks for addressing the row-tracking issue. I reran the 10 iterator/reader tests with the head classes on JDK 8; they pass. However, the original capability-loss premise does not hold for the current production path: Parquet and ORC create VectorizedRowIterator, which already overrides copy(ColumnVector[]) and returns a VectorizedRowIterator, preserving VectorizedRecordIterator for Arrow conversion. That override exists on the base as well as this head. The new TestingSpecializedIterator adds no production capability, so checking its object identity does not demonstrate an affected user read. The extra DataFileRecordReader ownership logic is needed to make the new identity shortcut safe; it does not establish an independent end-to-end benefit over the existing copy path. Closing this version because that benefit has not been demonstrated and the stated existing-reader defect is already handled. We can revisit a focused change with a real affected iterator/caller and failing read, or a representative end-to-end allocation/throughput comparison showing that avoiding the copy is worthwhile. Please keep that concrete consumer in the regression test. -- 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]
