tuhaihe commented on code in PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47#discussion_r2762906934
##########
cli/cmd/cluster.go:
##########
@@ -111,9 +111,6 @@ func GenerateOutput(cmd *command, clusterData *ClusterData)
error {
}
response := ""
for _, failedCommand := range clusterData.Output.FailedCommands {
- if failedCommand == nil {
- continue
- }
Review Comment:
Good question! The nil check was removed because the upstream library
`apache/cloudberry-go-libs` changed the type of `FailedCommands` from
`[]*cluster.ShellCommand` (pointer slice) to `[]cluster.ShellCommand` (value
slice).
In Go, when iterate over a value slice with `for _, item := range slice`,
each `item` is a struct value, not a pointer. Struct values cannot be nil —
only pointers, interfaces, maps, slices, channels, and functions can be nil.
If we kept the `if failedCommand == nil` check, the code would not compile:
```
invalid operation: failedCommand == nil (mismatched types
cluster.ShellCommand and untyped nil)
```
- See GitHub actions:
https://github.com/apache/cloudberry-pxf/actions/runs/21662582531/job/62451094343#step:9:7017
Additionally, looking at the [
NewRemoteOutput
function](https://github.com/apache/cloudberry-go-libs/blob/main/cluster/cluster.go#L210-L227)
in `cloudberry-go-libs`, the `FailedCommands` slice is constructed by only
appending commands where `command.Error != nil`. So the slice will only contain
valid, meaningful entries — there's no possibility of a "nil-like" zero-value
struct sneaking in.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
