Skip to content

Insert NULL for []byte(nil) values #147

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

Merged
merged 7 commits into from
Nov 1, 2013
Merged

Insert NULL for []byte(nil) values #147

merged 7 commits into from
Nov 1, 2013

Conversation

julienschmidt
Copy link
Member

This patch makes a behavior change for inserting data into a database:

Currently []byte(nil) is treated the same way as []byte{} / []byte(""), which equals an empty string.This relies on the current language specification: http://golang.org/ref/spec

This PR changes to behavior to treat []byte(nil) as NULL values, therefore the same way as interface{}(nil).
Here is why:

Fixes #144

@arnehormann
Copy link
Member

LGTM after you add a test... 😏

Also reordered the tests so that the general tests are on top
@julienschmidt
Copy link
Member Author

PTAL at the new test. I reordered the tests so that the general tests are on top and fail first.
The new test starts at line 504.

Any objections against this change?

@arnehormann
Copy link
Member

I'd prefer to put (future) reorderings into an extra PR and keep functional and layout changes separate.
LGTM nontheless.

@julienschmidt
Copy link
Member Author

PTAL, I found another bug: Empty strings / []byte("") produced false nil values

@@ -632,40 +632,31 @@ func skipLengthEnodedString(b []byte) (int, error) {
return n, io.EOF
}

func readLengthEncodedInteger(b []byte) (num uint64, isNull bool, n int) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave these in please. They are a pretty concise documentation of what is returned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still prefer the short form below. We don't have to use the vars, I just like to keep them.

@arnehormann
Copy link
Member

LGTM when Travis says so

julienschmidt added a commit that referenced this pull request Nov 1, 2013
Insert NULL for []byte(nil) values
@julienschmidt julienschmidt merged commit 4615006 into master Nov 1, 2013
@julienschmidt julienschmidt deleted the byte_nil branch November 1, 2013 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[]byte convert to NULL
2 participants