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

UUID decode to string incorrectly. #322

Open
james-lawrence opened this issue Nov 25, 2024 · 8 comments
Open

UUID decode to string incorrectly. #322

james-lawrence opened this issue Nov 25, 2024 · 8 comments

Comments

@james-lawrence
Copy link

james-lawrence commented Nov 25, 2024

when passing a sql.NullString to the driver for a uuid field we get a string of the literal uuid bytes instead of the uuid string format.

got:  �dW�Dw��4���
expected: 01936457-ab44-7710-bc9d-1f3414ac9deb

strictly speaking I tried to use the driver provided uuid type however the tooling I'm using can't compile use the datatype due to cgo usage. it'd be nice if the datatypes were in their own pure go package and operated on raw binary.

pgtype by jackc is a wonderful example of a phenomenal database.Value library.

@apocelipes
Copy link
Contributor

As I know, UUIDs are stored as binary data in the duckdb. So they are not strings. To get a string value you should encode the UUID.

You can use github.com/google/uuid.NullUUID instead of NullString. github.com/google/uuid.UUID also has a String() method that can correctly encode uuid values. For example:

var u uuid.NullUUID
err := db.QueryRow("SELECT uuid_field FROM foo WHERE id=?", id).Scan(&u)
if err != nil {
        panic(err)
}
if u.Valid {
        str = u.UUID.String()
}

As an alternative, you can also use the standard library's sql.Null[UUID] (need v1.23+) after the '(*UUID).Scan' was fixed. The usage is similar to github.com/google/uuid.NullUUID but no need to import a 3rd-party module.

@james-lawrence
Copy link
Author

james-lawrence commented Dec 9, 2024

Yes they're stored as binary. Bigger problem for me is can't use the library provided types due to cgo :/ guess i should have focused on that aspect.

Would you be interested in a PR that breaks the types into a separate module with no cgo usage?

@taniabogatsch
Copy link
Collaborator

cc @marcboeker - I like the idea, but it will involve some work. Also, we need to discuss who would maintain such a package.

@james-lawrence
Copy link
Author

It can be within this package just a subpackage i believe. I'd gladly help with the initial conversion, and some light maintenance.

@james-lawrence
Copy link
Author

@taniabogatsch I'll take a look this weekend to start experimenting and come up with a proposal.

@james-lawrence
Copy link
Author

alright did a quick spike. moving the db types into their own directory and separating them from the cgo code. also added a small uuidx module to drop the google/uuid dependency since it was primarily for tests anyways.

Only bit I think that is missing is driver.Value impementations for the non-composite types. I'll explore further next week based on commentary / my own investigations.

@taniabogatsch
Copy link
Collaborator

Hi, @james-lawrence - thanks for starting to tackle this. I just had a quick look at your draft. Moving the types into a separate package would break backward compatibility, right? If that is the case, then we should only move to this solution when changing the major version, e.g., when moving to a v2 (#232).

@james-lawrence
Copy link
Author

I mean we could type alias to preserve compat

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

No branches or pull requests

3 participants