kbendick commented on a change in pull request #1427:
URL: https://github.com/apache/iceberg/pull/1427#discussion_r483931007
##########
File path: core/src/main/java/org/apache/iceberg/IncrementalDataTableScan.java
##########
@@ -74,7 +74,7 @@ public TableScan appendsAfter(long newFromSnapshotId) {
Set<Long> snapshotIds = Sets.newHashSet(Iterables.transform(snapshots,
Snapshot::snapshotId));
Set<ManifestFile> manifests = FluentIterable
.from(snapshots)
- .transformAndConcat(s -> s.dataManifests())
+ .transformAndConcat(Snapshot::dataManifests)
Review comment:
Forgive me if this is a silly question, as I am more of a scala
developer traditionally and it's been a long day.
Does this change the semantics? Previously it was calling the
`dataManifests` method of each value in the iterable. And I notice later on
there's a call to `this::isNamespace`. Is this method reference expected to be
called in a static fashion (the interface doesn't declare it static) or will it
be called on each instance of the iterable? I only ask because
`BaseSnapshot#dataManifests` is a potentially side-effecting function.
But again, I might just be working too late with too much Scala in the
brain, so forgive me if I've just not brushed up on my Java lately. My brain is
_really_ trying to `flatMap` here so I could very well just need to brush up on
my Java.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
