Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: don't use zero value of mysql.Result #969

Merged
merged 3 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ func (s *clientTestSuite) TestConn_Insert() {
pkg, err := s.c.Execute(str)
require.NoError(s.T(), err)
require.Equal(s.T(), uint64(1), pkg.AffectedRows)
require.EqualValues(s.T(), 0, pkg.ColumnNumber())
}

func (s *clientTestSuite) TestConn_Insert2() {
Expand Down
8 changes: 4 additions & 4 deletions client/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,10 @@ func (c *Conn) ExecuteMultiple(query string, perResultCallback ExecPerResultCall
// streaming session
// if this would end up in WriteValue, it would just be ignored as all
// responses should have been handled in perResultCallback
return &Result{Resultset: &Resultset{
Streaming: StreamingMultiple,
StreamingDone: true,
}}, nil
rs := NewResultset(0)
rs.Streaming = StreamingMultiple
rs.StreamingDone = true
return NewResult(rs), nil
}

// ExecuteSelectStreaming will call perRowCallback for every row in resultset
Expand Down
12 changes: 6 additions & 6 deletions client/conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,20 +113,20 @@ func (s *connTestSuite) TestExecuteMultiple() {
count := 0
result, err := s.c.ExecuteMultiple(strings.Join(queries, "; "), func(result *mysql.Result, err error) {
switch count {
// the INSERT/DELETE query have no resultset, but should have set affectedrows
// the err should be nil
// also, since this is not the last query, the SERVER_MORE_RESULTS_EXISTS
// flag should be set
case 0, 2:
// the INSERT/DELETE query have empty resultset, but should have set affectedrows
// the err should be nil
// also, since this is not the last query, the SERVER_MORE_RESULTS_EXISTS
// flag should be set
require.True(s.T(), result.Status&mysql.SERVER_MORE_RESULTS_EXISTS > 0)
require.Nil(s.T(), result.Resultset)
require.Equal(s.T(), 0, result.Resultset.RowNumber())
require.Equal(s.T(), uint64(1), result.AffectedRows)
require.NoError(s.T(), err)
case 1:
// the SELECT query should have an resultset
// still not the last query, flag should be set
require.True(s.T(), result.Status&mysql.SERVER_MORE_RESULTS_EXISTS > 0)
require.NotNil(s.T(), result.Resultset)
require.Greater(s.T(), result.Resultset.RowNumber(), 0)
require.NoError(s.T(), err)
case 3:
// this query is obviously bogus so the error should be non-nil
Expand Down
6 changes: 2 additions & 4 deletions client/resp.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (c *Conn) handleOKPacket(data []byte) (*Result, error) {
var n int
var pos = 1

r := new(Result)
r := NewResultReserveResultset(0)

r.AffectedRows, _, n = LengthEncodedInt(data[pos:])
pos += n
Expand Down Expand Up @@ -283,9 +283,7 @@ func (c *Conn) readResultset(data []byte, binary bool) (*Result, error) {
return nil, ErrMalformPacket
}

result := &Result{
Resultset: NewResultset(int(count)),
}
result := NewResultReserveResultset(int(count))

if err := c.readResultColumns(result); err != nil {
return nil, errors.Trace(err)
Expand Down
14 changes: 14 additions & 0 deletions mysql/result.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package mysql

// Result should be created by NewResultWithoutRows or NewResult. The zero value
// of Result is invalid.
type Result struct {
Status uint16
Warnings uint16
Expand All @@ -10,6 +12,18 @@ type Result struct {
*Resultset
}

func NewResult(resultset *Resultset) *Result {
return &Result{
Resultset: resultset,
}
}

func NewResultReserveResultset(fieldCount int) *Result {
return &Result{
Resultset: NewResultset(fieldCount),
}
}

type Executer interface {
Execute(query string, args ...interface{}) (*Result, error)
}
Expand Down
16 changes: 3 additions & 13 deletions mysql/resultset.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const (
StreamingMultiple
)

// Resultset should be created with NewResultset to avoid nil pointer and reduce
// GC pressure.
type Resultset struct {
Fields []*Field
FieldNames map[string]int
Expand Down Expand Up @@ -61,9 +63,7 @@ func (r *Resultset) Reset(fieldsCount int) {
r.RowDatas = r.RowDatas[:0]

if r.FieldNames != nil {
for k := range r.FieldNames {
delete(r.FieldNames, k)
}
clear(r.FieldNames)
} else {
r.FieldNames = make(map[string]int)
}
Expand All @@ -80,22 +80,12 @@ func (r *Resultset) Reset(fieldsCount int) {
}

// RowNumber is returning the number of rows in the [Resultset].
//
// For a nil [Resultset] 0 is returned.
func (r *Resultset) RowNumber() int {
if r == nil {
return 0
}
return len(r.Values)
}

// ColumnNumber is returning the number of fields in the [Resultset].
//
// For a nil [Resultset] 0 is returned.
func (r *Resultset) ColumnNumber() int {
if r == nil {
return 0
}
return len(r.Fields)
}

Expand Down
8 changes: 2 additions & 6 deletions mysql/resultset_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,7 @@ func formatField(field *Field, value interface{}) error {
}

func BuildSimpleTextResultset(names []string, values [][]interface{}) (*Resultset, error) {
r := new(Resultset)

r.Fields = make([]*Field, len(names))
r := NewResultset(len(names))

var b []byte

Expand Down Expand Up @@ -234,9 +232,7 @@ func BuildSimpleTextResultset(names []string, values [][]interface{}) (*Resultse
}

func BuildSimpleBinaryResultset(names []string, values [][]interface{}) (*Resultset, error) {
r := new(Resultset)

r.Fields = make([]*Field, len(names))
r := NewResultset(len(names))

var b []byte

Expand Down
6 changes: 3 additions & 3 deletions mysql/resultset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package mysql
import "testing"

func TestColumnNumber(t *testing.T) {
r := Result{}
// Make sure ColumnNumber doesn't panic if ResultSet is nil
// https://github.com/go-mysql-org/go-mysql/issues/964
r := NewResultReserveResultset(0)
// Make sure ColumnNumber doesn't panic when constructing a Result with 0
// columns. https://github.com/go-mysql-org/go-mysql/issues/964
r.ColumnNumber()
}
8 changes: 1 addition & 7 deletions server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,7 @@ func (h EmptyHandler) HandleQuery(query string) (*Result, error) {
if err != nil {
return nil, err
}
return &mysql.Result{
Status: 0,
Warnings: 0,
InsertId: 0,
AffectedRows: 0,
Resultset: r,
}, nil
return mysql.NewResult(r), nil
}

return nil, fmt.Errorf("not supported now")
Expand Down
2 changes: 1 addition & 1 deletion server/resp.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

func (c *Conn) writeOK(r *Result) error {
if r == nil {
r = &Result{}
r = NewResultReserveResultset(0)
}

r.Status |= c.status
Expand Down
2 changes: 1 addition & 1 deletion server/stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func (c *Conn) handleStmtReset(data []byte) (*Result, error) {

s.ResetParams()

return &Result{}, nil
return NewResultReserveResultset(0), nil
}

// stmt close command has no response
Expand Down
Loading