search

microsoft / go-mssqldb

Microsoft SQL server driver written in go language
BSD 3-Clause "New" or "Revised" License
283 stars 63 forks source link

Change returned reflect.Value in Rows.ColumnTypeScanType to UniqueIdentifer for GUID types #172

Closed kenshaw closed 7 months ago

kenshaw commented 7 months ago

The Rows.ColumnScanType is returning reflect.TypeOf([]byte{}) for GUID types (typeGuid). This changes it so that it instead returns reflect.TypeOf(UniqueIdentifier{}), which has the knock down effects of using packages being able to correctly ascertain the column type for for UniqueIdentifier's.

This is needed for use by usql (see xo/usql#439) in order to correctly format/display GUIDs cleanly and as users expect.

shueybubbles commented 7 months ago

thx for opening a PR. Is your change based on the desire to have the driver swap the bytes instead of clients having to know about SQL's difference in byte order?

This change feels like it needs a broader community discussion, though. What applications could it break?

sqlcmd already expects it to be []byte. Does this change warrant a major version upgrade to make it clear that apps like sqlcmd will need to change their code appropriately?

shueybubbles commented 7 months ago

Related issues from the original driver https://github.com/denisenkom/go-mssqldb/issues/405 https://github.com/denisenkom/go-mssqldb/issues/732

Does this change affect the ability to scan a null value? See https://github.com/denisenkom/go-mssqldb/issues/725

kenshaw commented 7 months ago

@shueybubbles Prior to submitting this PR, I did not look at what other SQL drivers do, nor did I read the Go sql/driver documentation properly. This driver is likely doing the right thing and returning reflect.TypeOf([]byte{}) in this instance. I believe I know what I should do in usql that will not require any changes to this driver.

kenshaw commented 7 months ago

@shueybubbles Apologies for the time involved reviewing this, and do appreciate it. Thanks!