Make #[diesel(embed)] fields be somewhat-checked by #[check_for_backend]
#4484
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ckend]` Currently when a struct has `#[derive(Selectable)]`, `#[check_for_backend(...)]`, and `embed`s another `Selectable` struct that doesn't have `#[check_for_backend]` but whose `FromSqlRow` and `Selectable` impls don't match with regards to `CompatibleType`, the `#[check_for_backend]` does not point us to which of the embed fields is the culprit, because they are ignored. It seems that this can be avoided by generating the checks even for `embed` fields. The check for this was disabled in d6d9260 when the experimented-with approach was "always generating the check on Selectable" with the justification that it was consequently checked by the underlying field's own Selectable checks, before it was decided to instead have the `#[check_for_backend(...)]` attribute, so it seems that this justification was indeed legitimate at the time it was introduced, and doesn't hold anymore now.
Ten0
force-pushed
the
check_embed_fields_with_check_for_backend
branch
from
e9e8890 to
a824e0d
Compare
weiznich
approved these changes
Feb 14, 2025
weiznich
added
the
maybe backport
weiznich
added a commit
to weiznich/diesel
that referenced
this pull request
Feb 14, 2025
This commit prepares a 2.2.8 release that includes the following changes: * diesel-rs#4472 * diesel-rs#4484 * diesel-rs#4486 * diesel-rs#4480
weiznich
added a commit
to weiznich/diesel
that referenced
this pull request
Feb 14, 2025
This commit prepares a 2.2.8 release that includes the following changes: * diesel-rs#4472 * diesel-rs#4484 * diesel-rs#4486 * diesel-rs#4480
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently when a struct has
#[derive(Selectable)],#[check_for_backend(...)], andembeds anotherSelectablestruct that doesn't have#[check_for_backend]but whoseFromSqlRowandSelectableimpls don't match with regards toCompatibleType, the#[check_for_backend]does not point us to which of the embed fields is the culprit, because they are ignored.It seems that this can be avoided by generating the checks even for
embedfields.The compatibility check was disabled on
#[embed]fields in d6d9260 when the experimented-with approach was "always generating the check on Selectable" with the justification that it was consequently checked by the underlying field's own Selectable checks, before it was decided to instead have the#[check_for_backend(...)]attribute, so it seems that this justification was indeed legitimate at the time it was introduced, and doesn't hold anymore now.