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

Use same table join code for both read and list functions #3663

Merged
merged 32 commits into from
Jul 28, 2023

Conversation

dullbananas
Copy link
Collaborator

This will fix #2763 and might reduce compile time (#3610).

It works by doing the table joins in a closure so that the return type can be inferred. The closure is inside of a function that returns 2 closures for read and list, which both capture and call the closure that has the join logic.

@dessalines
Copy link
Member

Nice, this will be great to have.

PostId,
Option<PersonId>,
Option<bool>,
)| async move {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rustfmt issue: rust-lang/style-team#138

@@ -0,0 +1,3 @@
use crate::schema::person;

diesel::alias!(person as person1: Person1, person as person2: Person2);
Copy link
Member

Choose a reason for hiding this comment

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

No need for a separate file, you should be able to put the cfg attribute directly on the macro. Or otherwise put a mod {} declaration in lib.rs

@dullbananas dullbananas marked this pull request as ready for review July 22, 2023 04:10
@dullbananas
Copy link
Collaborator Author

This is now ready to merge if I didn't mess up anything

@dullbananas
Copy link
Collaborator Author

The build time of lemmy_db_views when re-running the first cargo command in fix-clippy changed from 208.4s to 65.6s

@Nutomic
Copy link
Member

Nutomic commented Jul 26, 2023

Fails to compile:

    Checking lemmy_db_views v0.18.1 (/woodpecker/src/github.com/LemmyNet/lemmy/crates/db_views)
error[E0424]: expected value, found module `self`
   --> crates/db_views/src/comment_view.rs:242:10
    |
57  | fn queries<'a>() -> Queries<
    |    ------- this function can't have a `self` parameter
...
242 |       if self.post_id.is_some() {
    |          ^^^^ `self` value is a keyword only available in methods with a `self` parameter

}

// `()` is used to prevent type inference error
impl Queries<(), ()> {
Copy link
Member

Choose a reason for hiding this comment

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

I really do wish there was a better way to handle this, but oh well. Seems like such a common case, but because our type is so complicated, its near impossible to reverse.

We'll still merge, but if you get some time, ask @weiznich on matrix or here if there really is no better alternative for what we need.

@dullbananas
Copy link
Collaborator Author

It now compiles

@Nutomic Nutomic merged commit 9a5a13c into LemmyNet:main Jul 28, 2023
1 check passed
@Nutomic
Copy link
Member

Nutomic commented Jul 28, 2023

Thanks!

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.

Move query join logic into common functions
3 participants