ToGuid conversion incorrect

[mdsitton]

So i discovered that the default conversion the C# Guid type is not correct when trying to round-trip the type through a string:

using Medo;
Uuid7 id = Uuid7.NewUuid7();

Console.WriteLine($"uuid7:\t{id.ToString()}");
Console.WriteLine($"MsSql:\t{id.ToGuidMsSql().ToString()}");
Console.WriteLine($"ToGuid:\t{id.ToGuid().ToString()}");

output:

PS D:\development\uuidv7test> dotnet run
uuid7:  01899148-b29e-703a-848b-af5e76f5e9c2
MsSql:  01899148-b29e-703a-848b-af5e76f5e9c2
ToGuid: 48918901-9eb2-3a70-848b-af5e76f5e9c2

The internal Guid type is stored in a little endian format, however there is a new constructor to convert big endian bytes to little endian when this gets released in a .net version:
https://github.com/dotnet/runtime/blob/d389ab955dbb65547643fa1aed52669b02c04294/src/libraries/System.Private.CoreLib/src/System/Guid.cs#L69
dotnet/runtime#87993

For now a manual conversion is required though.

The current code manually converts to little endian for the ToGuidMsSql function but this should be done all the time IMO.

Additionally if you try to round trip through bytes to guid and back it works from the uuid side but it's still not correct. Since it's just interpreting the big endian bytes as little endian and then reading those back exactly as they were input. But if you round-trip through ToGuidMsSql it's also wrong because they don't get flipped on the way back into uuid7

Uuid7 idRt = new Uuid7(id.ToGuid());
Uuid7 idRtMs = new Uuid7(id.ToGuidMsSql());

Console.WriteLine($"rt:\t{idRt.ToString()}");
Console.WriteLine($"rt2:\t{idRtMs.ToString()}");

rt:       01899148-b29e-703a-848b-af5e76f5e9c2
rtMsSql:  48918901-9eb2-3a70-848b-af5e76f5e9c2

So this is what the proper conversion should look like:

// Read and rewrite values to flip to what Guid expects
var flip = (Span<byte> bytes) =>
{
    var aPos = bytes.Slice(0, 4);
    var bPos = bytes.Slice(4, 2);
    var cPos = bytes.Slice(6, 2);

    int a = BinaryPrimitives.ReadInt32BigEndian(aPos);
    short b = BinaryPrimitives.ReadInt16BigEndian(bPos);
    short c = BinaryPrimitives.ReadInt16BigEndian(cPos);

    BinaryPrimitives.WriteInt32LittleEndian(aPos, a);
    BinaryPrimitives.WriteInt16LittleEndian(bPos, b);
    BinaryPrimitives.WriteInt16LittleEndian(cPos, c);
};

// Correct manual conversion:
Span<byte> bytes = stackalloc byte[16];
id.TryWriteBytes(bytes);

// flip to little endian for Guid
flip(bytes);
var fixedGuid = new Guid(bytes);

Span<byte> guidBytes = stackalloc byte[16];
fixedGuid.TryWriteBytes(guidBytes);

// flip to big endian for Uuid7
flip(guidBytes);

var uuid7Rt = new Uuid7(guidBytes);
Console.WriteLine($"guid--rt:\t{fixedGuid.ToString()}");
Console.WriteLine($"uuid7-rt:\t{uuid7Rt.ToString()}");

guid--rt:       01899148-b29e-703a-848b-af5e76f5e9c2
uuid7-rt:       01899148-b29e-703a-848b-af5e76f5e9c2

[mdsitton]

If wanted i can make a pull request for this fix. But i think this is a large enough issue that it should be made widely known!

[mdsitton]

Additionally for anyone with this requirement here is an extension method for now to help with the conversion.

using System.Buffers.Binary;
using Medo;

public static class GuidConvertionExtension
{
    public static Guid ToGuidLE(this Uuid7 uuid7)
    {
        Span<byte> bytes = stackalloc byte[16];
        uuid7.TryWriteBytes(bytes);
        // Only required when on LittleEndian system
        if (BitConverter.IsLittleEndian)
        {
            var aPos = bytes.Slice(0, 4);
            var bPos = bytes.Slice(4, 2);
            var cPos = bytes.Slice(6, 2);

            int a = BinaryPrimitives.ReadInt32BigEndian(aPos);
            short b = BinaryPrimitives.ReadInt16BigEndian(bPos);
            short c = BinaryPrimitives.ReadInt16BigEndian(cPos);

            BinaryPrimitives.WriteInt32LittleEndian(aPos, a);
            BinaryPrimitives.WriteInt16LittleEndian(bPos, b);
            BinaryPrimitives.WriteInt16LittleEndian(cPos, c);
        }
        return new Guid(bytes);
    }
}

[medo64]

Hi,

While converting to and from Guid, I intentionaly prioritize both being indentical in their binary form. Unfortunately, as you have seen above, this leads to a difference in their textual format. As most databases actually deal with binary data and not their text representation, I believe this is the correct default.

I will look into adding additional overrides with explicit endiann overloads but default behavior will stay binary comptibility for now.

The only database that actually uses LE encoding when storing UUID internally is MS SQL and thus the overloads for the same. However, all other databases correctly treat UUID as a binary blob and thus doing endian-sensitive operation by default would mess them up.

I view this as a damn-if-you-do-damn-if-you-don't scenario and I decided to be damned in binary format.

[mdsitton]

I wouldn't say this is valid i ran into issues using Npgsql where it converts expecting the binary form of the Guid to be in little endian

Here is the code in question:
https://github.com/npgsql/npgsql/blob/main/src/Npgsql/Internal/TypeHandlers/UuidHandler.cs

[mdsitton]

Additionally I would argue that the string representation is something that is more standardized and expected to be consistent across implementations

[medo64]

The primary use of this library is to support UUID v7. Fields in UUID v7 are explicitly specified to be in big-endian.

While I am okay with adding some overloads to help with Guid compatibility (thus adding NewGuidMsSql and ToGuidMsSql methods), I will not make this class compatible with Guid when doing so would mean going against the RFC.

[mdsitton]

this library uses the string representation to move data to sqlite:
https://github.com/praeclarum/sqlite-net/blob/e8a24a8b2ecb4fd700c5fe46062239a9b08472fd/src/SQLite.cs#L3315

The official microsoft sqlite binding does as well:
https://github.com/dotnet/efcore/blob/d0e0824f3e7997a80a2f6f3bc13234a0077ffb30/src/EFCore.Relational/Storage/GuidTypeMapping.cs#L56C35-L56C35

Also looks like the mysql library is also doing byte swapping:
https://github.com/mysql/mysql-connector-net/blob/1341887fc28fc3181e9bf65a8dc6534e82b84284/EFCore6/src/Storage/Internal/MySQLGuidTypeMapping.cs#L130

So i certainly would NOT generalize that all databases are just using the exact .net binary representation here...

Just glancing over a few of the popular libraries here i don't see any consistency in how it's used binary vs text vs little endian or big endian formats.

[mdsitton]

The primary use of this library is to support UUID v7. Fields in UUID v7 are explicitly specified to be in big-endian.

While I am okay with adding some overloads to help with Guid compatibility (thus adding NewGuidMsSql and ToGuidMsSql methods), I will not make this class compatible with Guid when doing so would mean going against the RFC.

Yeah i agree that this library should probably keep the big endian format

[mdsitton]

I personally think that the default conversions to/from and comparisons against guid are a mistake and any conversions should be explicit where the user chooses the conversion form they need. To prevent any potential compatibility issues.

As-is it's very easy to hit weird behavior and hit this depending on the database you end up using. And if you end up changing things like using different databases under production/development (such as using sqlite to test locally) then you might hit these conversion issues.

[medo64]

Please note that additional method has been added for conversion to Guid with endianness:

var uuid = Uuid7.NewUuid7();
var guid = uuid.ToGuid(matchGuidEndianness: true);

Also, one can convert from such Guid:

var guid = Guid.NewGuid();
var uuid = Uuid7.FromGuid(guid, matchGuidEndianness: true);