lostluck commented on a change in pull request #15400:
URL: https://github.com/apache/beam/pull/15400#discussion_r705808589
##########
File path: sdks/go/pkg/beam/core/runtime/exec/translate.go
##########
@@ -457,7 +457,7 @@ func (b *builder) makeLink(from string, id linkID) (Node,
error) {
}
}
- case graph.Combine:
+ case graph.Combine, graph.CombineGlobally:
cn := &Combine{UID: b.idgen.New(), Out: out[0]}
cn.Fn, err = graph.AsCombineFn(fn)
if err != nil {
Review comment:
We should print out, in the urn switch below, what urn we're not
handling in the default case. `default: // For unlifted combines`
We shouldn't be missing any urns really, but a paranoid check can't hurt.
The [doc for lifted
combines](https://docs.google.com/document/d/1-3mEs3Y7bIkJ0hmQ6SiHpVIFu5vbY6Zcpw-7tOMVg4U/edit#heading=h.i62jhvh3wkod)
doesn't mention any different handling for global combines though, which is
frustrating, and the proto only mentions we should understand the combine
components, not that there's a "accumulate things locally" option that doesn't
have an iterable value...
--
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]
