Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GUID instead of UUID #378

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ahjulstad
Copy link

This solves #366 for me, but more testing (and unittests) is necessary.

As far as I can understand, the SQL GUID type, isn't really a UInt128, but rather a composite struct. (link

struct GUID
{
    uint32_t Data1;
    uint16_t Data2;
    uint16_t Data3;
    uint8_t  Data4[8];
}

This PR adds a new GUID struct in the ODBC package, and uses that instead of Base.UUID.

As a lot of application code might expect to receive Base.UUID from results, some more thought should probably go into this.

Things to consider

  • Comparison (GUID and UUID could be argued to have different sorting order)
  • Equality (easy, but not yet done)
  • automatic conversion to Base.UUID, if that is something one should do?

Also, I have only tested this against a MSSQL database (my test rig in dev container here: https://github.com/ahjulstad/debug-julia-odbc ), so I have no idea if this breaks other databases.

@ahjulstad
Copy link
Author

ahjulstad commented Oct 23, 2023

This task has grown larger than what I was originally expecting, and help is needed on some items:

  • tests involving getproperty(row, prop).
  • the TIME SQL type.
  • .. and more

What I have now is a devcontainer setup with MariaDB and MSSQL, I have implemented a new API.GUID type, added conversions to and from Base.UUID. Many things work well, but not everything. I have to leave it like this for now, and hope to pick it up again later (unless someone else does it before me)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant