uniqueidentifier formatting on sql server

[birkmose]

Hi!,

When displaying uniqueidentifier columns on SQL server the output looks binary and not formatted?

E.g.

Connected with driver sqlserver (Microsoft SQL Server 16.0.5100.7245, RTM, Azure SQL Edge Developer (64-bit))
Type "help" for help.

ms:sa@localhost/master=> \d FooTable
                    BASE TABLE "FooTable"
           Name           |       Type       | Nullable | Default 
--------------------------+------------------+----------+---------
 Id                 | uniqueidentifier | "NO"     |  

ms:sa@localhost/master=> SELECT TOP 1 Id from FooTable;
               Id                
---------------------------------------
 F}\xa71\x83\xe4kN\x8aU\x03/\xcc\xf8WS 
(1 row)

ms:sa@localhost/master=> SELECT TOP 1 Id, convert(nvarchar(50), Id) ConvertedId from FooTable;
               Id                |                  ConvertedId                   
---------------------------------------+--------------------------------------
 F}\xa71\x83\xe4kN\x8aU\x03/\xcc\xf8WS | 31A77D46-E483-4E6B-8A55-032FCCF85753 
(1 row)

Saw some upstream issue that might be related to this?: https://github.com/microsoft/go-mssqldb/issues/92

[kenshaw]

Formatting issues like this are because that's what the underlying database driver is returning, in this case raw bytes. That gets escaped/displayed as shown. To get better formatting, try casting/converting this field. Microsoft's documentation gives this:

CONVERT(CHAR(255), id_field) AS 'char'

This should get back to usql as a string and then be displayed properly.

[birkmose]

Is there any way to customize (e.g. even a local build/patch/fork) usql to format it on the clientside without having to do the dance of modifying the query? I really like the look of usql, but if I get uniqueidentifiers formatted as binary blobs, that will severely limit the utility for my use-case at least. I'd have hoped that I could do stuff like select top 10 * from Foo and do exploration of datashapes.

[kenshaw]

@birkmose I've looked into the underlying type that should be returned by the driver, and it has a .String() method, so it should have been picked up by the tblfmt package properly. I'm trying to look into what the issue is, but for some reason Microsoft's SQL Server container images no longer work for me under Linux, even though I had been testing/using the same container image a week ago. I'll get to the bottom of this, as this is either an error in the underlying driver, or a minor logic issue with the tblfmt package. I can't debug/test this, however, without having a database to test against.

[kenshaw]

@birkmose I've been reading the SQL Server driver code, and it doesn't appear that it's ever allocating a UniqueIdentifier type in a call to .Scan. As such, this is likely an oversight or on purpose in the github.com/microsoft/go-mssqldb code base. I won't be able to know until I can get the container image working again.

[birkmose]

This is the image i tested with on linux:

mcr.microsoft.com/azure-sql-edge:latest

[kenshaw]

Breakage: https://github.com/microsoft/mssql-docker/issues/868

[kenshaw]

@birkmose After downgrading a kernel or two, I was able to get the container to run. The type being returned from the driver is indeed []uint8. Currently, this is being displayed properly by usql. The fix to this would be for the driver to correctly return a mssql.UniqueIdentifier type. That would require a change to Microsoft's SQL Server Go driver.

[kenshaw]

@birkmose I've dug deep into the SQL Server driver code, and it is not easy to fix this. From what I can tell, there's an internal call to parseRow that is effectively dropping the type information. There's a convertAssign func (that, from my reading, would not work, regardless, as nowhere in the codebase is a UniqueIdentifier ever created). Separately, the actual convertAssign doesn't but it doesn't seem to be in path that usql, and, subsequently, tblfmt call.

I could add a hack to usql that adds formatting for []byte of specific length 16, but even then this would be extremely broken, hackish, and really not what should be done.

The SQL Server driver just isn't up to the same kind of functionality/support that other database drivers are. Other drivers will convert to the "best type" when presented with a *interface{} on a .Scan call.

That said, the ultimate resolution to this would be to politely ask Microsoft to fix this in their code. I will implement the hack and push it to the tree, but the usql codebase is being overhauled at the moment for a greater set of features, and thus I won't be able to cut a release any time soon. The hack I'll implement should be available in the next couple hours.

[kenshaw]

@birkmose I think I have a non-hacky PR that I can get the MS repo to accept, which will require no changes to usql or tblfmt, but of course will require building from source until a new usql release is cut (assuming the PR is accepted).

[kenshaw]

@birkmose you can compile/use this from source yourself:

$ mkdir src
$ cd src
$ git clone https://github.com/kenshaw/go-mssqldb.git
$ git clone https://github.com/xo/usql.git
$ cd usql
$ go work init
$ go work use .
$ go work edit -replace github.com/microsoft/go-mssqldb=../go-mssqldb
$ go install

[birkmose]

Thanks @kenshaw!

The uniqueidentifier now works, but seems that master is broken - getting errors like: error: sqlserver: sql: Scan error on column index 2, name "SomeColumnName": converting NULL to string is unsupported

(for a column nvarchar(255) which is nullable).

[kenshaw]

@birkmose Ok; and that wasn't happening before?

[birkmose]

Not from the v0.17.5 branch no. Just tested on that branch again, and works fine there :)

[kenshaw]

I'll look into it.

[kenshaw]

@birkmose I pushed a new commit to fix this, without requiring any changes to the underlying driver. Please check it out and test it.

[birkmose]

Sorry for the late answer @kenshaw , got caught up in some other stuff. I can confirm that a simple git clone https://github.com/xo/usql.git; go install works as expected - I know see uniqueidentifier formatted as expected - thank you very much!