youngoli commented on a change in pull request #13070:
URL: https://github.com/apache/beam/pull/13070#discussion_r503530205
##########
File path: sdks/go/pkg/beam/core/runtime/exec/datasource.go
##########
@@ -354,12 +354,12 @@ func (n *DataSource) Split(splits []int64, frac float64,
bufSize int64) (SplitRe
return SplitResult{PI: s - 1, RI: s}, nil
}
// Otherwise, perform a sub-element split.
- p, r, err := su.Split(fr)
+ ps, rs, err := su.Split(fr)
if err != nil {
return SplitResult{}, err
}
- if p == nil || r == nil { // Unsuccessful split.
+ if len(ps) == 0 || len(rs) == 0 { // Unsuccessful split.
Review comment:
Just because once I moved to slices, having an empty slice seemed to
make more sense to indicate "the function worked, but the result was empty",
rather than nil slices which is what gets returned for errors. I could switch
it back to nil, since that was mostly a stylistic choice.
Checking over the code, yeah it looks like if one is empty I always just
return both empty. But I think I used an "or" here just to avoid relying on
implementation details of the split. Basically, if even one of them is missing
then then Datasource knows the split failed, and doesn't have to really know
what exactly happened.
----------------------------------------------------------------
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]
