Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough동일 두 사용자 간 중복된 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
시
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql`:
- Around line 80-88: The migration currently deletes merged chat_room rows and
immediately drops temp_duplicate_room_map and temp_direct_room_pairs, removing
any recovery data; instead, before deleting rows (involving chat_room,
temp_duplicate_room_map with from_room_id and keep_room_id), copy the mapping
and the original membership snapshot into permanent backup tables (e.g.,
backup_duplicate_room_map and backup_direct_room_memberships) or insert into an
archival schema, then perform the DELETE of chat_room; only drop the temporary
tables after a defined verification period or provide a separate cleanup
migration, ensuring functions/objects referencing temp_duplicate_room_map and
temp_direct_room_pairs are updated to use the backup tables for
audits/rollbacks.
- Around line 43-48: The current ROW_NUMBER() ordering (ORDER BY
COALESCE(last_message_at, created_at) DESC, created_at DESC, room_id DESC) can
pick an empty room (no messages) as the winner, leaving
last_message_content/last_message_sent_at stale; update the merge logic to
prefer rooms that have messages by ordering first on message existence and
last_message_at (e.g., ORDER BY (last_message_at IS NOT NULL) DESC,
last_message_at DESC, created_at DESC, room_id DESC) so rooms with any messages
win, and add a follow-up step after the merge to recompute/populate
last_message_content and last_message_sent_at (the columns introduced in
V1__init.sql) for the kept room from actual messages.
- Around line 75-78: Before deleting loser memberships, merge per-user
membership state from loser -> keep: update the target rows in chat_room_member
by joining chat_room_member crm (source rows with chat_room_id = m.from_room_id)
to the corresponding target rows t (chat_room_id = m.to_room_id AND t.user_id =
crm.user_id) using temp_duplicate_room_map m, and set t.visible_message_from and
t.left_at to preserve the most conservative/non-lossy value (e.g., choose the
earliest/non-null timestamp via COALESCE/LEAST or equivalent), then only after
that updated merge run the DELETE crm ... JOIN temp_duplicate_room_map m
statement; reference chat_room_member, temp_duplicate_room_map,
visible_message_from, left_at and the existing DELETE crm statement to locate
where to insert the UPDATE merge step.
- Line 1: Remove the hardcoded schema selection by deleting the `USE konect;`
statement from the V69__merge_duplicate_direct_chat_rooms.sql migration; ensure
the migration does not reference the konect schema anywhere (no schema-qualified
table names) so it relies on the runtime datasource/schema
(spring.datasource.url / ${MYSQL_URL}) like other migrations and will work
across environments and permission setups.
- Around line 69-72: The UPDATE on table chat_message (joining
temp_duplicate_room_map) will trigger the column-level ON UPDATE
CURRENT_TIMESTAMP for updated_at and overwrite original timestamps; modify the
UPDATE that sets cm.chat_room_id = m.keep_room_id to also explicitly set
updated_at = updated_at so the existing updated_at values are preserved (locate
the UPDATE statement referencing chat_message, temp_duplicate_room_map,
cm.chat_room_id and add updated_at = updated_at to the SET clause).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b37ba681-9fd1-4c89-8572-a854b879ec8d
📒 Files selected for processing (1)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
- USE konect; 문장 삭제하여 runtime datasource 기반 동작하도록 수정 - 다양한 환경 및 권한 설정에서 정상 작동하도록 개선
- ROW_NUMBER() 정렬 로직 수정: (last_message_at IS NOT NULL) DESC를 첫 번째 정렬 기준으로 추가하여 메시지가 있는 방을 우선 선택 - 병합 후 keep_room의 last_message_content와 last_message_sent_at을 실제 메시지 기반으로 재계산하는 단계 추가 - 빈 방이 winner로 선택되어 last_message_* 컬럼이 stale 상태로 남는 문제 방지
- chat_message 테이블 UPDATE 시 ON UPDATE CURRENT_TIMESTAMP로 인해 updated_at이 자동 갱신되는 문제 방지 - updated_at = updated_at 명시적 설정으로 원본 타임스탬프 유지
- 삭제 대상 방 멤버십 삭제 전, visible_message_from과 left_at 값을 keep 방으로 병합하는 UPDATE 단계 추가 (step 4) - visible_message_from: 더 이른 값(더 많은 메시지 조회 가능) 선택 - left_at: 둘 중 하나라도 나간 경우 나간 것으로 처리 (더 이른 값 선택) - 기존 멤버십 데이터 손실 방지 및 step 번호 재정렬 (5-8)
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql`:
- Around line 77-89: The UPDATE ignores merging the read pointer `last_read_at`,
causing messages already read in the loser room to become unread; modify the
`UPDATE chat_room_member` (aliases t and crm, joining temp_duplicate_room_map m)
to also preserve the later non-null `last_read_at` value for the kept member
(e.g. set `t.last_read_at` to the greatest/non-null of `t.last_read_at` and
`crm.last_read_at` using COALESCE/GREATEST or equivalent null-safe logic) so the
merged room retains the correct latest read pointer alongside
`visible_message_from` and `left_at`.
- Around line 6-7: The migration drops temporary mapping tables
(temp_direct_room_pairs, temp_duplicate_room_map) up front which makes the
process non-retriable: if membership deletions for the "loser" room (the DELETE
from membership steps referenced around the membership removal block) fail, the
loser can be left with 0 members and will not be reselected because the `(SELECT
COUNT(*) ...) = 2` candidate filter only finds rooms with exactly 2 members; fix
by making the delete phase idempotent and the mapping durable across retry —
either (A) do not DROP temp_direct_room_pairs and temp_duplicate_room_map at the
start; instead create them only if not exists or preserve them until all room
deletions complete, or (B) broaden the candidate filter to include rooms with 0
members when consulting temp_duplicate_room_map and the duplicate selection
query, and ensure the membership/deletion steps for the loser room are wrapped
so they can be safely re-run (e.g., use DELETE ... WHERE EXISTS checks or
conditional room deletion guarded by the mapping), referencing
temp_direct_room_pairs, temp_duplicate_room_map, the `(SELECT COUNT(*) ...) = 2`
filter, and the membership deletion block to implement one of these retriable
strategies.
- Around line 35-65: The temp_duplicate_room_map logic can map multiple
from_room_id rows to the same keep_room_id which causes the UPDATE of
chat_room_member to perform multiple conflicting SETs for the same target row;
instead, aggregate loser memberships by (keep_room_id, user_id) first (e.g.
create a loser_agg CTE that SELECTs m.keep_room_id, crm.user_id,
MIN(crm.visible_message_from) AS min_visible_from and MIN(CASE WHEN crm.left_at
IS NOT NULL THEN crm.left_at END) AS min_left_at FROM temp_duplicate_room_map m
JOIN chat_room_member crm ON crm.chat_room_id = m.from_room_id GROUP BY
m.keep_room_id, crm.user_id) and then perform a single UPDATE joining
chat_room_member t to loser_agg la on t.chat_room_id = la.keep_room_id AND
t.user_id = la.user_id, setting t.visible_message_from =
LEAST(t.visible_message_from, la.min_visible_from) and t.left_at using the
conditional LEAST logic shown (preserve NULL semantics) so each keep membership
row is updated exactly once.
- Around line 107-120: The LEFT JOIN subquery that selects messages by WHERE
created_at = (SELECT MAX(created_at) ...) can return multiple rows when messages
share the same created_at, causing non-deterministic updates to
cr.last_message_content/last_message_sent_at; replace the subquery with a
deterministic row selection using ROW_NUMBER() over (PARTITION BY chat_room_id
ORDER BY created_at DESC, id DESC) on chat_message (alias e.g., cm_ranked) and
join only rows where rn = 1 so each chat_room_id yields exactly one latest
message (use chat_message.id as a tiebreaker if needed) and update cr using that
single-row result (refer to chat_message, cr, last_message_content,
last_message_sent_at, created_at, content, and the latest_msg alias).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c6666b4-c193-44b1-bb97-ab1ca7f63591
📒 Files selected for processing (1)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Show resolved
Hide resolved
- last_read_at: GREATEST로 병합하여 더 나중 읽음 시점 보존 - custom_room_name: COALESCE로 사용자 설정 방 이름 보존 - notification_mute_setting: target_id를 keep_room_id로 업데이트 (이미 keep 방에 설정 있으면 UNIQUE 제약으로 자동 삭제됨) - step 번호 재정렬 (4-9)
- 기존: MAX(created_at)만 사용하여 타임스탬프 동일 시 여러 행 반환 가능 - 개선: MAX(id)로 최신 메시지를 결정론적으로 선택 - 서브쿼리에서 chat_room_id별 MAX(id)를 먼저 계산 후 해당 id를 가진 메시지와 조인하여 단일 행 보장
- temp_duplicate_room_map을 CREATE TABLE IF NOT EXISTS로 생성하여 재시도 시 기존 매핑 테이블 재사용 - 후보 방 선정 시 매핑 테이블에 이미 있는 방도 포함 (재시도 시 멤버 0명이 된 방도 처리 가능) - INSERT ... ON DUPLICATE KEY UPDATE로 매핑 idempotent 처리 - temp_direct_room_pairs는 매 실행 새로 생성하여 최신 상태 반영
- 여러 from_room_id가 같은 keep_room_id로 매핑될 때 동일한 타겟 행에 대한 다중 업데이트 충돌 방지 - loser 멤버십을 (keep_room_id, user_id) 기준으로 집계하여 단일 UPDATE로 처리하도록 개선
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql`:
- Around line 49-97: The current INSERT into temp_duplicate_room_map (with ON
DUPLICATE KEY UPDATE) allows remapping on retries and can flip keep/from; change
it so mappings are only created when temp_duplicate_room_map is empty. Modify
the INSERT ... SELECT that uses temp_direct_room_pairs to append a WHERE NOT
EXISTS (SELECT 1 FROM temp_duplicate_room_map) to the SELECT (or wrap the whole
insert in a conditional check) and remove the ON DUPLICATE KEY UPDATE clause so
no existing mappings get overwritten; keep references to
temp_duplicate_room_map, temp_direct_room_pairs, and the INSERT ... ON DUPLICATE
KEY UPDATE statement when making the change.
- Around line 137-142: The UPDATE of notification_mute_setting (statement
touching alias nms and temp_duplicate_room_map m) can violate the UNIQUE
(user_id, target_type, target_id_key) constraint when a user already has a mute
row for the keep_room_id; before performing the UPDATE nms.target_id =
m.keep_room_id add conflict resolution: delete existing conflicting rows for the
same user (e.g., DELETE FROM notification_mute_setting WHERE
target_type='CHAT_ROOM' AND target_id IN (SELECT keep_room_id FROM
temp_duplicate_room_map) AND (user_id, target_type, target_id) matches to avoid
duplicates), or alternatively replace the UPDATE with an INSERT ... ON DUPLICATE
KEY UPDATE pattern that merges loser rows into the kept row; implement one of
these approaches in the notification_mute_setting migration block to ensure no
unique-key collisions occur when mapping from_room_id -> keep_room_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f230128f-da24-4a7c-bc60-7bac60f7efe1
📒 Files selected for processing (1)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
재시도 시 ON DUPLICATE KEY UPDATE로 인해 keep/from 방향이 뒤바뀔 수 있는 문제를 해결하기 위해: - 매핑 테이블이 비어있을 때만 INSERT 실행 (NOT EXISTS 조건 추가) - ON DUPLICATE KEY UPDATE 절 제거 이제 재시도 시 기존 매핑이 유지되어 병합 방향이 일관되게 유지됨.
notification_mute_setting UPDATE 시 (user_id, target_type, target_id) UNIQUE 제약조건 위반 문제를 해결하기 위해: - UPDATE 전에 keep_room에 이미 존재하는 뮤트 설정을 먼저 삭제 - 동일 사용자가 from_room과 keep_room 모두 뮤트 설정 시 from_room 설정이 우선하도록 처리 이제 중복 키 충돌 없이 알림 뮤트 설정 병합 가능.
발견된 잠재적 문제 및 해결: 1. 연산자 우선순위 버그 (치명적) - WHERE a AND b OR c → WHERE a AND (b OR c) 로 괄호 추가 2. chat_room_member 데이터 유실 (치명적) - loser 방에만 있는 멤버 INSERT 추가 (4b단계) - is_owner 컬럼 병합 로직 추가 (누락되었었음) 3. notification_mute_setting UNIQUE 충돌 - UPDATE 전 keep 방의 충돌하는 row 먼저 삭제 4. 재시도 안정성 - NOT EXISTS + ON DUPLICATE KEY UPDATE 제거로 매핑 보호 5. 주석 및 문서화 - 엣지케이스 설명 추가 - 각 단계 목적 명확화
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql (3)
160-188:⚠️ Potential issue | 🟠 Major[LEVEL: MAJOR] 삭제 직후 helper map을 drop하면 운영 복구 근거가 사라집니다.
Line 161-164에서 loser room을 삭제한 뒤 Line 187-188에서
temp_duplicate_room_map까지 제거하면, 잘못 병합된 건을 추적하거나 수동 복구할from_room_id -> keep_room_id근거가 남지 않습니다. 검증 기간 동안은 매핑과 원본 멤버십 스냅샷을 영구 백업 테이블에 남기고, 정리는 별도 migration으로 분리하는 편이 안전합니다. As per coding guidelines, "파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다."
171-181:⚠️ Potential issue | 🟠 Major[LEVEL: MAJOR]
latest_msg가 "최신 created_at, 동률 시 id" 규칙을 구현하지 못하고 있습니다.현재 서브쿼리는 방별
MAX(id)만 고르므로, winner 선정에 쓰는last_message_at기준과 room summary 갱신 기준이 달라집니다.last_message_*는created_at DESC, id DESC로 정렬한 1건을 뽑아야 실제 마지막 메시지와 일치합니다.최소 수정 예시
LEFT JOIN ( - SELECT - cm1.chat_room_id, - cm1.content, - cm1.created_at - FROM chat_message cm1 - JOIN ( - SELECT chat_room_id, MAX(id) AS max_id - FROM chat_message - GROUP BY chat_room_id - ) cm2 ON cm2.chat_room_id = cm1.chat_room_id AND cm2.max_id = cm1.id + SELECT chat_room_id, content, created_at + FROM ( + SELECT + chat_room_id, + content, + created_at, + ROW_NUMBER() OVER ( + PARTITION BY chat_room_id + ORDER BY created_at DESC, id DESC + ) AS rn + FROM chat_message + ) ranked_messages + WHERE rn = 1 ) latest_msg ON latest_msg.chat_room_id = cr.id🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql` around lines 171 - 181, The current subquery that builds latest_msg selects MAX(id) per chat_room_id (cm2.max_id) which doesn’t implement the required tie-breaker by created_at then id; replace the cm1/cm2 MAX(id) approach with a deterministic "pick latest by created_at DESC, id DESC" selection (e.g., use ROW_NUMBER() OVER (PARTITION BY chat_room_id ORDER BY created_at DESC, id DESC) and filter where row_number = 1) so the joined latest_msg matches the same ordering used for last_message_at and room summaries; update the LEFT JOIN block that references chat_message, cm1 and cm2 accordingly.
132-152:⚠️ Potential issue | 🔴 Critical[LEVEL: CRITICAL] 여러 loser 방의 mute row가 같은 keep 방으로 모이면 아직도 UNIQUE 충돌이 납니다.
Line 134-144는 기존 keep-room row만 지웁니다. 3개 이상 중복 방에서 같은 사용자가 loser 방 둘 이상을 mute한 경우, Line 147-152가 여러 row를 동일한
(user_id, CHAT_ROOM, keep_room_id)로 동시에 갱신하게 되어uq_notification_mute_setting_user_target가 다시 깨집니다. update 전에(user_id, keep_room_id)기준으로 loser mute를 1건으로 집계/정리하거나,INSERT ... SELECT ... ON DUPLICATE KEY UPDATE형태로 병합해야 합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql` around lines 132 - 152, The current UPDATE on notification_mute_setting (joined to temp_duplicate_room_map) will cause UNIQUE key violations when multiple loser rows for the same user map to the same keep_room_id; fix by merging loser rows per (user_id, keep_room_id) before writing. Replace the direct UPDATE of notification_mute_setting (the block using temp_duplicate_room_map -> UPDATE nms JOIN temp_duplicate_room_map m ...) with an INSERT ... SELECT that aggregates losers by user_id and keep_room_id (e.g., SELECT user_id, 'CHAT_ROOM' AS target_type, m.keep_room_id AS target_id, MAX(updated_at) AS updated_at, ... GROUP BY user_id, m.keep_room_id) into notification_mute_setting and use ON DUPLICATE KEY UPDATE to merge values (keeping the newest updated_at), then delete the original loser rows (target_id = m.from_room_id); reference temp_duplicate_room_map, notification_mute_setting and uq_notification_mute_setting_user_target in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql`:
- Around line 171-181: The current subquery that builds latest_msg selects
MAX(id) per chat_room_id (cm2.max_id) which doesn’t implement the required
tie-breaker by created_at then id; replace the cm1/cm2 MAX(id) approach with a
deterministic "pick latest by created_at DESC, id DESC" selection (e.g., use
ROW_NUMBER() OVER (PARTITION BY chat_room_id ORDER BY created_at DESC, id DESC)
and filter where row_number = 1) so the joined latest_msg matches the same
ordering used for last_message_at and room summaries; update the LEFT JOIN block
that references chat_message, cm1 and cm2 accordingly.
- Around line 132-152: The current UPDATE on notification_mute_setting (joined
to temp_duplicate_room_map) will cause UNIQUE key violations when multiple loser
rows for the same user map to the same keep_room_id; fix by merging loser rows
per (user_id, keep_room_id) before writing. Replace the direct UPDATE of
notification_mute_setting (the block using temp_duplicate_room_map -> UPDATE nms
JOIN temp_duplicate_room_map m ...) with an INSERT ... SELECT that aggregates
losers by user_id and keep_room_id (e.g., SELECT user_id, 'CHAT_ROOM' AS
target_type, m.keep_room_id AS target_id, MAX(updated_at) AS updated_at, ...
GROUP BY user_id, m.keep_room_id) into notification_mute_setting and use ON
DUPLICATE KEY UPDATE to merge values (keeping the newest updated_at), then
delete the original loser rows (target_id = m.from_room_id); reference
temp_duplicate_room_map, notification_mute_setting and
uq_notification_mute_setting_user_target in your change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f11e47aa-530f-4765-9687-d57822367f04
📒 Files selected for processing (1)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Codex adversarial review에서 지적된 문제 해결: 1. chat_room_member orphan INSERT PK 충돌 - GROUP BY (keep_room_id, user_id)로 집계하여 중복 삽입 방지 - MAX/MIN으로 각 컬럼 병합 규칙 적용 2. notification_mute_setting UNIQUE 충돌 - GROUP BY로 loser 설정 집계 (temp_mute_setting_agg) - MAX(is_muted)로 동일 사용자의 여러 설정 병합 - DELETE-INSERT 패턴으로 충돌 없이 마이그레이션 이제 다중 loser 방 → 단일 keep 방 시나리오에서도 안전하게 마이그레이션 실행 가능.
MySQL의 LEAST/GREATEST는 인자 중 하나라도 NULL이면 결과도 NULL을 반환하는 문제 해결: - visible_message_from: CASE로 NULL 처리 후 LEAST - last_read_at: CASE로 NULL 처리 후 GREATEST - left_at: 기존 CASE 패턴 유지 이제 loser/keep 중 하나라도 NULL이어도 유효한 값이 보존되고, 둘 다 NULL이면 NULL 유지.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql (1)
31-59:⚠️ Potential issue | 🟠 Major[LEVEL: MAJOR] 재시도 경로에서 “0명 loser 방”이 후보에 다시 포함되지 않습니다.
Line 43-47의
JOIN chat_room_member c1/c2가 2개 멤버 행을 전제해서, Line 55-58의OR EXISTS가 참이어도 멤버가 0명인 room은 결과에서 탈락합니다. 주석 의도(재시도 시 0명 방 처리)와 실제 동작이 불일치합니다.최소 수정 예시
CREATE TABLE temp_direct_room_pairs AS -SELECT +SELECT cr.id AS room_id, LEAST(c1.user_id, c2.user_id) AS user1_id, GREATEST(c1.user_id, c2.user_id) AS user2_id, cr.created_at, ( SELECT MAX(cm.created_at) FROM chat_message cm WHERE cm.chat_room_id = cr.id ) AS last_message_at FROM chat_room cr JOIN chat_room_member c1 ON c1.chat_room_id = cr.id JOIN chat_room_member c2 ON c2.chat_room_id = cr.id AND c1.user_id < c2.user_id WHERE cr.room_type = 'DIRECT' AND ( ( SELECT COUNT(*) FROM chat_room_member m WHERE m.chat_room_id = cr.id ) = 2 - OR EXISTS ( - SELECT 1 FROM temp_duplicate_room_map existing - WHERE existing.from_room_id = cr.id OR existing.keep_room_id = cr.id - ) - ); + ) +UNION DISTINCT +SELECT + cr.id AS room_id, + existing.user1_id, + existing.user2_id, + cr.created_at, + ( + SELECT MAX(cm.created_at) + FROM chat_message cm + WHERE cm.chat_room_id = cr.id + ) AS last_message_at +FROM chat_room cr +JOIN temp_duplicate_room_map existing + ON existing.from_room_id = cr.id OR existing.keep_room_id = cr.id +WHERE cr.room_type = 'DIRECT';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql` around lines 31 - 59, The query drops rooms with zero members because the inner JOINs to chat_room_member (c1/c2) filter them out before the EXISTS check; update the temp_direct_room_pairs creation to use LEFT JOIN for chat_room_member c1 and c2 (keep the pairing predicate c1.user_id < c2.user_id in the ON clause, not in WHERE) so rooms with no members are preserved and still included when the OR EXISTS (temp_duplicate_room_map) branch matches; targets: temp_direct_room_pairs, chat_room_member c1/c2, and temp_duplicate_room_map.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql`:
- Around line 247-257: The current subquery uses MAX(id) to pick the "latest"
message (cm2 selecting MAX(id)), which can pick a message with an older
created_at; change the logic to select by MAX(created_at) instead and break ties
deterministically by id: replace the cm2 subquery with one that selects
chat_room_id and MAX(created_at) AS max_created_at, join cm1 on chat_room_id AND
cm1.created_at = cm2.max_created_at, and to handle multiple messages with
identical created_at add a secondary condition ensuring cm1.id = (SELECT MAX(id)
FROM chat_message WHERE chat_room_id = cm1.chat_room_id AND created_at =
cm2.max_created_at) so the chosen row is the true latest by timestamp with id as
a tiebreaker.
---
Duplicate comments:
In `@src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql`:
- Around line 31-59: The query drops rooms with zero members because the inner
JOINs to chat_room_member (c1/c2) filter them out before the EXISTS check;
update the temp_direct_room_pairs creation to use LEFT JOIN for chat_room_member
c1 and c2 (keep the pairing predicate c1.user_id < c2.user_id in the ON clause,
not in WHERE) so rooms with no members are preserved and still included when the
OR EXISTS (temp_duplicate_room_map) branch matches; targets:
temp_direct_room_pairs, chat_room_member c1/c2, and temp_duplicate_room_map.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 85281e45-8496-45bf-9c80-8da112f2ab8b
📒 Files selected for processing (1)
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🧰 Additional context used
📓 Path-based instructions (2)
src/main/resources/db/migration/**/*.sql
⚙️ CodeRabbit configuration file
src/main/resources/db/migration/**/*.sql: Flyway 마이그레이션 리뷰 규칙:
- 버전 파일명 규칙(V{number}__{description}.sql) 위반 여부를 우선 확인한다.
- 이미 배포된 마이그레이션 수정/재번호 부여 위험이 있으면 반드시 차단 코멘트를 남긴다.
- 파괴적 변경(drop, rename 등)은 롤백 가능성과 운영 영향 관점에서 검토한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
**/*
⚙️ CodeRabbit configuration file
**/*: 공통 리뷰 톤 가이드:
- 모든 코멘트는 첫 줄에
[LEVEL: ...]태그를 포함한다.- 과장된 표현 없이 사실 기반으로 작성한다.
- 한 코멘트에는 하나의 이슈만 다룬다.
- 코드 예시가 필요하면 최소 수정 예시를 제시한다.
- 가독성/단순화/확장성 이슈를 발견하면 우선순위를 높여 코멘트한다.
Files:
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
src/main/resources/db/migration/V69__merge_duplicate_direct_chat_rooms.sql
Outdated
Show resolved
Hide resolved
last_message_content 갱신 시 MAX(id) 대신 MAX(created_at)을 사용하여 실제 최신 메시지를 선택하도록 수정: - MAX(created_at)으로 최신 메시지 우선 선택 - 동일 타임스탬프 시 MAX(id)로 타임브레이커 적용 - 이전: id 기준 (시간 역순 불가능한 엣지케이스 존재) - 이후: 시간 기준 (결정론적이고 정확한 최신 메시지)
재시도 시 이미 처리된 방(from_room_id/keep_room_id)이 0명이 될 수 있는 경우를 처리하기 위해: - JOIN → LEFT JOIN 변경 - 0명 방도 EXISTS 조건으로 포함되도록 보존 - c1.user_id < c2.user_id는 ON 절에 유지 이제 이전 실행에서 멤버가 모두 삭제된 방도 재시도 시 매핑 테이블에서 찾을 수 있음.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 개요
이전 마이그레이션의 문제로 동일 유저를 대상으로 한 채팅방이 중복되어 만들어진 문제가 발생했습니다.
이에 따라
POST /chats/roomsAPI를 통해 특정 유저와 채팅방을 만들려면 이미 만들어진 여러 개의 방으로 인해 단일 조회가 깨지게 되었습니다.이를 해결하기 위해 중복되는 방은 채팅 메시지를 병합하여 하나의 채팅방으로 통합합니다.
🚀 주요 변경 내용
V69__merge_duplicate_direct_chat_rooms.sql💬 참고 사항
✅ Checklist (완료 조건)