Skip to content

Simplify the structure and codegen of UpdateStatement #367

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

Merged
merged 1 commit into from
Jul 4, 2016

Conversation

sgrif
Copy link
Member

@sgrif sgrif commented Jul 2, 2016

The main goal of this refactoring was to remove the implementations of
SaveChangesDsl from codegen and provide them generically instead.
Since I want the code outside of codegen to be as approachable as
possible, I made a few other changes to reduce the where clause of these
implementations. I still would like to reduce the where clause further,
but this is at a decent place. The following tree of changes were in
support of this goal:

  • Identifiable needs to have the table. This is so we can generically do
    table.find(id) for T: Identifiable.
  • Structs can now be passed directly to update and delete
    • This caused us to change UpdateTarget to be a struct, and
      introduce an AsUpdateTarget trait so that we can abstract over
      whether the fields on that struct are a reference or not

All of this will drastically simplify the non-procedural-macro form of
#[derive(AsChangeset)], and hopefully lead to more refactoring in the
future.

It should be noted that this changes the query on SQLite from

UPDATE users SET ... WHERE id = 1;
SELECT * FROM users WHERE id = 1 LIMIT 1;

to

UPDATE users SET ... WHERE id = 1;
SELECT * FROM users WHERE id = 1;

I can get the LIMIT 1 back in there if needed, but it makes the where clause more complex, and I don't think it matters to keep that here. I couldn't simplify it in the way I wanted to by pulling first into a separate trait because of rust-lang/rust#34260

The main goal of this refactoring was to remove the implementations of
`SaveChangesDsl` from codegen and provide them generically instead.
Since I want the code outside of codegen to be as approachable as
possible, I made a few other changes to reduce the where clause of these
implementations. I still would like to reduce the where clause further,
but this is at a decent place. The following tree of changes were in
support of this goal:

- Identifiable needs to have the table. This is so we can generically do
  `table.find(id)` for `T: Identifiable`.
- Structs can now be passed directly to `update` and `delete`
  - This caused us to change `UpdateTarget` to be a struct, and
    introduce an `AsUpdateTarget` trait so that we can abstract over
    whether the fields on that struct are a reference or not

All of this will drastically simplify the non-procedural-macro form of
`#[derive(AsChangeset)]`, and hopefully lead to more refactoring in the
future.
@sgrif
Copy link
Member Author

sgrif commented Jul 2, 2016

r? @diesel-rs/contributors

@fables-tales
Copy link
Contributor

LGTM

@sgrif sgrif merged commit a737308 into master Jul 4, 2016
@sgrif sgrif deleted the sg-refactoring branch July 4, 2016 09:44
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