nehsyc commented on a change in pull request #14530:
URL: https://github.com/apache/beam/pull/14530#discussion_r613440809
##########
File path:
sdks/java/core/src/main/java/org/apache/beam/sdk/util/IdentityWindowFn.java
##########
@@ -63,14 +62,16 @@ public IdentityWindowFn(Coder<? extends BoundedWindow>
coder) {
}
@Override
- public Collection<BoundedWindow> assignWindows(WindowFn<T,
BoundedWindow>.AssignContext c)
- throws Exception {
+ public Collection<BoundedWindow> assignWindows(WindowFn<T,
BoundedWindow>.AssignContext c) {
// The window is provided by the prior WindowFn, which also provides the
coder for them
return Collections.singleton(c.window());
}
@Override
public boolean isCompatible(WindowFn<?, ?> other) {
+ // Only compatible with itself.
+ if (this.equals(other)) return true;
Review comment:
This was needed because for stateful DoFn, the direct runner replaces it
with `GbkThenStatefulParDo` which applies a windowing without an explicit
window fn:
https://github.com/apache/beam/blob/2db98ee64a36a2e15b7a8873fbbecb8a4e3e36d9/runners/direct-java/src/main/java/org/apache/beam/runners/direct/ParDoMultiOverrideFactory.java#L202
which triggers a `Flatten` of the input although in this case there is only
one input pcollection:
https://github.com/apache/beam/blob/2db98ee64a36a2e15b7a8873fbbecb8a4e3e36d9/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/windowing/Window.java#L407
`Flatten` requires the windowing of the input(s) to be compatible including
with itself:
https://github.com/apache/beam/blob/2db98ee64a36a2e15b7a8873fbbecb8a4e3e36d9/sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/Flatten.java#L104
So two options AFAIT:
- Allowing `IdentityWindowFn` to be compatible with itself (which makes
sense I guess?)
- Skipping windowing fn comparison for the same input PCollection in
`Flatten`
--
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]
