Conversation
Walkthrough두 저장소 파일의 부서별 순위 계산 로직을 재구성하여, 부서당 상위 30명 기준의 중첩 순위 계산(ROW_NUMBER)과 재계산된 부서 합계(COALESCE 적용)를 도입했습니다. Changes
개요두 저장소 파일의 부서별 순위 계산 로직을 재구성했습니다. DepartmentRankingRepository에 recalculated_rankings CTE를 추가하여 부서별 상위 30명 기준의 순위를 사전 계산하고, StudySessionRepository의 두 쿼리를 ROW_NUMBER 기반의 중첩된 분할 순위 지정 방식으로 재작성했습니다. 변경사항
코드 리뷰 예상 소요 시간🎯 4 (Complex) | ⏱️ ~45분
Possibly related PRs
제안된 검토자
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java (1)
100-119: totalMillis 계산에서/1000누락 (단위 불일치 버그)Line 100-109의
totalMillis계산에서/1000이 누락되어 있습니다. 반면 Line 110-119의ranking계산에서는/1000이 포함되어 있습니다.결과적으로:
totalMillis는 마이크로초 단위로 반환됨ranking은 밀리초 기준으로 산정됨이로 인해 데이터 일관성 문제와 예상치 못한 동작이 발생할 수 있습니다.
CAST(COALESCE(SUM( TIMESTAMPDIFF(MICROSECOND, GREATEST(s.start_time, :periodStart), CASE WHEN s.end_time IS NULL THEN :periodEnd WHEN s.end_time > :periodEnd THEN :periodEnd ELSE s.end_time END - ) + ) / 1000 ), 0) AS SIGNED) as totalMillis,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/gpt/geumpumtabackend/rank/repository/DepartmentRankingRepository.java(2 hunks)src/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java(1 hunks)
🔇 Additional comments (3)
src/main/java/com/gpt/geumpumtabackend/rank/repository/DepartmentRankingRepository.java (1)
46-78: 상위 30명 필터링 로직 구현이 적절합니다학과별 상위 30명만 집계에 반영하는 로직이 올바르게 구현되었습니다:
- 내부 쿼리에서 유저별 totalMillis 계산 및
ROW_NUMBER()로 학과 내 순위 산출WHERE deptRank <= 30으로 상위 30명 필터링- 외부 쿼리에서
SUM(totalMillis)로 학과별 합산src/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java (2)
143-185: 현재 진행 중인 학과 랭킹 로직이 올바르게 구현되었습니다학과별 상위 30명 필터링 로직이 정확히 구현되었습니다:
- 내부 서브쿼리에서
ROW_NUMBER() OVER (PARTITION BY u.department ...)로 학과별 순위 산출WHERE deptRank <= 30으로 필터링- 진행 중인 세션(
s.end_time IS NULL)에 대해:now파라미터를 사용하여 현재 시간까지 계산
191-225: 종료된 기간의 학과 랭킹 로직이 적절합니다
calculateCurrentDepartmentRanking과 일관된 구조로 상위 30명 필터링이 구현되었습니다. Line 218에서s.end_time >= :periodStart조건으로 완료된 세션만 집계하며, 종료된 기간에 적합한 처리입니다.
src/main/java/com/gpt/geumpumtabackend/rank/repository/DepartmentRankingRepository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/com/gpt/geumpumtabackend/rank/repository/DepartmentRankingRepository.java (2)
61-69: 동점 사용자 처리를 위해RANK()사용 고려
ROW_NUMBER()는 동점인 경우에도 임의로 순번을 부여합니다. 예를 들어 30위와 31위 사용자의totalMillis가 동일할 경우, 한 명은 포함되고 한 명은 제외될 수 있습니다.동점 사용자를 모두 포함시키려면
RANK()를 사용하는 것이 더 공정할 수 있습니다. 다만 이 경우 학과당 30명을 초과할 수 있으므로 의도에 따라 결정해 주세요.
51-76: 중복 계산 리팩토링 고려
totalMillis계산이 SELECT 절(lines 55-60)과 ORDER BY 절(lines 63-68)에서 중복됩니다. 가독성과 유지보수성을 위해 내부 서브쿼리를 한 단계 더 추가하여 계산을 한 번만 수행하도록 리팩토링할 수 있습니다.recalculated_rankings AS ( SELECT department, CAST(SUM(totalMillis) AS SIGNED) as totalMillis, RANK() OVER (ORDER BY SUM(totalMillis) DESC) as ranking FROM ( SELECT - u.department, - u.id as userId, - COALESCE(SUM( - TIMESTAMPDIFF(MICROSECOND, - GREATEST(s.start_time, DATE(:period)), - LEAST(s.end_time, DATE_ADD(DATE(:period), INTERVAL 1 DAY)) - ) / 1000 - ), 0) as totalMillis, - ROW_NUMBER() OVER ( - PARTITION BY u.department - ORDER BY COALESCE(SUM( - TIMESTAMPDIFF(MICROSECOND, - GREATEST(s.start_time, DATE(:period)), - LEAST(s.end_time, DATE_ADD(DATE(:period), INTERVAL 1 DAY)) - ) / 1000 - ), 0) DESC - ) as deptRank - FROM user u - LEFT JOIN study_session s ON u.id = s.user_id - AND s.start_time < DATE_ADD(DATE(:period), INTERVAL 1 DAY) - AND s.end_time >= DATE(:period) - WHERE u.role = 'USER' AND u.department IS NOT NULL - GROUP BY u.department, u.id + department, + userId, + totalMillis, + ROW_NUMBER() OVER ( + PARTITION BY department + ORDER BY totalMillis DESC + ) as deptRank + FROM ( + SELECT + u.department, + u.id as userId, + COALESCE(SUM( + TIMESTAMPDIFF(MICROSECOND, + GREATEST(s.start_time, DATE(:period)), + LEAST(s.end_time, DATE_ADD(DATE(:period), INTERVAL 1 DAY)) + ) / 1000 + ), 0) as totalMillis + FROM user u + LEFT JOIN study_session s ON u.id = s.user_id + AND s.start_time < DATE_ADD(DATE(:period), INTERVAL 1 DAY) + AND s.end_time >= DATE(:period) + WHERE u.role = 'USER' AND u.department IS NOT NULL + GROUP BY u.department, u.id + ) user_totals ) ranked_users WHERE deptRank <= 30 GROUP BY department )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/gpt/geumpumtabackend/rank/repository/DepartmentRankingRepository.java(2 hunks)
🔇 Additional comments (1)
src/main/java/com/gpt/geumpumtabackend/rank/repository/DepartmentRankingRepository.java (1)
80-88: 이전 리뷰 이슈 해결 확인 및 fallback 로직 검토 필요Line 82에서
RANK() OVER (ORDER BY COALESCE(...) DESC)를 사용하여 이전 리뷰에서 지적된 랭킹 일관성 문제가 해결되었습니다.다만,
COALESCE(rr.totalMillis, dr.total_millis, 0)fallback 로직에서dr.total_millis는 기존 로직(전체 사용자 기준)으로 계산된 값입니다. 해당 기간에 활동한 사용자가 없는 학과의 경우, 새로운 상위 30명 기준이 아닌 기존 방식의 값이 사용될 수 있습니다.이것이 의도된 동작인지 확인해 주세요. 만약 모든 학과에 동일한 계산 방식을 적용하려면
drfallback을 제거하고COALESCE(rr.totalMillis, 0)만 사용하는 것을 고려해 주세요.
2 participants
🚀 1. 개요
학과별 랭킹 산정 시, 각 학과의 상위 30명만 집계에 반영하도록 로직을 변경했습니다.
학과별 인원 수 편차로 인해 소수 인원 학과가 상대적으로 불리해지는 문제를 완화하고,
보다 공정한 학과 간 비교가 가능하도록 하는 것이 목적입니다.
📝 2. 주요 변경 사항
2-1. 공통 개념: 학과별 상위 30인 집계 로직 도입
기존에는 학과에 속한 모든 유저의 totalMillis 합계를 기반으로 학과 랭킹을 계산했습니다.
이번 변경에서는 아래와 같은 흐름으로 랭킹이 계산됩니다.
user+study_session기준으로 유저별 totalMillis를 계산ROW_NUMBER() OVER (PARTITION BY u.department ORDER BY totalMillis DESC)를 통해학과별 유저 랭크(deptRank) 산출
WHERE deptRank <= 30조건으로 각 학과별 상위 30명만 필터링SUM(totalMillis)를 다시 계산하여 학과별 totalMillis 재산정RANK() OVER (ORDER BY SUM(totalMillis) DESC)로 학과 랭킹 부여Summary by CodeRabbit
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.