[FIX] 구형 MIDI 데이터 호환 마이그레이션 추가 - #112
Merged
Merged
Conversation
ownue
requested review from
kimyw1018,
on1yoneprivate,
p1001q and
rkdehdrbs7885-oss
August 1, 2026 14:07
📝 WalkthroughWalkthrough
ChangesMIDI 데이터 정규화
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant V3Migration as V3 migration
participant ValidationBlock as PLpgSQL validation block
participant MidiData as playing.midi_data
V3Migration->>ValidationBlock: execute validation
ValidationBlock->>MidiData: read non-NULL midi_data
MidiData-->>ValidationBlock: return JSON events
ValidationBlock->>ValidationBlock: validate fields and ranges
ValidationBlock->>MidiData: update normalized JSON
MidiData-->>V3Migration: complete migration
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🧹 Nitpick comments (2)
src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql (2)
10-22: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win검증 루프를 집합 기반 SQL로 바꾸는 것을 검토하세요.
현재 구조는
playing전체 행을 PL/pgSQL 루프로 읽고, 각 행의 이벤트를 다시 루프로 읽습니다. 행 수와 이벤트 수의 곱만큼 인터프리터 오버헤드가 발생합니다. 마이그레이션은 배포 중 테이블 잠금 시간에 직접 영향을 줍니다.집합 기반 검증으로 바꾸면 단일 쿼리로 위반 행을 찾을 수 있습니다. 예를 들어
jsonb_array_elements를LATERAL로 펼친 뒤 위반 조건을WHERE로 모아count(*)또는 첫 위반 행만 조회하고, 결과가 있을 때 한 번만RAISE EXCEPTION을 호출하는 방식입니다.또한 검증 루프는 이미 정규화된 행까지 전부 재검사합니다. UPDATE의
WHERE절(Line 109-116)과 같은 조건으로 대상을 좁히면 불필요한 작업을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql` around lines 10 - 22, Replace the nested validation loops around playing_row and midi_event with a set-based query using jsonb_array_elements via LATERAL, applying the same normalization eligibility conditions as the UPDATE WHERE clause and checking array validity in the query. Capture the first violating playing_id (or equivalent aggregate result) and raise a single exception only when a violation exists, preserving the current error behavior.
109-116: 🚀 Performance & Scalability | 🔵 Trivial멱등성 조건은 정확합니다. 배포 차단 위험과 잠금 시간을 함께 검토하세요.
WHERE절이sequence누락,timestamp_ms누락,timestampMs잔존을 모두 확인하므로 재실행 시 대상 행이 0건이 됩니다. 멱등성 설계가 깔끔합니다.운영 측면에서 두 가지를 검토하세요.
- 검증 블록은 손상 이벤트를 하나만 발견해도 예외를 발생시킵니다. Flyway 마이그레이션이 실패하면 배포가 중단되고 애플리케이션이 기동하지 않습니다. 운영 DB의 레거시 데이터 상태를 미리 확인할 수 없다면, 위반 행을 격리 테이블에 기록하고 정상 행만 정규화하는 방식이 배포 위험을 낮춥니다. 최소한 스테이징에서 같은 데이터셋으로 검증 블록을 먼저 실행하세요.
- 이
UPDATE는playing테이블 전체를 대상 후보로 삼고, Flyway 기본 설정에서 단일 트랜잭션으로 실행됩니다. 행 수가 많으면 잠금 시간과 WAL 증가량이 커집니다. 대용량이라면playing_id범위 기준 배치 처리를 고려하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql` around lines 109 - 116, Review the migration’s validation and UPDATE flow for production safety: run the validation against an equivalent staging dataset before deployment, and if operationally required, isolate rows containing malformed events so valid rows can still be normalized instead of aborting the migration. For large playing tables, replace the single full-table transaction with playing_id-range batches while preserving the existing idempotent predicates for sequence, timestamp_ms, and timestampMs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql`:
- Around line 89-101: Update the sequence fallback in the JSON aggregation to
represent the 0-based order within each identical timestamp group, rather than
the array-wide event_ordinality. Compute the timestamp-partitioned row number in
a subquery before jsonb_agg, while preserving existing sequence values and
timestamp_ms fallback behavior.
---
Nitpick comments:
In `@src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql`:
- Around line 10-22: Replace the nested validation loops around playing_row and
midi_event with a set-based query using jsonb_array_elements via LATERAL,
applying the same normalization eligibility conditions as the UPDATE WHERE
clause and checking array validity in the query. Capture the first violating
playing_id (or equivalent aggregate result) and raise a single exception only
when a violation exists, preserving the current error behavior.
- Around line 109-116: Review the migration’s validation and UPDATE flow for
production safety: run the validation against an equivalent staging dataset
before deployment, and if operationally required, isolate rows containing
malformed events so valid rows can still be normalized instead of aborting the
migration. For large playing tables, replace the single full-table transaction
with playing_id-range batches while preserving the existing idempotent
predicates for sequence, timestamp_ms, and timestampMs.
🪄 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: CHILL
Plan: Pro Plus
Run ID: cea82cd3-88de-475a-b6b5-05da42af5d9e
📒 Files selected for processing (1)
src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql
Comment on lines
+89
to
+101
| SELECT jsonb_agg( | ||
| (midi_event - 'timestampMs' - 'sequence') | ||
| || jsonb_build_object( | ||
| 'sequence', COALESCE( | ||
| NULLIF(midi_event -> 'sequence', 'null'::JSONB), | ||
| to_jsonb((event_ordinality - 1)::INTEGER) | ||
| ), | ||
| 'timestamp_ms', COALESCE( | ||
| NULLIF(midi_event -> 'timestamp_ms', 'null'::JSONB), | ||
| NULLIF(midi_event -> 'timestampMs', 'null'::JSONB) | ||
| ) | ||
| ) | ||
| ORDER BY event_ordinality |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
sequence 기본값의 의미를 재확인하세요.
MidiEventData.sequence의 주석은 "동일 timestamp 내 순서 값"입니다. 여기서는 배열 전체의 0-based 인덱스를 넣습니다. 값은 sequence >= 0 검증을 통과하지만, 의미는 원래 정의와 다릅니다. 이후 동일 timestamp 그룹을 sequence로 정렬하거나 그룹 크기를 계산하는 로직이 추가되면 잘못된 결과가 나옵니다.
동일 timestamp 내 순번으로 채우려면 timestamp 값으로 파티션한 윈도 함수를 사용하세요.
♻️ timestamp 기준 순번을 부여하는 예시
SELECT jsonb_agg(
(midi_event - 'timestampMs' - 'sequence')
|| jsonb_build_object(
'sequence', COALESCE(
NULLIF(midi_event -> 'sequence', 'null'::JSONB),
- to_jsonb((event_ordinality - 1)::INTEGER)
+ to_jsonb((row_number() OVER (
+ PARTITION BY COALESCE(
+ NULLIF(midi_event -> 'timestamp_ms', 'null'::JSONB),
+ NULLIF(midi_event -> 'timestampMs', 'null'::JSONB)
+ )
+ ORDER BY event_ordinality
+ ) - 1)::INTEGER)
),참고: PostgreSQL 윈도 함수 문서(row_number, PARTITION BY)를 확인하세요. 윈도 함수와 jsonb_agg 집계를 같은 SELECT 절에 두면 계산 순서 문제가 생기므로, 서브쿼리로 순번을 먼저 계산한 뒤 집계하는 구조를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/db/migration/V3__normalize_legacy_playing_midi_data.sql`
around lines 89 - 101, Update the sequence fallback in the JSON aggregation to
represent the 0-based order within each identical timestamp group, rather than
the array-wide event_ordinality. Compute the timestamp-partitioned row number in
a subquery before jsonb_agg, while preserving existing sequence values and
timestamp_ms fallback behavior.
rkdehdrbs7885-oss
approved these changes
Aug 1, 2026
on1yoneprivate
approved these changes
Aug 1, 2026
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
3 participants
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.
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
히스토리 API 테스트 중 COMMON_500_01 에러가 뜨는 것을 확인했습니다. -> 로컬 테스트상 문제가 없어, 커밋 목록을 확인한 결과 midi_data의 구조를 변경했을 때 서버 DB에는 반영이 되지 않았던 게 문제라고 판단했습니당...
따라서 마이그레이션 파일을 추가하였습니다.
🔥 리뷰 요청 사항
X
✅ 체크리스트
📎 참고 사항
X
Summary by CodeRabbit