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

* *: don't use zero value of mysql.Result Signed-off-by: lance6716 <[email protected]> * try to fix CI Signed-off-by: lance6716 <[email protected]> * fix CI Signed-off-by: lance6716 <[email protected]> --------- Signed-off-by: lance6716 <[email protected]>
go-mysql-org · Jan 14, 2025 · d02e79a · d02e79a
1 parent 7108b1d
commit d02e79a
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 45 deletions.
diff --git a/client/client_test.go b/client/client_test.go
@@ -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() {

diff --git a/client/conn.go b/client/conn.go
@@ -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

diff --git a/client/conn_test.go b/client/conn_test.go
@@ -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

diff --git a/client/resp.go b/client/resp.go
@@ -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
@@ -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)

diff --git a/mysql/result.go b/mysql/result.go
@@ -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
@@ -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)
 }

diff --git a/mysql/resultset.go b/mysql/resultset.go
@@ -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
@@ -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)
 	}
@@ -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)
 }
 

diff --git a/mysql/resultset_helper.go b/mysql/resultset_helper.go
@@ -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
 
@@ -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
 

diff --git a/mysql/resultset_test.go b/mysql/resultset_test.go
@@ -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()
 }
diff --git a/server/command.go b/server/command.go
@@ -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")

diff --git a/server/resp.go b/server/resp.go
@@ -10,7 +10,7 @@ import (
 
 func (c *Conn) writeOK(r *Result) error {
 	if r == nil {
-		r = &Result{}
+		r = NewResultReserveResultset(0)
 	}
 
 	r.Status |= c.status

diff --git a/server/stmt.go b/server/stmt.go
@@ -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