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

hex2uuid formats UUIDs #158

Merged
merged 3 commits into from
Sep 18, 2020
Merged

hex2uuid formats UUIDs #158

merged 3 commits into from
Sep 18, 2020

Conversation

kaapstorm
Copy link
Contributor

This allows UUIDs to be recognized by databases with a UUID data type, like PostgreSQL and SQL Server.

cc @czue

This allows UUIDs to be recognized by databases with a UUID data type,
like PostgreSQL and SQL Server.
Copy link
Contributor

@snopoke snopoke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will still save it as a string in the DB. Is that OK or are you wanting to change the DB column type as well?

@kaapstorm
Copy link
Contributor Author

Yes, setting the value to a string is fine. Postgres and SQL Server accept string UUIDs for their UUID column types.

It seemed to me that for DET to set/change the column type, I'd need to create a SQLAlchemy UUID data type. For the use case I'm looking at, the column type has been set manually, so I'd like to use --strict-types so that DET does not try to change the column type back to TEXT.

Do you think the name "hex2uuid" is confusing, and people will expect it to set/change the column type to UUID? Or is it OK for people to set the column type manually and use the --strict-types argument? How do we generally feel about using --strict-types?

@snopoke
Copy link
Contributor

snopoke commented Sep 17, 2020

Do you think the name "hex2uuid" is confusing, and people will expect it to set/change the column type to UUID?

I do think the name doesn't match the other XtoY functions. I think just renaming to format-uuid would be fine.

@kaapstorm kaapstorm merged commit 3b2294c into master Sep 18, 2020
@kaapstorm kaapstorm deleted the nh/uuid branch September 18, 2020 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants