zeroshade commented on code in PR #732:
URL: https://github.com/apache/arrow-go/pull/732#discussion_r3397696591
##########
arrow/flight/flightsql/server_test.go:
##########
@@ -133,6 +133,78 @@ func (*testServer) DoGetStatement(ctx context.Context,
ticket flightsql.Statemen
return
}
+func (*testServer) CreatePreparedStatement(ctx context.Context, req
flightsql.ActionCreatePreparedStatementRequest) (result
flightsql.ActionCreatePreparedStatementResult, err error) {
+ query := req.GetQuery()
+ var isUpdate *bool
+ result.Handle = []byte(query)
+ switch query {
+ case "prepared query":
+ isUpdate := false
+ result.IsUpdate = &isUpdate
+ case "prepared update":
+ isUpdate := true
+ result.IsUpdate = &isUpdate
+ default:
+ err = fmt.Errorf("unknown query: %s", query)
+ }
+ if isUpdate != nil {
Review Comment:
The `isUpdate := …` declarations inside the two cases above shadow the outer
`var isUpdate *bool`, so this `if isUpdate != nil` always sees nil — it's dead
code. The cases already set `result.IsUpdate` directly, so the outer `var
isUpdate *bool` and this block can both be removed.
##########
arrow/flight/flightsql/client.go:
##########
@@ -1348,6 +1351,11 @@ func (p *PreparedStatement) DatasetSchema()
*arrow.Schema { return p.datasetSche
// the prepared statement.
func (p *PreparedStatement) ParameterSchema() *arrow.Schema { return
p.paramSchema }
+// IsUpdate returns the server's hint about whether this prepared statement
+// should be executed as an update (true) or query (false). If nil, the server
+// did not provide a hint and the client can choose how to execute the
statement.
+func (p *PreparedStatement) IsUpdate() *bool { return p.isUpdate }
Review Comment:
Returning the internal `*bool` leaks mutable state (a caller can do
`*ps.IsUpdate() = false` and mutate the statement) and isn't idiomatic for an
optional. Prefer `IsUpdate() (val bool, ok bool)`, or return a copy.
--
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]
