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

derive FromRow: sqlx(default) for all fields #2801

Merged
merged 6 commits into from
Oct 17, 2023

Conversation

grgi
Copy link
Contributor

@grgi grgi commented Oct 6, 2023

This commit adds sqlx(default) as a struct attribute when deriving FromRow - imitating the similar behaviour of serde(default). E.g. the following two definitions are equivalent:

#[derive(sqlx::FromRow)]
#[sqlx(default)]
struct Options {
    option_a: Option<String>,
    option_b: Option<i32>,
}

derives the same FromRow implementation as

#[derive(sqlx::FromRow)]
struct Options {
    #[sqlx(default)]
    option_a: Option<String>,
    #[sqlx(default)]
    option_b: Option<i32>,
}

@abonander
Copy link
Collaborator

Strictly speaking, if you want it to be equivalent to #[serde(default)] then it should actually require the annotated struct itself to implement Default: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7fb4f62b44e7e63acc2e1cfe1599a59a

It actually constructs the struct with Default::default() and takes fields from it if they don't appear in the input.

The two approaches are equivalent if #[derive(Default)] is also used on the struct itself, since that fills all the fields with Default::default() anyway, but a manual Default impl can change that behavior.

Because of that it's probably the more flexible and less surprising option, although it does preclude implementing Drop for the struct since you can't move non-Copy fields from a struct that implements Drop.

@grgi grgi force-pushed the fromrow-default-struct branch from 1764275 to bbe1eaa Compare October 8, 2023 11:57
@grgi
Copy link
Contributor Author

grgi commented Oct 9, 2023

Thank you for the suggestion! I have changed it accordingly, so now it requires Default and moves the values from the default implementation if missing from the query.

Copy link
Collaborator

@abonander abonander left a comment

Choose a reason for hiding this comment

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

Just some copy-editing for the docs.

sqlx-core/src/from_row.rs Outdated Show resolved Hide resolved
sqlx-core/src/from_row.rs Show resolved Hide resolved
@abonander abonander merged commit 54c5d6b into launchbadge:main Oct 17, 2023
64 checks passed
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