Microsoft.Data.Sqlite: Store Guids as TEXT

[bricelam]

Yet another regret like #15019 and #15020 is the decision to store GUID values as BLOB. Now that we know how undefined the binary format of a GIUD is, storing them as TEXT probably would have been a better choice. Should we try and change it in 3.0?

[ajcvickers]

Notes from triage: let's do this, but with guidance on:

• How to run an in-place update of values (if possible)
• How to use a value converter to get back to the old behavior

[bricelam]

UPDATE MyTable
SET GuidColumn = hex(substr(GuidColumn, 4, 1)) ||
                 hex(substr(GuidColumn, 3, 1)) ||
                 hex(substr(GuidColumn, 2, 1)) ||
                 hex(substr(GuidColumn, 1, 1)) || '-' ||
                 hex(substr(GuidColumn, 6, 1)) ||
                 hex(substr(GuidColumn, 5, 1)) || '-' ||
                 hex(substr(GuidColumn, 8, 1)) ||
                 hex(substr(GuidColumn, 7, 1)) || '-' ||
                 hex(substr(GuidColumn, 9, 2)) || '-' ||
                 hex(substr(GuidColumn, 11, 6))
WHERE typeof(GuidColumn) == 'blob';

modelBuilder
    .Entity<MyEntity>()
    .Property(e => e.GuidProperty)
    .HasConversion(
        g => g.ToByteArray(),
        b => new Guid(b));

[jol64]

• How to use a value converter to get back to the old behavior

actually I am not so much interested in whether you store GUIDs as BLOBs (my preference) or TEXT, but I´d be very much interested in how to implement value converters as I am trying to optimize storage of my own data, and this sounds the better approach then doing that whenever composing a statement. I do hope it can be done in C# and does not require C(++).

Forgive my pickiness, but https://github.com/aspnet/Microsoft.Data.Sqlite/wiki/Data-Type-Mappings still lists BLOB, and should get a comment changing as of...
Thanks, Joachim

[bricelam]

See Value Conversions in the EF Core docs.

As the aspnet/Microsoft.Data.Sqlite repo says, it is obsolete and has been merged into aspnet/EntityFrameworkCore. Anything there should be considered outdated.

[MaceWindu]

@bricelam

Now that we know how undefined the binary format of a GIUD is, storing them as TEXT probably would have been a better choice

could you provide more details on what's wrong with guid as binary ?

[bricelam]

@MaceWindu If you write the value using .NET's Guid.ToByteArray(), but read the value in, say, Java, you'll get a different GUID value since the binary format of GUID values is not defined.

[jol64]

I tend to disagree. The binary format is defined in https://tools.ietf.org/html/rfc4122 chapter 4. I suspect most of the issues arise from the "network order" which is not really the natural order for Intel processors.

[MaceWindu]

@bricelam

That's good point. SQLite's weak type system doesn't make life easier when you need interoperability.
Still text doesn't solve this issue completely as it doesn't fix sql equality operations when different Guid formats used (which are more than two).

[jol64]

guids used as keys are bad for databases anyway, you´ll figure out that once your database becomes IO bound...
having said that, I´d suggest to follow RFC 4122, have type conversions for the typical MS string and guid data types, and otherwise document a warning...

[bricelam]

it doesn't fix sql equality operations when different Guid formats used

Correct. Convert everything to text: (you can do this in a Migration for each of your tables)

-- Convert Guid values from BLOB to TEXT
UPDATE myTable
SET guidColumn = hex(substr(guidColumn, 4, 1)) ||
                 hex(substr(guidColumn, 3, 1)) ||
                 hex(substr(guidColumn, 2, 1)) ||
                 hex(substr(guidColumn, 1, 1)) || '-' ||
                 hex(substr(guidColumn, 6, 1)) ||
                 hex(substr(guidColumn, 5, 1)) || '-' ||
                 hex(substr(guidColumn, 8, 1)) ||
                 hex(substr(guidColumn, 7, 1)) || '-' ||
                 hex(substr(guidColumn, 9, 2)) || '-' ||
                 hex(substr(guidColumn, 11, 6))
WHERE typeof(guidColumn) == 'blob';

[bricelam]

@jol64 I don't know the exact details of how the bytes were being interpreted differently, but we did get reports saying that they were

[bricelam]

Maybe the confusion was on the "most significant" byte of the nodes...

[Leon99]

Hey @bricelam, is there a way to make Microsoft.EntityFrameworkCore.Sqlite convert Guids to text in lower case? And just out of curiosity, what was the reason to use the upper case format?

[bricelam]

modelBuilder
    .Entity<MyEntity>()
    .Property(e => e.GuidProperty)
    .HasConversion(
        g => g.ToString(),
        s => new Guid(s));

Apparently, I felt like upper case was "more canonical" in 2017. 🤷‍♂️

[Leon99]

@bricelam any chances to change that to make it in line with the RFC/ITU-T/rest of .NET Framework? Or, at the very least, document that behaviour/workaround? It costed me a few hours of bashing my head against a wall (see #19651). BTW I'm not sure I'm fine with you workaround - I use SQLite for testing and modifying code to accommodate specifics of the testing tooling doesn't sound like a good idea.

[bricelam]

That's a hard call to make--it's a considerable breaking change. Is it worth breaking all existing databases just to make yours work without a workaround? Can you just alter the data in your database? It wouldn't be an issue if the values were inserted via Microsoft.Data.Sqlite or EF Core.

If you still feel strongly that we should change it, can you request that we do so in issue #19651?

[bricelam]

lol, looks like SQLite 3.31.0 (released yesterday) adds support for UUIDs. Maybe a breaking change is in order...

[jol64]

I´d suggest not to change the default but have out of the box value converters (#15078 (comment)) that can be used to configure it.
Just my 2cts...

[Leon99]

@bricelam you certainly shouldn't do the fix for the sake of my case. Only for the sake of following IETF and ITU-T standards, .NET's System.Guid.ToString() and all those people who rely on industry standards.
Does it have to be a breaking change - couldn't you just add an option to the provider?