add UniqueIdentifier type #203
Conversation
SQL Server's UniqueIdentifier type uses native endianness for some of its fields within the UUID. Add a new type, UniqueIdentifier, that correctly handles scanning based on the type of data being scanned. Scanning a []byte swaps the bytes of the fields that SQL Server stores using native endianness. Scanning a string decodes the hex-encoded values without swapping any bytes. Implement fmt.Stringer so that the returned value of String is congruent with the behavior of CAST(myuniqueidentifer AS varchar(36)).
|
|
||
| var nilUUID = make([]byte, 16) // RFC 4122 section 4.1.7 says a nil UUID is all zeros. | ||
|
|
||
| type UniqueIdentifier []byte |
There was a problem hiding this comment.
Why is this a slice rather than an array. I would expect:
type UUID [16]byte
|
|
||
| switch vt := v.(type) { | ||
| case []byte: | ||
| if len(vt) != 16 { |
There was a problem hiding this comment.
This check would go away then.
There was a problem hiding this comment.
Yes, you're correct about that this should remain.
| } | ||
| } | ||
|
|
||
| *u = make(UniqueIdentifier, 16) |
There was a problem hiding this comment.
Then UUID wouldn't need an alloc.
| if len(b) != 16 { | ||
| b = nilUUID | ||
| } | ||
| return fmt.Sprintf("%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:]) |
There was a problem hiding this comment.
In sql server, select newid(); returns letters in capitals.
Maybe fmt.Sprintf("%X-%X-%X-%X-%X", ...)
| } | ||
|
|
||
| func (u UniqueIdentifier) Equal(u2 UniqueIdentifier) bool { | ||
| return bytes.Equal(u, u2) |
There was a problem hiding this comment.
If an array, it would be return u == u2.
Change the underlying type of UniqueIdentifier to ([]byte -> [16]byte). Changing the type to an array also allowed UniqueIdentifier.Equal to be removed since array types are comparable. Modify UniqueIdentifier.String to return a string that uses capitalized hex encoding for closer parity with SQL Server's conversion of its UniqueIdentifer type to character types.
|
Thanks for the feedback, @kardianos. The only suggestion that I wasn't comfortable taking was about eliminating the check for the length of the PTAL |
| } | ||
|
|
||
| } | ||
| func TestUniqueIdentifierImplementsScanner(t *testing.T) { |
There was a problem hiding this comment.
Remove this test and add a line above the implementation:
var _ sql.Scanner = &UniqueIdentifier{}
var _ fmt.Stringer = UniqueIdentifier{}
var _ driver.Valuer = UniqueIdentifier{}
|
|
||
| switch vt := v.(type) { | ||
| case []byte: | ||
| if len(vt) != 16 { |
There was a problem hiding this comment.
Yes, you're correct about that this should remain.
| "fmt" | ||
| ) | ||
|
|
||
| type UniqueIdentifier [16]byte |
There was a problem hiding this comment.
Can we call this UUID? Or is there a reason not to? I'm not sure I care too much, but I can never seem to type UniqueIdentifier right if I'm not paying attention...
There was a problem hiding this comment.
It seems more appropriate to me to keep the name congruent with the SQL Server data type that it represents.
Favor compile-time type checking over type assertion to verify interface implementations of UniqueIdentifier.
|
PTAL |
* Vulnerabilty depency x/net * Att Dependencies
SQL Server's UniqueIdentifier type uses native endianness for some of
its fields within the UUID. Add a new type, UniqueIdentifier, that
correctly handles scanning based on the type of data being scanned.
Scanning a []byte swaps the bytes of the fields that SQL Server stores
using native endianness.
Scanning a string decodes the hex-encoded values without swapping any
bytes.
Implement fmt.Stringer so that the returned value of String is congruent
with the behavior of CAST(myuniqueidentifer AS varchar(36)).