-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
refactor: migrate activity repo to kysely #15203
Conversation
Co-authored-by: Jason Rasmussen <[email protected]>
const { count } = await this.db | ||
.selectFrom('activity') | ||
.select((eb) => eb.fn.countAll().as('count')) | ||
.leftJoin('users', 'users.id', 'activity.userId') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user relation should be an inner join.
.where('activity.albumId', '=', albumId) | ||
.where('activity.isLiked', '=', false) | ||
.where('users.deletedAt', 'is', null) | ||
.where('assets.deletedAt', 'is', null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do this check, then wouldn't the left join effectively work like an inner join because it requires an asset with a null deletedAt
to exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be right. You are saying you could rewrite this as an inner join with the condition of joining on id and where deleted at is null?
Co-authored-by: Jason Rasmussen <[email protected]>
Co-authored-by: Jason Rasmussen <[email protected]>
Co-authored-by: Jason Rasmussen <[email protected]>
No description provided.