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

Remove left joins and use only one call to select method in post_view.rs #3865

Merged
merged 59 commits into from
Sep 4, 2023

Conversation

dullbananas
Copy link
Collaborator

This makes the code cleaner and might also improve build time because now there's only one call to select.

I will probably make this change to other views in separate pull requests in the future.

@dullbananas dullbananas marked this pull request as ready for review August 12, 2023 01:43
@dullbananas dullbananas marked this pull request as draft August 12, 2023 20:32
@dullbananas
Copy link
Collaborator Author

dullbananas commented Aug 12, 2023

I will try to get rid of the other left joins

Left join is only useful if there's a possibility of a row on the left side being joined with more than one row on the right

@phiresky
Copy link
Collaborator

This is great imo. I was just looking at those queries for #3872 and all those left joins are pretty hard to understand and reason about. This is much easier to read. It will need a bit of testing though to see if the query plans are still the same or better. I think they should be and could even be better because with left joins sometimes postgresql estimates multiple rows being retrieved from the left join.

@phiresky
Copy link
Collaborator

it would be great if you could also make it so all those person_id_join things aren't actually called when person_id is null (logged out) instead of calling it with a fake -1 value

@dullbananas dullbananas marked this pull request as ready for review August 27, 2023 01:23
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

@Nutomic and @phiresky should probably take a look before merging also.

@phiresky
Copy link
Collaborator

looks good to me 👍

@dessalines
Copy link
Member

nutomic has some comments above too tho.

@Nutomic Nutomic merged commit 5b5ac0f into LemmyNet:main Sep 4, 2023
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.

5 participants