openinx commented on a change in pull request #2881:
URL: https://github.com/apache/iceberg/pull/2881#discussion_r678048412
##########
File path: api/src/main/java/org/apache/iceberg/io/ClosingIterator.java
##########
@@ -25,33 +25,30 @@
public class ClosingIterator<T> implements Iterator<T> {
private final CloseableIterator<T> iterator;
- private boolean shouldClose = false;
public ClosingIterator(CloseableIterator<T> iterator) {
this.iterator = iterator;
-
}
@Override
public boolean hasNext() {
boolean hasNext = iterator.hasNext();
- this.shouldClose = !hasNext;
+ if (!hasNext) {
+ close();
Review comment:
I think you mean in the read pattern (for the original `ClosingIterator`)
```java
while (hasNext()) {
T val = next();
}
```
When the hasNext() return false, then we won't call the `next()` to close
the internal `CloseableIterator`. Yes, I think you are correct, but I think
we will need to guarantee that we will only close the `CloseableIterator` once
even if we called the `ClosingIterator#hasNext` many times, otherwise once
we've called the first `hasNext`, all the following `hasNext` check will be
thrown an exception, that's not the expected behavior.
--
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]
