refactor: 이메일 유니크키 제거 #525
Conversation
Gyuhyeok99
requested review from
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
whqtker
as code owners
Walkthrough
Estimated code review effort
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/db/migration/V33__drop_site_user_email_unique.sql(1 hunks)
⏰ 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: build
| @@ -0,0 +1 @@ | |||
| ALTER TABLE site_user DROP KEY site_user_email_unique; | |||
There was a problem hiding this comment.
중복 키 제거 후 일반 인덱스를 추가해 주세요. 이메일로 사용자 조회가 빈번한 서비스 특성상, 유니크 키를 삭제하면 해당 컬럼의 인덱스 자체가 사라져 쿼리 성능이 급격히 떨어질 수 있어요. DROP 뒤에 동일 컬럼으로 일반 인덱스를 생성해 주시면 성능 저하를 예방할 수 있겠습니다.
ALTER TABLE site_user DROP KEY site_user_email_unique;
+CREATE INDEX idx_site_user_email ON site_user (email);🤖 Prompt for AI Agents
In src/main/resources/db/migration/V33__drop_site_user_email_unique.sql around
lines 1 to 1, after dropping the unique key you must create a non-unique index
on the same email column to avoid query performance degradation; modify the
migration so that immediately after ALTER TABLE site_user DROP KEY
site_user_email_unique; you add a CREATE INDEX (e.g., CREATE INDEX
idx_site_user_email ON site_user(email);) ensuring the index is non-unique and
uses the same column name.
There was a problem hiding this comment.
유니크 특정이 email + authType으로 되고 있고 그거에 대한 유니크 키(+ 인덱스)가 있는데 상관 없지 않아?
@coderabbitai
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for composite unique key on (email, authType) in migration files
echo "=== Searching for composite unique key on email and authType ==="
rg -n -A2 -B2 "UNIQUE.*email.*authType|UNIQUE.*authType.*email|email.*authType.*UNIQUE|authType.*email.*UNIQUE" src/main/resources/db/migration/
echo -e "\n=== Checking for unique constraints in site_user table creation ==="
rg -n -A10 -B5 "CREATE TABLE.*site_user" src/main/resources/db/migration/
echo -e "\n=== Looking for any unique key definitions with email ==="
rg -n "UNIQUE KEY.*email" src/main/resources/db/migration/
Length of output: 1657
🏁 Script executed:
#!/bin/bash
# Get the full site_user table definition
echo "=== Full site_user table definition ==="
sed -n '/CREATE TABLE.*site_user/,/^);/p' src/main/resources/db/migration/V1__init.sql
echo -e "\n=== Searching for ALTER TABLE site_user with UNIQUE ==="
rg -n "ALTER TABLE.*site_user.*UNIQUE|ALTER TABLE.*site_user.*ADD.*KEY" src/main/resources/db/migration/
echo -e "\n=== Looking for authType column ==="
rg -n "authType|auth_type" src/main/resources/db/migration/V1__init.sql
echo -e "\n=== All unique key additions in migration files ==="
rg -n "ADD.*UNIQUE|CREATE.*UNIQUE" src/main/resources/db/migration/
Length of output: 1090
🏁 Script executed:
#!/bin/bash
# Search for authType across all migration files
echo "=== Searching for authType in all migration files ==="
rg -n "authType|auth_type" src/main/resources/db/migration/
echo -e "\n=== List all migration files to see the sequence ==="
ls -1 src/main/resources/db/migration/ | sort -V
echo -e "\n=== Check if there are any recent migrations that add composite unique key ==="
rg -n -C3 "email.*auth|auth.*email" src/main/resources/db/migration/
echo -e "\n=== Look for all CONSTRAINT or KEY definitions in site_user ==="
rg -n "site_user" src/main/resources/db/migration/ | grep -i "constraint\|key\|index"
Length of output: 8701
@Gyuhyeok99, 맞습니다! 제가 잘못 판단했네요. 😅
V3 마이그레이션에서 이미 uk_site_user_email_auth_type UNIQUE (email, auth_type) 복합 유니크 키가 생성되어 있고, MySQL의 최좌측 프리픽스(leftmost prefix) 규칙에 따라 (email, auth_type) 인덱스는 email 단독 조회에도 효율적으로 사용될 수 있습니다.
따라서 email 단독 유니크 키를 제거하더라도 복합 인덱스가 email 필터링 쿼리의 성능을 충분히 보장하므로, 별도의 일반 인덱스를 추가할 필요가 없습니다.
이전 제안을 철회합니다. 현재 마이그레이션 스크립트는 그대로 사용하셔도 문제없습니다! 👍
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
/email/sign-up엔드포인트에 접근하면EmailSignUpTokenProvider의issueEmailSignUpToken메서드 호출- 해당 메서드에서
existsByEmailAndAuthType를 통해 (email, authType)으로 중복 여부 검증 - 이후 발급받은 signUpToken을 통해 이메일 회원가입 진행,
SignUpService의signUp메서드 호출 validateUserNotDuplicated메서드를 통해 (email, authType)으로 중복 여부 검증- 이후 계속 진행 ...
제가 말씀드린 중복 검증은 이런 부분이었습니다 ! 근데 다시 생각해보니 동시성 문제가 발생하면 애플리케이션 레벨에서 문제를 감지하지 못할 거 같아서 있는 게 낫겠다는 생각으로 바뀌었습니다. 혹시 이러한 부분에서 중복 검증이 없다고 생각하신 건가요 ? 제가 봤을 땐 검증 자체는 동일하다고 보여서요 ... 제가 놓친 부분이 있다면 말씀해주세요 !!
|
네 맞습니다!
이렇게 별도 api이다보니 각각 검증하는 게 더 나을 거 같다는 생각도 가지고있었습니다! |
|
제가 로직이 동일하다는 거에 과하게 초점을 맞췄었네요. 어쨌든 현 상태 유지하는 거는 동의합니다 ! |
2 participants
관련 이슈
작업 내용
중복검증이라는 느낌은 들지 않아서 그냥 이메일 유니크키 제거 스크립트만 작성하였습니다.
특이 사항
어떤 pr이 먼저 반영될지 몰라 저도 V33으로 했습니다! 아래 pr이 먼저 반영된다면 34로 업데이트 해놓겠습니다~
리뷰 요구사항 (선택)
혹시 어떤 점에서 중복검증이라고 생각하셨는지 의견 남겨주시면 좋을 거 같습니다!
전 이거만 반영해도 이슈 없을 것이라고 일단 생각했습니다!