thswlsqls commented on code in PR #17252:
URL: https://github.com/apache/iceberg/pull/17252#discussion_r3635310205
##########
arrow/src/main/java/org/apache/iceberg/arrow/vectorized/NullabilityHolder.java:
##########
@@ -75,6 +75,15 @@ public int numNulls() {
return numNulls;
}
+ /**
+ * Resets this holder so it can be reused for the next batch.
+ *
+ * <p>Only the null count is cleared. The per-index markers returned by
{@link #isNullAt(int)} are
+ * deliberately left untouched: for every batch, writers set the marker of
each index through
+ * {@link #setNull(int)}/{@link #setNotNull(int)} (or their bulk variants
{@link #setNulls(int,
+ * int)}/{@link #setNotNulls(int, int)}) before the markers are read, so
values carried over from
+ * a previous batch are overwritten rather than observed.
Review Comment:
@ldudas-marx Thanks for the review. Moved the reasoning into the method
body. I kept one line in the Javadoc — that `reset()` clears only the null
count and leaves the per-index markers — since that's observable by any caller
that reaches `isNullAt(int)` after a reset, which is what made this look like a
bug in #15809. Happy to move all of it into the body if a committer prefers.
--
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]
