qingfureal commented on code in PR #58801:
URL: https://github.com/apache/spark/pull/58801#discussion_r4030002682
##########
core/src/main/java/org/apache/spark/util/collection/unsafe/sort/UnsafeSorterIterator.java:
##########
@@ -33,6 +33,18 @@ public abstract class UnsafeSorterIterator {
public abstract long getKeyPrefix();
+ /**
+ * The total number of records this iterator was created over.
+ *
+ * <p>This is a fixed total rather than a remaining count: it does not
change as
+ * {@link #loadNext()} is called, and it does not account for records the
iterator was advanced
+ * past before being handed on. It therefore tells you how many records the
iterator will still
+ * produce only while the iterator is untouched.
+ *
+ * <p>Iterators that aggregate others sum their inputs' totals and inherit
the same caveat: the
+ * sum describes what the aggregate produces only if no input was advanced
past records the
+ * aggregate will not go on to emit.
+ */
Review Comment:
Good call, trimmed. I kept one clause beyond the first sentence:
> it does not change as `loadNext()` is called, and it does not account for
records the iterator was advanced past and will not emit.
The "advanced past" half is the part that actually catches people:
`ChainedIterator` sums its children's totals, but `getIterator(startIndex)`
advances those children via `moveOver()` first, so the sum includes records the
aggregate never emits. Without that clause the doc reads as if the total always
predicts what you'll get.
--
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]
