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

Tentatively moved uuid type and implemented blob-based variant for SQLite #4521

Closed

Conversation

LucaCappelletti94
Copy link

As per title, I am trying to implement support for UUID with SQLite backend, as I described here: #4520

Contrarily to what I described, I have started out with the binary version as I found other code available in diesel that was similar enough and I wanted to start with this first contribution in the smallest and simplest possible way.

@weiznich
Copy link
Member

weiznich commented Mar 6, 2025

Thanks for submitting this PR. There is a lot of related discussion in #364 why this is not desirable. I cannot see how any of this is addressed by PR or #4520. It's not that we don't know how to implement this mapping, rather that we feel it's not desirable to have it at all. The variant to have two different feature flags is also not helpful as:

  • Features need to be additive, that's not the case here. What happens if both features are enable?
  • It adds more features to diesel, which makes it even harder to test things. The current feature count is already barley manageable.

@LucaCappelletti94
Copy link
Author

Ok, I will try to make a crate adding such SQLite types that do not come with diesel with the necessary wrappers on it.

@LucaCappelletti94
Copy link
Author

Trying to do what I proposed in the previous comment is not viable as, for orphan rule, it is solely possible to implement the necessary traits in the diesel crate. I thought this was the case, but just in case I was wrong I tried nevertheless, of course getting:

upstream crates may add a new impl of trait `diesel::Expression` for type `&uuid::Uuid` in future versions

This makes it necessary to implement the proposed traits FromSql<UUID, SQLite> and ToSql<UUID, SQLite> here, or it would make it impossible to simply use uuid::Uuid as a datatype in the structs associated to the tables.

@weiznich
Copy link
Member

weiznich commented Mar 6, 2025

As demonstrated by for example jiff-diesel it's totally possible to provide support for such types outside of diesel.

As a more general note: We need to draw the line somewhere which type mappings are supported in diesel and which are not supported as we cannot support every possible third party type.

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.

2 participants