Skip to content

Commit c342f65

Browse files
egonelbreegonelbre
authored
perf(spanner): better error handling (#10734)
- perf(spanner): avoid using fmt.Errorf unnecessarily - perf(spanner): avoid duplicated errors.New in UnmarshalJSON - fix(spanner): error strings should not be capitalized Updates #9749
1 parent bbe7b9c commit c342f65

File tree

12 files changed

+43
-35
lines changed

12 files changed

+43
-35
lines changed

‎spanner/client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func validDatabaseName(db string) error {
9595
func parseDatabaseName(db string) (project, instance, database string, err error) {
9696
matches := validDBPattern.FindStringSubmatch(db)
9797
if len(matches) == 0 {
98-
return "", "", "", fmt.Errorf("Failed to parse database name from %q according to pattern %q",
98+
return "", "", "", fmt.Errorf("failed to parse database name from %q according to pattern %q",
9999
db, validDBPattern.String())
100100
}
101101
return matches[1], matches[2], matches[3], nil

‎spanner/client_benchmarks_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ package spanner
1818

1919
import (
2020
"context"
21-
"fmt"
21+
"errors"
2222
"math/rand"
2323
"reflect"
2424
"sync"
@@ -87,7 +87,7 @@ func createBenchmarkServer(incStep uint64) (server *MockedSpannerInMemTestServer
8787
if uint64(client.idleSessions.idleList.Len()) == client.idleSessions.MinOpened {
8888
return nil
8989
}
90-
return fmt.Errorf("not yet initialized")
90+
return errors.New("not yet initialized")
9191
})
9292
return
9393
}

‎spanner/client_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1930,7 +1930,7 @@ func TestClient_ReadWriteTransaction_BufferedWriteBeforeSqlStatementWithError(t
19301930
// We ignore the error and proceed to commit the transaction.
19311931
_, err := tx.Update(ctx, NewStatement(UpdateBarSetFoo))
19321932
if err == nil {
1933-
return fmt.Errorf("missing expected InvalidArgument error")
1933+
return errors.New("missing expected InvalidArgument error")
19341934
}
19351935
return nil
19361936
})

‎spanner/integration_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2840,7 +2840,7 @@ func TestIntegration_StructTypes(t *testing.T) {
28402840
return fmt.Errorf("len(rows) = %d; want 1", len(rows))
28412841
}
28422842
if !rows[0].Valid {
2843-
return fmt.Errorf("rows[0] is NULL")
2843+
return errors.New("rows[0] is NULL")
28442844
}
28452845
var i, j int64
28462846
if err := rows[0].Row.Columns(&i, &j); err != nil {

‎spanner/row.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,10 +418,10 @@ func (r *Row) ToStructLenient(p interface{}) error {
418418
// err := spanner.SelectAll(row, &singersByPtr, spanner.WithLenient())
419419
func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptions) error {
420420
if rows == nil {
421-
return fmt.Errorf("rows is nil")
421+
return errors.New("rows is nil")
422422
}
423423
if destination == nil {
424-
return fmt.Errorf("destination is nil")
424+
return errors.New("destination is nil")
425425
}
426426
dstVal := reflect.ValueOf(destination)
427427
if !dstVal.IsValid() || (dstVal.Kind() == reflect.Ptr && dstVal.IsNil()) {

‎spanner/session_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"bytes"
2121
"container/heap"
2222
"context"
23+
"errors"
2324
"fmt"
2425
"math/rand"
2526
"reflect"
@@ -279,7 +280,7 @@ func TestTakeFromIdleListChecked(t *testing.T) {
279280
numOpened := uint64(sp.idleList.Len())
280281
sp.mu.Unlock()
281282
if numOpened < sp.SessionPoolConfig.incStep-1 {
282-
return fmt.Errorf("creation not yet finished")
283+
return errors.New("creation not yet finished")
283284
}
284285
return nil
285286
})
@@ -1900,7 +1901,7 @@ func TestMaintainer_DeletesSessions(t *testing.T) {
19001901
sp.mu.Lock()
19011902
defer sp.mu.Unlock()
19021903
if sp.numOpened > 0 {
1903-
return fmt.Errorf("session pool still contains more than 0 sessions")
1904+
return errors.New("session pool still contains more than 0 sessions")
19041905
}
19051906
return nil
19061907
})
@@ -2023,7 +2024,7 @@ func TestSessionCreationIsDistributedOverChannels(t *testing.T) {
20232024
numOpened := uint64(sp.idleList.Len())
20242025
sp.mu.Unlock()
20252026
if numOpened < spc.MinOpened {
2026-
return fmt.Errorf("not yet initialized")
2027+
return errors.New("not yet initialized")
20272028
}
20282029
return nil
20292030
})

‎spanner/sessionclient_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package spanner
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"sync"
2324
"testing"
@@ -260,7 +261,7 @@ func TestBatchCreateAndCloseSession(t *testing.T) {
260261
client.idleSessions.mu.Lock()
261262
defer client.idleSessions.mu.Unlock()
262263
if client.idleSessions.multiplexedSession == nil {
263-
return fmt.Errorf("multiplexed session not created yet")
264+
return errors.New("multiplexed session not created yet")
264265
}
265266
return nil
266267
})
@@ -475,7 +476,7 @@ func TestBatchCreateSessions_ServerExhausted(t *testing.T) {
475476
if isMultiplexEnabled {
476477
waitFor(t, func() error {
477478
if client.idleSessions.multiplexedSession == nil {
478-
return fmt.Errorf("multiplexed session not created yet")
479+
return errors.New("multiplexed session not created yet")
479480
}
480481
return nil
481482
})

‎spanner/spannertest/db_eval.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package spannertest
2020

2121
import (
2222
"bytes"
23+
"errors"
2324
"fmt"
2425
"regexp"
2526
"strconv"
@@ -284,7 +285,7 @@ func (ec evalContext) evalArithOp(e spansql.ArithOp) (interface{}, error) {
284285
}
285286
if rhs == 0 {
286287
// TODO: Does real Spanner use a specific error code here?
287-
return nil, fmt.Errorf("divide by zero")
288+
return nil, errors.New("divide by zero")
288289
}
289290
return lhs / rhs, nil
290291
case spansql.Add, spansql.Sub, spansql.Mul:
@@ -714,7 +715,7 @@ func (ec evalContext) evalExtractExpr(expr spansql.ExtractExpr) (result interfac
714715
return int64(v.Year), nil
715716
}
716717
}
717-
return nil, fmt.Errorf("Extract with part %v not supported", expr.Part)
718+
return nil, fmt.Errorf("extract with part %v not supported", expr.Part)
718719
}
719720

720721
func (ec evalContext) evalAtTimeZoneExpr(expr spansql.AtTimeZoneExpr) (result interface{}, err error) {
@@ -916,7 +917,7 @@ func (ec evalContext) colInfo(e spansql.Expr) (colInfo, error) {
916917
return colInfo{}, err
917918
}
918919
if ci.Type.Array {
919-
return colInfo{}, fmt.Errorf("can't nest array literals")
920+
return colInfo{}, errors.New("can't nest array literals")
920921
}
921922
ci.Type.Array = true
922923
return ci, nil

‎spanner/spannertest/db_query.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package spannertest
1818

1919
import (
20+
"errors"
2021
"fmt"
2122
"io"
2223
"sort"
@@ -445,7 +446,7 @@ func (d *database) evalSelect(sel spansql.Select, qc *queryContext) (si *selIter
445446
// First stage is to identify the data source.
446447
// If there's a FROM then that names a table to use.
447448
if len(sel.From) > 1 {
448-
return nil, fmt.Errorf("selecting with more than one FROM clause not yet supported")
449+
return nil, errors.New("selecting with more than one FROM clause not yet supported")
449450
}
450451
if len(sel.From) == 1 {
451452
var err error
@@ -751,11 +752,11 @@ func (d *database) evalSelectFrom(qc *queryContext, ec evalContext, sf spansql.S
751752

752753
func newJoinIter(lhs, rhs *rawIter, lhsEC, rhsEC evalContext, sfj spansql.SelectFromJoin) (*joinIter, evalContext, error) {
753754
if sfj.On != nil && len(sfj.Using) > 0 {
754-
return nil, evalContext{}, fmt.Errorf("JOIN may not have both ON and USING clauses")
755+
return nil, evalContext{}, errors.New("JOIN may not have both ON and USING clauses")
755756
}
756757
if sfj.On == nil && len(sfj.Using) == 0 && sfj.Type != spansql.CrossJoin {
757758
// TODO: This isn't correct for joining against a non-table.
758-
return nil, evalContext{}, fmt.Errorf("non-CROSS JOIN must have ON or USING clause")
759+
return nil, evalContext{}, errors.New("non-CROSS JOIN must have ON or USING clause")
759760
}
760761

761762
// Start with the context from the LHS (aliases and params should be the same on both sides).

‎spanner/spannertest/inmem.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ package spannertest
5050
import (
5151
"context"
5252
"encoding/base64"
53+
"errors"
5354
"fmt"
5455
"io"
5556
"log"
@@ -629,10 +630,10 @@ func (s *server) StreamingRead(req *spannerpb.ReadRequest, stream spannerpb.Span
629630
}
630631
if len(req.ResumeToken) > 0 {
631632
// This should only happen if we send resume_token ourselves.
632-
return fmt.Errorf("read resumption not supported")
633+
return errors.New("read resumption not supported")
633634
}
634635
if len(req.PartitionToken) > 0 {
635-
return fmt.Errorf("partition restrictions not supported")
636+
return errors.New("partition restrictions not supported")
636637
}
637638

638639
var ri rowIter

0 commit comments

Comments
 (0)