Skip to content

Commit

Permalink
chore(spanner): fix some go vet issues (#9758)
Browse files Browse the repository at this point in the history 
This fixes some trivial go vet issues. Each commit is dedicated to specific issue class (mostly).
  • Loading branch information
@egonelbre
egonelbre committed May 2, 2024
1 parent 176dfcf commit 966f83d
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 73 deletions.
56 changes: 29 additions & 27 deletions spanner/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2425,7 +2425,6 @@ func TestClient_ReadWriteTransaction_DoNotLeakSessionOnPanic(t *testing.T) {

_, err := client.ReadWriteTransaction(ctx, func(ctx context.Context, tx *ReadWriteTransaction) error {
panic("cause panic")
return nil
})
if err != nil {
t.Fatalf("Unexpected error during transaction: %v", err)
Expand Down Expand Up @@ -3317,7 +3316,7 @@ func TestClient_Apply_ApplyOptions(t *testing.T) {
if err != nil {
t.Fatalf("failed applying mutations: %v", err)
}
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{Priority: tt.wantPriority, TransactionTag: tt.wantTransactionTag})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{Priority: tt.wantPriority, TransactionTag: tt.wantTransactionTag})
})
}
}
Expand Down Expand Up @@ -3731,7 +3730,10 @@ func TestReadWriteTransaction_ContextTimeoutDuringCommit(t *testing.T) {
tx.BufferWrite([]*Mutation{Insert("FOO", []string{"ID", "NAME"}, []interface{}{int64(1), "bar"})})
return nil
})
errContext, _ := context.WithTimeout(context.Background(), -time.Second)

errContext, cancel := context.WithTimeout(context.Background(), -time.Second)
defer cancel()

w := toSpannerErrorWithCommitInfo(errContext.Err(), true).(*Error)
var se *Error
if !errorAs(err, &se) {
Expand Down Expand Up @@ -4632,7 +4634,7 @@ func TestClient_ReadOnlyTransaction_Priority(t *testing.T) {
iter.Next()
iter.Stop()

checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 2, sppb.RequestOptions{Priority: qo.Priority})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 2, &sppb.RequestOptions{Priority: qo.Priority})
tx.Close()
}
}
Expand Down Expand Up @@ -4664,11 +4666,11 @@ func TestClient_ReadWriteTransaction_Priority(t *testing.T) {
tx.BatchUpdateWithOptions(context.Background(), []Statement{
NewStatement(UpdateBarSetFoo),
}, qo)
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, sppb.RequestOptions{Priority: qo.Priority})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, &sppb.RequestOptions{Priority: qo.Priority})

return nil
}, to)
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{Priority: to.CommitPriority})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{Priority: to.CommitPriority})
}
}
}
Expand Down Expand Up @@ -4700,9 +4702,9 @@ func TestClient_StmtBasedReadWriteTransaction_Priority(t *testing.T) {
NewStatement(UpdateBarSetFoo),
}, qo)

checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, sppb.RequestOptions{Priority: qo.Priority})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, &sppb.RequestOptions{Priority: qo.Priority})
tx.Commit(context.Background())
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{Priority: to.CommitPriority})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{Priority: to.CommitPriority})
}
}
}
Expand All @@ -4718,7 +4720,7 @@ func TestClient_PDML_Priority(t *testing.T) {
{Priority: sppb.RequestOptions_PRIORITY_HIGH},
} {
client.PartitionedUpdateWithOptions(context.Background(), NewStatement(UpdateBarSetFoo), qo)
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 1, sppb.RequestOptions{Priority: qo.Priority})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 1, &sppb.RequestOptions{Priority: qo.Priority})
}
}

Expand Down Expand Up @@ -4767,16 +4769,16 @@ func TestClient_Apply_Priority(t *testing.T) {
defer teardown()

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{})

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})}, Priority(sppb.RequestOptions_PRIORITY_HIGH))
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{Priority: sppb.RequestOptions_PRIORITY_HIGH})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{Priority: sppb.RequestOptions_PRIORITY_HIGH})

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})}, ApplyAtLeastOnce())
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{})

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})}, ApplyAtLeastOnce(), Priority(sppb.RequestOptions_PRIORITY_MEDIUM))
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{Priority: sppb.RequestOptions_PRIORITY_MEDIUM})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{Priority: sppb.RequestOptions_PRIORITY_MEDIUM})
}

func TestClient_ReadOnlyTransaction_Tag(t *testing.T) {
Expand All @@ -4803,7 +4805,7 @@ func TestClient_ReadOnlyTransaction_Tag(t *testing.T) {
iter.Next()
iter.Stop()

checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 2, sppb.RequestOptions{RequestTag: qo.RequestTag})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 2, &sppb.RequestOptions{RequestTag: qo.RequestTag})
tx.Close()
}
}
Expand Down Expand Up @@ -4838,10 +4840,10 @@ func TestClient_ReadWriteTransaction_Tag(t *testing.T) {

// Check for SQL requests inside the transaction to prevent the check to
// drain the commit request from the server.
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, sppb.RequestOptions{RequestTag: qo.RequestTag, TransactionTag: to.TransactionTag})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, &sppb.RequestOptions{RequestTag: qo.RequestTag, TransactionTag: to.TransactionTag})
return nil
}, to)
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{TransactionTag: to.TransactionTag})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{TransactionTag: to.TransactionTag})
}
}
}
Expand Down Expand Up @@ -4872,10 +4874,10 @@ func TestClient_StmtBasedReadWriteTransaction_Tag(t *testing.T) {
tx.BatchUpdateWithOptions(context.Background(), []Statement{
NewStatement(UpdateBarSetFoo),
}, qo)
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, sppb.RequestOptions{RequestTag: qo.RequestTag, TransactionTag: to.TransactionTag})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 4, &sppb.RequestOptions{RequestTag: qo.RequestTag, TransactionTag: to.TransactionTag})

tx.Commit(context.Background())
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{TransactionTag: to.TransactionTag})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{TransactionTag: to.TransactionTag})
}
}
}
Expand All @@ -4891,7 +4893,7 @@ func TestClient_PDML_Tag(t *testing.T) {
{RequestTag: "request-tag-1"},
} {
client.PartitionedUpdateWithOptions(context.Background(), NewStatement(UpdateBarSetFoo), qo)
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 1, sppb.RequestOptions{RequestTag: qo.RequestTag})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 1, &sppb.RequestOptions{RequestTag: qo.RequestTag})
}
}

Expand All @@ -4902,16 +4904,16 @@ func TestClient_Apply_Tagging(t *testing.T) {
defer teardown()

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{})

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})}, TransactionTag("tx-tag"))
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{TransactionTag: "tx-tag"})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{TransactionTag: "tx-tag"})

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})}, ApplyAtLeastOnce())
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{})

client.Apply(context.Background(), []*Mutation{Insert("foo", []string{"col1"}, []interface{}{"val1"})}, ApplyAtLeastOnce(), TransactionTag("tx-tag"))
checkCommitForExpectedRequestOptions(t, server.TestSpanner, sppb.RequestOptions{TransactionTag: "tx-tag"})
checkCommitForExpectedRequestOptions(t, server.TestSpanner, &sppb.RequestOptions{TransactionTag: "tx-tag"})
}

func TestClient_PartitionQuery_RequestOptions(t *testing.T) {
Expand All @@ -4934,7 +4936,7 @@ func TestClient_PartitionQuery_RequestOptions(t *testing.T) {
iter.Next()
iter.Stop()
}
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, len(partitions), sppb.RequestOptions{RequestTag: qo.RequestTag, Priority: qo.Priority})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, len(partitions), &sppb.RequestOptions{RequestTag: qo.RequestTag, Priority: qo.Priority})
}
}

Expand All @@ -4958,11 +4960,11 @@ func TestClient_PartitionRead_RequestOptions(t *testing.T) {
iter.Next()
iter.Stop()
}
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, len(partitions), sppb.RequestOptions{RequestTag: ro.RequestTag, Priority: ro.Priority})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, len(partitions), &sppb.RequestOptions{RequestTag: ro.RequestTag, Priority: ro.Priority})
}
}

func checkRequestsForExpectedRequestOptions(t *testing.T, server InMemSpannerServer, reqCount int, ro sppb.RequestOptions) {
func checkRequestsForExpectedRequestOptions(t *testing.T, server InMemSpannerServer, reqCount int, ro *sppb.RequestOptions) {
reqs := drainRequestsFromServer(server)
reqOptions := []*sppb.RequestOptions{}

Expand Down Expand Up @@ -4998,7 +5000,7 @@ func checkRequestsForExpectedRequestOptions(t *testing.T, server InMemSpannerSer
}
}

func checkCommitForExpectedRequestOptions(t *testing.T, server InMemSpannerServer, ro sppb.RequestOptions) {
func checkCommitForExpectedRequestOptions(t *testing.T, server InMemSpannerServer, ro *sppb.RequestOptions) {
reqs := drainRequestsFromServer(server)
var commit *sppb.CommitRequest
var ok bool
Expand Down
2 changes: 1 addition & 1 deletion spanner/examples_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func ExampleCommitTimestamp() {
Creation spanner.NullTime // time.Time can also be used if column isNOT NULL
}

a := account{User: "Joe", Creation: spanner.NullTime{spanner.CommitTimestamp, true}}
a := account{User: "Joe", Creation: spanner.NullTime{Time: spanner.CommitTimestamp, Valid: true}}
m, err := spanner.InsertStruct("Accounts", a)
if err != nil {
// TODO: Handle error.
Expand Down
8 changes: 4 additions & 4 deletions spanner/internal/testutil/inmem_spanner_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ import (
"google.golang.org/protobuf/types/known/timestamppb"
)

var (
// KvMeta is the Metadata for mocked KV table.
KvMeta = spannerpb.ResultSetMetadata{
// KvMeta is the Metadata for mocked KV table.
func KvMeta() *spannerpb.ResultSetMetadata {
return &spannerpb.ResultSetMetadata{
RowType: &spannerpb.StructType{
Fields: []*spannerpb.StructType_Field{
{
Expand All @@ -56,7 +56,7 @@ var (
},
},
}
)
}

// StatementResultType indicates the type of result returned by a SQL
// statement.
Expand Down
2 changes: 1 addition & 1 deletion spanner/pdml_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func TestPartitionedUpdate_Tagging(t *testing.T) {
if err != nil {
t.Fatalf("expect no errors, but got %v", err)
}
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 1, sppb.RequestOptions{RequestTag: "pdml-tag"})
checkRequestsForExpectedRequestOptions(t, server.TestSpanner, 1, &sppb.RequestOptions{RequestTag: "pdml-tag"})
}

func TestPartitionedUpdate_ExcludeTxnFromChangeStreams(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions spanner/read_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ var (
// Metadata for mocked KV table, its rows are returned by SingleUse
// transactions.
kvMeta = func() *sppb.ResultSetMetadata {
meta := KvMeta
meta := KvMeta()
meta.Transaction = &sppb.Transaction{
ReadTimestamp: timestampProto(trxTs),
}
return &meta
return meta
}()
// Metadata for mocked ListKV table, which uses List for its key and value.
// Its rows are returned by snapshot readonly transactions, as indicated in
Expand Down
14 changes: 10 additions & 4 deletions spanner/spannertest/db_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,16 @@ func slurp(t *testing.T, ri rowIter) (all [][]interface{}) {
}

func listV(vs ...*structpb.Value) *structpb.ListValue { return &structpb.ListValue{Values: vs} }
func stringV(s string) *structpb.Value { return &structpb.Value{Kind: &structpb.Value_StringValue{s}} }
func floatV(f float64) *structpb.Value { return &structpb.Value{Kind: &structpb.Value_NumberValue{f}} }
func boolV(b bool) *structpb.Value { return &structpb.Value{Kind: &structpb.Value_BoolValue{b}} }
func nullV() *structpb.Value { return &structpb.Value{Kind: &structpb.Value_NullValue{}} }
func stringV(s string) *structpb.Value {
return &structpb.Value{Kind: &structpb.Value_StringValue{StringValue: s}}
}
func floatV(f float64) *structpb.Value {
return &structpb.Value{Kind: &structpb.Value_NumberValue{NumberValue: f}}
}
func boolV(b bool) *structpb.Value {
return &structpb.Value{Kind: &structpb.Value_BoolValue{BoolValue: b}}
}
func nullV() *structpb.Value { return &structpb.Value{Kind: &structpb.Value_NullValue{}} }

func boolParam(b bool) queryParam { return queryParam{Value: b, Type: boolType} }
func stringParam(s string) queryParam { return queryParam{Value: s, Type: stringType} }
Expand Down
20 changes: 10 additions & 10 deletions spanner/spannertest/inmem.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,15 +319,15 @@ func (l *lro) Run(s *server, stmts []spansql.DDLStmt) {
if st := s.runOneDDL(ctx, stmt); st.Code() != codes.OK {
l.mu.Lock()
l.state.Done = true
l.state.Result = &lropb.Operation_Error{st.Proto()}
l.state.Result = &lropb.Operation_Error{Error: st.Proto()}
l.mu.Unlock()
return
}
}

l.mu.Lock()
l.state.Done = true
l.state.Result = &lropb.Operation_Response{&anypb.Any{}}
l.state.Result = &lropb.Operation_Response{Response: &anypb.Any{}}
l.mu.Unlock()
}

Expand Down Expand Up @@ -961,24 +961,24 @@ func spannerValueFromValue(x interface{}) (*structpb.Value, error) {
default:
return nil, fmt.Errorf("unhandled database value type %T", x)
case bool:
return &structpb.Value{Kind: &structpb.Value_BoolValue{x}}, nil
return &structpb.Value{Kind: &structpb.Value_BoolValue{BoolValue: x}}, nil
case int64:
// The Spanner int64 is actually a decimal string.
s := strconv.FormatInt(x, 10)
return &structpb.Value{Kind: &structpb.Value_StringValue{s}}, nil
return &structpb.Value{Kind: &structpb.Value_StringValue{StringValue: s}}, nil
case float64:
return &structpb.Value{Kind: &structpb.Value_NumberValue{x}}, nil
return &structpb.Value{Kind: &structpb.Value_NumberValue{NumberValue: x}}, nil
case string:
return &structpb.Value{Kind: &structpb.Value_StringValue{x}}, nil
return &structpb.Value{Kind: &structpb.Value_StringValue{StringValue: x}}, nil
case []byte:
return &structpb.Value{Kind: &structpb.Value_StringValue{base64.StdEncoding.EncodeToString(x)}}, nil
return &structpb.Value{Kind: &structpb.Value_StringValue{StringValue: base64.StdEncoding.EncodeToString(x)}}, nil
case civil.Date:
// RFC 3339 date format.
return &structpb.Value{Kind: &structpb.Value_StringValue{x.String()}}, nil
return &structpb.Value{Kind: &structpb.Value_StringValue{StringValue: x.String()}}, nil
case time.Time:
// RFC 3339 timestamp format with zone Z.
s := x.Format("2006-01-02T15:04:05.999999999Z")
return &structpb.Value{Kind: &structpb.Value_StringValue{s}}, nil
return &structpb.Value{Kind: &structpb.Value_StringValue{StringValue: s}}, nil
case nil:
return &structpb.Value{Kind: &structpb.Value_NullValue{}}, nil
case []interface{}:
Expand All @@ -991,7 +991,7 @@ func spannerValueFromValue(x interface{}) (*structpb.Value, error) {
vs = append(vs, v)
}
return &structpb.Value{Kind: &structpb.Value_ListValue{
&structpb.ListValue{Values: vs},
ListValue: &structpb.ListValue{Values: vs},
}}, nil
}
}
Expand Down
8 changes: 4 additions & 4 deletions spanner/test/cloudexecutor/executor/internal/utility/executor_to_spanner_value_converter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func executorArrayValueToSpannerValue(t *spannerpb.Type, v *executorpb.Value, nu
}
out := make([]spanner.NullInt64, 0)
for _, value := range v.GetArrayValue().GetValue() {
out = append(out, spanner.NullInt64{value.GetIntValue(), !value.GetIsNull()})
out = append(out, spanner.NullInt64{Int64: value.GetIntValue(), Valid: !value.GetIsNull()})
}
return out, nil
case spannerpb.TypeCode_FLOAT64:
Expand All @@ -202,7 +202,7 @@ func executorArrayValueToSpannerValue(t *spannerpb.Type, v *executorpb.Value, nu
}
out := make([]spanner.NullFloat64, 0)
for _, value := range v.GetArrayValue().GetValue() {
out = append(out, spanner.NullFloat64{value.GetDoubleValue(), !value.GetIsNull()})
out = append(out, spanner.NullFloat64{Float64: value.GetDoubleValue(), Valid: !value.GetIsNull()})
}
return out, nil
case spannerpb.TypeCode_STRING:
Expand All @@ -211,7 +211,7 @@ func executorArrayValueToSpannerValue(t *spannerpb.Type, v *executorpb.Value, nu
}
out := make([]spanner.NullString, 0)
for _, value := range v.GetArrayValue().GetValue() {
out = append(out, spanner.NullString{value.GetStringValue(), !value.GetIsNull()})
out = append(out, spanner.NullString{StringVal: value.GetStringValue(), Valid: !value.GetIsNull()})
}
return out, nil
case spannerpb.TypeCode_BYTES:
Expand Down Expand Up @@ -277,7 +277,7 @@ func executorArrayValueToSpannerValue(t *spannerpb.Type, v *executorpb.Value, nu
if !ok {
return nil, spanner.ToSpannerError(status.Errorf(codes.InvalidArgument, "unexpected string value %q for numeric number", value.GetStringValue()))
}
out = append(out, spanner.NullNumeric{*y, true})
out = append(out, spanner.NullNumeric{Numeric: *y, Valid: true})
}
}
return out, nil
Expand Down

0 comments on commit 966f83d

Please sign in to comment.