amogh-jahagirdar commented on code in PR #17296:
URL: https://github.com/apache/iceberg/pull/17296#discussion_r3793276856
##########
arrow/src/main/java/org/apache/iceberg/arrow/vectorized/VectorizedArrowReader.java:
##########
@@ -775,7 +780,7 @@ public VectorHolder read(VectorHolder reuse, int
numValsToRead) {
ArrowVectorAccessor<?, String, ?, ?> idsAccessor =
ids == null ? null :
ArrowVectorAccessors.getVectorAccessor(idsHolder);
- BigIntVector rowIds = allocateBigIntVector(ROW_ID_ARROW_FIELD,
numValsToRead);
+ BigIntVector rowIds = resultVector(reuse);
Review Comment:
Not a blocker but I think we should document on the interface level that
implementations must implement reuse, and when it's not-null they should return
the same underlying vector with the values filled in.
Ideally we could refactor these interfaces into a way that we even avoid
this, but that's a bigger change.
##########
arrow/src/main/java/org/apache/iceberg/arrow/vectorized/VectorizedArrowReader.java:
##########
@@ -882,12 +921,47 @@ public void setBatchSize(int batchSize) {
this.nulls = newNullabilityHolder(batchSize);
}
+ // release the result vector when the batch grows, so the next read
allocates a vector that
+ // fits the new batch size
+ if (vec != null && vec.getValueCapacity() < batchSize) {
+ vec.close();
+ this.vec = null;
+ }
+
+ this.resultBatchSize = (batchSize == 0) ? DEFAULT_BATCH_SIZE : batchSize;
Review Comment:
Yeah this was just a copy paste from the original Position reader for the
new lineage readers, but as far as I can tell nothing actually passes in 0, and
nor should there be.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
