vrozov commented on issue #272: Refactor AvroIterable and prevent multiple iterators URL: https://github.com/apache/incubator-iceberg/pull/272#issuecomment-511017881 It goes in a circle. I already explained that people make other assumptions about Iterables and Iterators, but somehow it is OK to violate those assumptions and not OK to violate another assumption. Would you agree that iterating twice over the same file should be discouraged in favor of single iteration and when necessary it is possible to get Iterable without any extra performance penalty? In rear cases where multiple iterators are necessary, it is easy to go from ``` try (AvroIterable records = Avro.read(...).build()) { for (R record : records) { ... } for (R record : records) { ... } } ``` to ``` try (AvroIterable records = Avro.read(...).build()) { for (R record : records) { ... } } ... try (AvroIterable records = Avro.read(...).build()) { for (R record : records) { ... } } ``` There is also no better way. Iterators are not expected to be `Closeable` and it is very easy to lose `Closeable` iterator during Iterable transformation. For example `CloseableIterable.transform()` does not preserve `Closeable` and guava `Iterables` won't preserve it either. The only consistency I see in this PR review is the denial of the contribution. Arguments provided changed from "not a common violation" to "there is a better way". When I asked for a "use case" it was never provided. In any case, the PR is closed and there is no need to discuss this further. I am working away from contribution to the iceberg project.
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
