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

database/sql: memory error when scanning binary data with type sql.RawBytes for unicode characters #22039

Closed
mcspring opened this issue Sep 26, 2017 · 5 comments
Assignees

Comments

@mcspring
Copy link
Contributor

@mcspring mcspring commented Sep 26, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version devel +8e2d90dca8 Tue Sep 26 04:39:19 2017 +0000 darwin/amd64

Does this issue reproduce with the latest release?

YES

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"

What did you do?

I tried to read records from MySQL which stored binary data (image) to golang struct which defines data type as sql.RawBytes.

What did you expect to see?

Each field setups with correct value from MySQL, including unicode characters.

What did you see instead?

It works well for unicode characters when reading only one record. But it'll mess memory for more than one records and incorrect unicode characters already read to previous struct fields.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 26, 2017

Please provide more details, such as a program that demonstrates the problem. Thanks.

CC @kardianos

@kardianos
Copy link
Contributor

@kardianos kardianos commented Sep 26, 2017

It is hard to tell from your description, but have you tried copying the data out of the RawBytes container between each row? You should be able to use the copy function built-in.

rows.Scan(&rawBytesField)
copy(rowBytes, []byte(rawBytesField))
@kardianos kardianos self-assigned this Sep 26, 2017
@mcspring
Copy link
Contributor Author

@mcspring mcspring commented Sep 27, 2017

@ianlancetaylor

  • Here is the main codes I used.
func (m *Model) Query(query string, args ...interface{}) (result []map[string]interface{}, err error) {
	rows, err := m.db.Query(query, args...)
	if err != nil {
		return
	}

	columns, err := rows.Columns()
	if err != nil {
		return
	}
	scanValues := make([]interface{}, len(columns))

	for rows.Next() {
		for i := 0; i < len(columns); i++ {
			var value sql.RawBytes

			scanValues[i] = &value
		}

		err = rows.Scan(scanValues...)
		if err != nil {
			break
		}

		record := map[string]interface{}{}

		for i, col := range scanValues {
			record[columns[i]] = *(col.(*sql.RawBytes))
		}

		result = append(result, record)
	}

	if err = rows.Err(); err != nil {
		return
	}

	if len(result) == 0 {
		err = ErrNotFound
	}

	return
}
  • And here is the database definitions
*************************** 1. row ***************************
  Field: id
   Type: int(11)
   Null: NO
    Key: PRI
Default: NULL
  Extra: auto_increment
*************************** 2. row ***************************
  Field: name
   Type: varchar(32)
   Null: NO
    Key: MUL
Default:
  Extra:
*************************** 3. row ***************************
  Field: avatar
   Type: blob
   Null: YES
    Key:
Default: NULL
  Extra:

@kardianos After some research, I found this issue relates to two pieces of the codes within database/sql package.

For this, we should use cloneBytes method when converting sql.RawBytes to dest.

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index 3c387fb25c..8db4212f98 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -259,7 +259,7 @@ func convertAssign(dest, src interface{}) error {
                        if d == nil {
                                return errNilPtr
                        }
-                       *d = s
+                       *d = cloneBytes(s)
                        return nil
                }
        case time.Time:

PS: @kardianos copy the scanned value will work for the scene. And I tend to fix it from underline.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 31, 2017

From the docs:

RawBytes is a byte slice that holds a reference to memory owned by the database itself. After a Scan into a RawBytes, the slice is only valid until the next call to Next, Scan, or Close.

So if you use RawBytes and you don't copy it out yourself, you're not working with it correctly.

@mcspring
Copy link
Contributor Author

@mcspring mcspring commented Nov 1, 2017

@kardianos That sounds reasonable. Thx!

@mcspring mcspring closed this Nov 1, 2017
@golang golang locked and limited conversation to collaborators Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.