-
Notifications
You must be signed in to change notification settings - Fork 8
Further performance update for my-reviews #144
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
Conversation
Further performance update for my-reviews
| `, | ||
| ); | ||
| cteFragments.push( | ||
| Prisma.sql` |
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.
[performance]
The use of COUNT(*)::bigint and SUM(...)::bigint is consistent with the previous implementation, but ensure that the casting to bigint is necessary for the application logic. If the counts are expected to be within the range of integer, this casting might be unnecessary and could be optimized.
| ); | ||
| cteFragments.push( | ||
| Prisma.sql` | ||
| incomplete_reviews AS ( |
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.
[correctness]
The DISTINCT ON clause is used here to select unique resourceId entries. Ensure that this approach aligns with the intended logic, as DISTINCT ON can sometimes lead to unexpected results if the ordering is not carefully considered.
|
|
||
| const reviewTotalsJoin = adminUser | ||
| ? Prisma.sql` | ||
| LEFT JOIN LATERAL ( |
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.
[correctness]
The conditional logic for adminUser results in different SQL joins. Verify that the logic correctly handles all cases, especially when transitioning between admin and non-admin users, to ensure that the correct data is retrieved.
|
|
||
| const incompleteReviewsJoin = adminUser | ||
| ? Prisma.sql` | ||
| LEFT JOIN LATERAL ( |
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.
[maintainability]
The logic for determining hasIncompleteReviews and incompletePhaseName is duplicated for admin and non-admin users. Consider refactoring to reduce redundancy and improve maintainability.
No description provided.