JingsongLi commented on code in PR #9702:
URL: https://github.com/apache/paimon/pull/9702#discussion_r3968142704


##########
paimon-common/src/main/java/org/apache/paimon/data/columnar/ColumnarRowIterator.java:
##########
@@ -115,6 +115,10 @@ public ColumnarRowIterator copy(ColumnVector[] vectors) {
 
     public ColumnarRowIterator mapping(
             @Nullable PartitionInfo partitionInfo, @Nullable int[] 
indexMapping) {
+        if (partitionInfo == null && isIdentityMapping(indexMapping, 
row.batch().getArity())) {
+            return this;

Review Comment:
   **[P1] Keep row-tracking wrappers out of the reader's reusable batch**
   
   For an unpartitioned table with row tracking enabled, reading 
`t$row_tracking` can reach this branch with a full identity mapping: 
`FormatReaderMapping.Builder.trimKeyFields()` returns an explicit identity 
array even when the schema mapping is null. `DataFileRecordReader` then calls 
`assignRowTracking()`, which replaces entries in `batch.columns` in place. 
Returning the original iterator exposes the Parquet/ORC reader's reusable 
column array to those mutations, so each reused batch wraps the previous 
batch's wrappers. Since their `isNullAt()` always returns false, each metadata 
`getLong()` recursively traverses the accumulated wrappers, causing 
progressively slower reads and eventually `StackOverflowError`. Previously, 
`createMappedVectors()` plus `copy()` isolated these mutations in a separate 
column array.
   
   I reproduced this on JDK 8 with a real Parquet file, batch size 1, and the 
same `mapping(...).assignRowTracking(...)` sequence: this implementation 
overflows when checking the row at approximately 20,000 batches, while the 
identical test with the parent implementation completes all 100,000 batches. 
The four existing PR tests pass but do not exercise this reuse path.
   
   Please keep row-tracking decoration isolated from the format reader's column 
array, or retain the copy path when row tracking needs to modify the vectors, 
and add a regression covering repeated batch reuse.



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