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` (so it seems possible to use a method
reference to identify a particular instance, though that might be a special
case for `this`). Is the method reference usage here expected to be called in a
static fashion (the interface doesn't declare the method as 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 insert `flatMap` and several `_` for args 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]
