You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by "tuhaihe (via GitHub)" <gi...@apache.org> on 2026/02/04 07:13:04 UTC
[PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
tuhaihe opened a new pull request, #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47
<!--Thank you for contributing! -->
<!--In case of an existing issue or discussions, please reference it-->
closes: #ISSUE_Number
<!--Remove this section if no corresponding issue.-->
---
## Change logs
> Describe your change clearly, including what problem is being solved or what document is being added or updated.
## Contributor's checklist
Here are some reminders before you submit your pull request:
* Make sure that your Pull Request has a clear title and commit message. You can take the [Git commit template](https://github.com/apache/cloudberry/blob/main/.gitmessage) as a reference.
* Learn the [code contribution](https://cloudberry.apache.org/contribute/code) and [doc contribution](https://cloudberry.apache.org/contribute/doc) guides for better collaboration.
* Make sure that CICD workflow is successful.
* List your communications in the [GitHub Issues](https://github.com/apache/cloudberry-pxf/issues) or [Discussions](https://github.com/apache/cloudberry/discussions) (if has or needed).
* Feel free to ask for the [cloudberry committers](https://github.com/orgs/apache/teams/cloudberry-committers) or other people to help review and approve.
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "leborchuk (via GitHub)" <gi...@apache.org>.
leborchuk commented on code in PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47#discussion_r2762947085
##########
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:
Yep, I see there is now a slice of structs, and check for errors, so no nil items here anymore, thank you. It's safe to remove nil checking
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "tuhaihe (via GitHub)" <gi...@apache.org>.
tuhaihe merged PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "tuhaihe (via GitHub)" <gi...@apache.org>.
tuhaihe commented on PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47#issuecomment-3846199955
Hi @robertmu, could you please help review this PR when available? I know you are good at the `apache/cloudberry-go-libs`, Thanks!
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "leborchuk (via GitHub)" <gi...@apache.org>.
leborchuk commented on code in PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47#discussion_r2762859552
##########
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:
I see that we do not use nil as failed_command in tests anymore, but why failedCommand cannot be nil anymore? I'd still check for nil, else we would have nil dereference here. We could log on Error level all such situations. And even panic here, but not fail with nil dereference
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "robertmu (via GitHub)" <gi...@apache.org>.
robertmu commented on PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47#issuecomment-3846379132
> Hi @robertmu, could you please help review this PR when available? I know you are good at the `apache/cloudberry-go-libs`, Thanks!
LGTM
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "tuhaihe (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org
Re: [PR] Update go-libs url to `apache/cloudberry-go-libs` [cloudberry-pxf]
Posted by "robertmu (via GitHub)" <gi...@apache.org>.
robertmu commented on PR #47:
URL: https://github.com/apache/cloudberry-pxf/pull/47#issuecomment-3846396370
> These versions has almost identical commits (but different version numbers):
>
> ```
> github.com/greenplum-db/gp-common-go-libs v1.0.16
> github.com/apache/cloudberry-go-libs v1.0.12
> ```
>
> No regresssions expected.
That's a good observation. Regarding the version number appearing lower (v1.0.12 vs v1.0.16), this is because the cloudberry-go-libs repository hasn't tagged a formal release recently.
We are using a Go pseudo-version (v1.0.12-0.20250910...) which specifically points to a commit that has been fully synced with (and is effectively newer than v1.0.16) gp-common-go-libs. The code implementation is up-to-date despite the base version tag.
--
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: commits-unsubscribe@cloudberry.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org