[ https://issues.apache.org/jira/browse/BEAM-11104?focusedWorklogId=755347&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-755347 ]
ASF GitHub Bot logged work on BEAM-11104: ----------------------------------------- Author: ASF GitHub Bot Created on: 11/Apr/22 17:47 Start Date: 11/Apr/22 17:47 Worklog Time Spent: 10m Work Description: damccorm commented on code in PR #17334: URL: https://github.com/apache/beam/pull/17334#discussion_r847587000 ########## sdks/go/pkg/beam/core/runtime/exec/fn.go: ########## @@ -274,52 +286,60 @@ func (n *invoker) ret2(pn typex.PaneInfo, ws []typex.Window, ts typex.EventTime, // ret3 handles processing of a trio of return values. func (n *invoker) ret3(pn typex.PaneInfo, ws []typex.Window, ts typex.EventTime, r0, r1, r2 interface{}) (*FullValue, error) { switch { - case n.outEtIdx >= 0: + case n.outEtIdx == 0: if n.outErrIdx == 2 { if r2 != nil { return nil, r2.(error) } n.ret = FullValue{Windows: ws, Timestamp: r0.(typex.EventTime), Elm: r1, Pane: pn} return &n.ret, nil } - if n.outPcIdx >= 0 { + if n.outPcIdx == 2 { n.ret = FullValue{Windows: ws, Timestamp: r0.(typex.EventTime), Elm: r1, Pane: pn, Continuation: r2.(sdf.ProcessContinuation)} return &n.ret, nil } n.ret = FullValue{Windows: ws, Timestamp: r0.(typex.EventTime), Elm: r1, Elm2: r2, Pane: pn} return &n.ret, nil - default: - if n.outErrIdx == 2 { - if r2 != nil { - return nil, r2.(error) - } - if n.outPcIdx == 1 { - n.ret = FullValue{Windows: ws, Timestamp: ts, Elm: r0, Pane: pn, Continuation: r1.(sdf.ProcessContinuation)} - return &n.ret, nil - } - n.ret = FullValue{Windows: ws, Timestamp: ts, Elm: r0, Elm2: r1, Pane: pn} + case n.outErrIdx == 2: + if r2 != nil { + return nil, r2.(error) + } + if n.outPcIdx == 1 { + n.ret = FullValue{Windows: ws, Timestamp: ts, Elm: r0, Pane: pn, Continuation: r1.(sdf.ProcessContinuation)} return &n.ret, nil } + n.ret = FullValue{Windows: ws, Timestamp: ts, Elm: r0, Elm2: r1, Pane: pn} + return &n.ret, nil + default: n.ret = FullValue{Windows: ws, Timestamp: ts, Elm: r0, Elm2: r1, Pane: pn, Continuation: r2.(sdf.ProcessContinuation)} return &n.ret, nil } } // ret4 handles processing of a quad of return values. func (n *invoker) ret4(pn typex.PaneInfo, ws []typex.Window, ts typex.EventTime, r0, r1, r2, r3 interface{}) (*FullValue, error) { - if r3 != nil { - return nil, r3.(error) - } - if n.outEtIdx == 0 { - if n.outPcIdx >= 0 { - n.ret = FullValue{Windows: ws, Timestamp: r0.(typex.EventTime), Elm: r1, Pane: pn, Continuation: r2.(sdf.ProcessContinuation)} + switch { Review Comment: I don't really mind it too much here since its consistent with the other functions, but in general I don't love 1 condition switch statements (with or without a default) since its basically just an if. Consider changing it to an if Issue Time Tracking ------------------- Worklog Id: (was: 755347) Time Spent: 8h 40m (was: 8.5h) > [Go SDK] DoFn Self Checkpointing > -------------------------------- > > Key: BEAM-11104 > URL: https://issues.apache.org/jira/browse/BEAM-11104 > Project: Beam > Issue Type: Sub-task > Components: sdk-go > Reporter: Robert Burke > Assignee: Jack McCluskey > Priority: P3 > Time Spent: 8h 40m > Remaining Estimate: 0h > > Allow SplittableDoFns to self checkpoint. > Design doc: > https://docs.google.com/document/d/1_JbzjY9JR07ZK5v7PcZevUfzHPsqwzfV7W6AouNpMPk/edit?usp=sharing -- This message was sent by Atlassian Jira (v8.20.1#820001)