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

#BUG: sql.Execute get rows count will panic nil error #964

Closed
DoMySon opened this issue Jan 4, 2025 · 11 comments · Fixed by #969
Closed

#BUG: sql.Execute get rows count will panic nil error #964

DoMySon opened this issue Jan 4, 2025 · 11 comments · Fixed by #969

Comments

@DoMySon
Copy link

DoMySon commented Jan 4, 2025

var result, _ = db.Execute(`insert into (x,y,z) values(1,2,3)`)

var columns = result.Columns()  // there result != nil
var rows  = result.Rows()       // there result == nil (panic)

@dveeden dveeden added the bug label Jan 4, 2025
@dveeden
Copy link
Collaborator

dveeden commented Jan 4, 2025

Is this with github.com/go-mysql-org/go-mysql/driver or with github.com/go-mysql-org/go-mysql/client?

What version?

Could you give a full example that I can run to reproduce this?

Is there a stack ?

@DoMySon
Copy link
Author

DoMySon commented Jan 10, 2025

it's github.com/go-mysql-org/go-mysql/client, version is v1.10.0
i just execute an insert operation.
image

@DoMySon
Copy link
Author

DoMySon commented Jan 10, 2025

first result not nil, but next operation always nil then panic

@dveeden dveeden added the client label Jan 10, 2025
@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

I can reproduce this with this code:

package main

import (
	"fmt"

	"github.com/go-mysql-org/go-mysql/client"
	_ "github.com/go-mysql-org/go-mysql/client"
)

func main() {
	fd, err := client.Connect("127.0.0.1:3306", "root", "", "test")
	if err != nil {
		panic(err)
	}
	fmt.Printf("fd = %#v\n", fd)

	fd.Execute(`drop table if exists t`)
	fd.Execute(`create table t(x int, y int, z int, primary key (x,y,z))`)

	result, err := fd.Execute(`insert into t(x,y,z) values(1,2,3)`)
	if err != nil {
		panic(err)
	}
	fmt.Printf("result = %#v\n", result)

	rows := result.RowNumber()
	fmt.Printf("rows = %#v\n", rows)

	columns := result.ColumnNumber()
	fmt.Printf("columns = %#v\n", columns)
}

Result:

$ go run main.go 
fd = &client.Conn{Conn:(*packet.Conn)(0xc0000fa980), user:"root", password:"", db:"test", tlsConfig:(*tls.Config)(nil), proto:"tcp", ReadTimeout:0, WriteTimeout:0, BufferSize:65536, serverVersion:"9.1.0", capability:0xdfffffff, ccaps:0x0, attributes:map[string]string{"_client_name":"go-mysql", "_os":"linux", "_platform":"amd64", "_runtime_version":"go1.23.4"}, status:0x2, charset:"utf8", collation:"", salt:[]uint8{0x1d, 0x53, 0x6c, 0xa, 0x6f, 0x3f, 0x7e, 0x1f, 0x31, 0x40, 0x42, 0x23, 0x33, 0x66, 0x60, 0x6d, 0x7f, 0xb, 0x69, 0x3c}, authPluginName:"caching_sha2_password", connectionID:0x15}
result = &mysql.Result{Status:0x2, Warnings:0x0, InsertId:0x0, AffectedRows:0x1, Resultset:(*mysql.Resultset)(nil)}
rows = 0
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x611500]

goroutine 1 [running]:
github.com/go-mysql-org/go-mysql/mysql.(*Resultset).ColumnNumber(0x800620?)
	/home/dvaneeden/go/pkg/mod/github.com/go-mysql-org/[email protected]/mysql/resultset.go:90
main.main()
	/tmp/issue_964/main.go:29 +0x1c5
exit status 2

This is on Linux with MySQL 9.1.0

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

The problem here is that ColumnNumber() uses r.Fields while r is nil.

(dlv) s
> github.com/go-mysql-org/go-mysql/mysql.(*Resultset).ColumnNumber() /home/dvaneeden/go/pkg/mod/github.com/go-mysql-org/[email protected]/mysql/resultset.go:89 (PC: 0x786264)
    84:			return 0
    85:		}
    86:		return len(r.Values)
    87:	}
    88:	
=>  89:	func (r *Resultset) ColumnNumber() int {
    90:		return len(r.Fields)
    91:	}
    92:	
    93:	func (r *Resultset) GetValue(row, column int) (interface{}, error) {
    94:		if row >= len(r.Values) || row < 0 {
(dlv) p r
*github.com/go-mysql-org/go-mysql/mysql.Resultset nil

@DoMySon
Copy link
Author

DoMySon commented Jan 10, 2025

Yes, I use a more violent approach here to directly judge

...
if r == nil {
   return 0
}
...

@dveeden
Copy link
Collaborator

dveeden commented Jan 10, 2025

@DoMySon isn't that the same as what #968 does?

@DoMySon
Copy link
Author

DoMySon commented Jan 10, 2025

@dveeden It's the same

@DoMySon
Copy link
Author

DoMySon commented Jan 10, 2025

However, this situation is not intuitive. In fact, unless it is manually cleared, it will not be empty. There may be a deeper bug.

@lance6716
Copy link
Collaborator

@DoMySon Can you provide a full runnable, reproduce code for us? Or answer my questions to explain what's happened in your example?

var result, _ = db.Execute(`insert into (x,y,z) values(1,2,3)`)

var columns = result.Columns()  // there result != nil
var rows  = result.Rows()       // there result == nil (panic)
  1. Why didn't you check the error returned from db.Execute? When there's some error, result can be nil.
  2. The result in three lines are not related? Why it's not nil and then no modification made but it becomes nil?

@lance6716
Copy link
Collaborator

lance6716 commented Jan 10, 2025

Oh I see your screenshot.

-- update --

The root cause is we always return a not-nil mysql.Result, but it embeds a *Resultset which can be nil. So when calls methods on *Resultset the function receiver is nil, then causes panic.

All exported methods on *Resultset should consider this case because user may call them. I'll keep this issue open and create another PR to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants