alexeykudinkin commented on code in PR #7752:
URL: https://github.com/apache/hudi/pull/7752#discussion_r1106128276
##########
hudi-common/src/main/java/org/apache/hudi/common/util/collection/CloseableMappingIterator.java:
##########
@@ -22,8 +22,8 @@
import java.util.function.Function;
-// TODO java-doc
-public class CloseableMappingIterator<I, O> extends MappingIterator<I, O>
implements ClosableIterator<O> {
+public class CloseableMappingIterator<I, O> extends MappingIterator<I, O>
Review Comment:
Can you help me understand what you want to have clarified in there?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/log/AbstractHoodieLogRecordReader.java:
##########
@@ -807,16 +809,17 @@ private ClosableIteratorWithSchema<HoodieRecord>
getRecordsIterator(
Option<Pair<Function<HoodieRecord, HoodieRecord>, Schema>>
schemaEvolutionTransformerOpt =
composeEvolvedSchemaTransformer(dataBlock);
+
// In case when schema has been evolved original persisted records will
have to be
// transformed to adhere to the new schema
- if (schemaEvolutionTransformerOpt.isPresent()) {
- return ClosableIteratorWithSchema.newInstance(
- new CloseableMappingIterator<>(blockRecordsIterator,
- schemaEvolutionTransformerOpt.get().getLeft()),
- schemaEvolutionTransformerOpt.get().getRight());
- } else {
- return ClosableIteratorWithSchema.newInstance(blockRecordsIterator,
dataBlock.getSchema());
- }
+ Function<HoodieRecord, HoodieRecord> transformer =
+ schemaEvolutionTransformerOpt.map(Pair::getLeft)
+ .orElse(Function.identity());
+
+ Schema schema = schemaEvolutionTransformerOpt.map(Pair::getRight)
+ .orElse(dataBlock.getSchema());
+
Review Comment:
Not sure i understand what your concern is: we now duplicate much less code
at the expense of having empty transformer (which would be optimized out by the
JVM)
##########
hudi-common/src/main/java/org/apache/hudi/common/util/ClosableIterator.java:
##########
@@ -24,8 +24,29 @@
* An iterator that give a chance to release resources.
*
* @param <R> The return type
+ *
+ * TODO move under common.util.collection
*/
Review Comment:
I get your point, but not everyone is the same -- i do follow-up on the
TODOs so all in all i find this still helpful even when there's no Jira ticket
associated (it's a minor change that doesn't seem to be worth creating a Jira
for it)
##########
hudi-common/src/main/java/org/apache/hudi/common/util/ClosableIterator.java:
##########
@@ -24,8 +24,29 @@
* An iterator that give a chance to release resources.
*
* @param <R> The return type
+ *
+ * TODO move under common.util.collection
*/
Review Comment:
This one actually i've left while doing another change, now since this PR
dedicated to the cleanup i will take it up in this one
--
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]
