Refactor: 주식 엔티티 id 전략 변경 #84
Conversation
- 기존 identity에서 sequence로의 변경
WalkthroughSwitches JPA ID generation for two stock entity classes from IDENTITY to SEQUENCE with named generators and allocationSize=50, and consolidates persistence imports to jakarta.persistence.*. No other fields or behaviors changed. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant EM as JPA EntityManager
participant DB as Database (Sequence)
App->>EM: persist(Entity)
EM->>DB: SELECT nextval('..._stock_info_seq')
DB-->>EM: sequence value
EM->>DB: INSERT into table with generated ID
DB-->>EM: insert OK
EM-->>App: managed entity with ID
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (4)
src/main/java/naughty/tuzamate/domain/stock/entity/KrxStockInfo.java (1)
3-3: Prefer explicit JPA imports over wildcard.Improves readability and keeps Checkstyle/IDE settings happy in many codebases.
Apply this diff:
-import jakarta.persistence.*; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.SequenceGenerator;src/main/java/naughty/tuzamate/domain/stock/entity/NasdaqStockInfo.java (3)
14-20: Confirm matching DB sequence and migration exist for nasdaq_stock_info_seq.Same concerns as KRX: DB support, sequence creation, increment, and initialization to avoid PK conflicts with existing rows.
If using Postgres:
CREATE SEQUENCE IF NOT EXISTS nasdaq_stock_info_seq INCREMENT BY 50; SELECT setval('nasdaq_stock_info_seq', GREATEST((SELECT COALESCE(MAX(id),0) FROM nasdaq_stock_info), 1), true);I can add Flyway migrations if you’d like.
3-3: Avoid wildcard JPA import for consistency.Apply this diff:
-import jakarta.persistence.*; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.SequenceGenerator;
19-19: Minor: align comment style on allocationSize with KRX file or remove both.KRX includes a Korean inline comment explaining the 50-ID pool; this file doesn’t. Consider adding a brief comment here or removing in both for consistency.
- allocationSize = 50 + allocationSize = 50 // pre-allocate 50 IDs for batching
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/main/java/naughty/tuzamate/domain/stock/entity/KrxStockInfo.java(2 hunks)src/main/java/naughty/tuzamate/domain/stock/entity/NasdaqStockInfo.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/naughty/tuzamate/domain/stock/entity/KrxStockInfo.java (3)
src/main/java/naughty/tuzamate/domain/stock/repository/KrxStockInfoRepository.java (1)
KrxStockInfoRepository(6-7)src/main/java/naughty/tuzamate/domain/stock/entity/StockCode.java (1)
StockCode(10-24)src/main/java/naughty/tuzamate/domain/stock/dto/krx/KrxDto.java (1)
KrxStockInfoDto(39-62)
src/main/java/naughty/tuzamate/domain/stock/entity/NasdaqStockInfo.java (3)
src/main/java/naughty/tuzamate/domain/stock/dto/nasdaq/NasdaqDto.java (1)
NasdaqInfoDto(10-33)src/main/java/naughty/tuzamate/domain/stock/repository/NasdaqStockInfoRepository.java (1)
NasdaqStockInfoRepository(6-7)src/main/java/naughty/tuzamate/domain/savings/entity/Savings.java (1)
Savings(6-35)
🔇 Additional comments (3)
src/main/java/naughty/tuzamate/domain/stock/entity/KrxStockInfo.java (2)
14-20: Good call switching to SEQUENCE — unlocks batching and better insert throughput.Moving off IDENTITY allows Hibernate to pre-allocate IDs and batch inserts effectively. The dedicated generator per-entity is also a clean separation.
19-19: allocationSize=50: ensure DB sequence increment matches for optimal performance.For Hibernate’s pooled optimizer to be effective (and to avoid warnings), set the DB sequence INCREMENT BY 50. Otherwise you’ll still work correctly but incur extra sequence calls and see larger gaps on restarts.
Example (Postgres):
ALTER SEQUENCE krx_stock_info_seq INCREMENT BY 50;src/main/java/naughty/tuzamate/domain/stock/entity/NasdaqStockInfo.java (1)
14-20: SEQUENCE strategy looks good and consistent with KRX.Dedicated sequence and generator name mirror KrxStockInfo and enable batching.
| @GeneratedValue(strategy = GenerationType.SEQUENCE, | ||
| generator = "krx_stock_info_seq") | ||
| @SequenceGenerator( | ||
| name = "krx_stock_info_seq", | ||
| sequenceName = "krx_stock_info_seq", | ||
| allocationSize = 50 // 시퀀스 한 번에 50개 id 확보 | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify DB dialect and add DB migrations for the sequence (prod/staging).
SEQUENCE requires DB support and presence of the named sequence. If you’re on MySQL, this will break; if Postgres/Oracle, add migrations to create and initialize the sequence(s) accordingly. Also ensure start value ≥ max(id)+1 to avoid PK collisions on existing tables.
Run the following to verify DB dialect and presence of sequence migrations:
If you’re on Postgres, a Flyway SQL example:
-- Vxxx__create_stock_sequences.sql
CREATE SEQUENCE IF NOT EXISTS krx_stock_info_seq INCREMENT BY 50;
SELECT setval('krx_stock_info_seq', GREATEST((SELECT COALESCE(MAX(id),0) FROM krx_stock_info), 1), true);
CREATE SEQUENCE IF NOT EXISTS nasdaq_stock_info_seq INCREMENT BY 50;
SELECT setval('nasdaq_stock_info_seq', GREATEST((SELECT COALESCE(MAX(id),0) FROM nasdaq_stock_info), 1), true);If on Oracle:
BEGIN
EXECUTE IMMEDIATE 'CREATE SEQUENCE krx_stock_info_seq INCREMENT BY 50 START WITH 1';
EXCEPTION WHEN OTHERS THEN IF SQLCODE != -955 THEN RAISE; END IF; END;
/
BEGIN
EXECUTE IMMEDIATE 'CREATE SEQUENCE nasdaq_stock_info_seq INCREMENT BY 50 START WITH 1';
EXCEPTION WHEN OTHERS THEN IF SQLCODE != -955 THEN RAISE; END IF; END;
/If on MySQL/MariaDB: SEQUENCE is not supported in MySQL (and differs in MariaDB). You’ll need IDENTITY or TABLE generator there. I can generate the required DB-specific migrations or revert strategy if needed.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Checking DB dialect config (application.*) =="
fd -a 'application.*' src/main/resources | xargs -r -I{} sh -c 'echo "-- {} --"; rg -n -C2 -e "spring.datasource.url|spring.jpa.database-platform|hibernate.dialect" "{}" || true'
echo
echo "== Stock entities using @GeneratedValue strategies =="
rg -nP -C3 --glob 'src/main/java/**/domain/stock/entity/**' '@GeneratedValue\s*\(.*?GenerationType\.\w+'
echo
echo "== Migration scripts referencing the new sequences =="
rg -n -C2 -e 'create sequence|alter sequence|setval|krx_stock_info_seq|nasdaq_stock_info_seq' --hiddenLength of output: 415
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Repo top-level =="
ls -la
echo
echo "== Find application.* files =="
fd -a 'application.*' || true
echo
echo "== Search for Spring DB / Hibernate config in repository =="
rg -nS --hidden -e 'spring\.datasource|spring\.jpa|hibernate\.dialect|spring\.datasource\.url|spring\.jpa\.database-platform' || true
echo
echo "== Search for GenerationType.SEQUENCE / @SequenceGenerator / @GeneratedValue usages =="
rg -nP --hidden -C3 -e 'GenerationType\.SEQUENCE|@SequenceGenerator|@GeneratedValue' || true
echo
echo "== Search for migration SQL referencing sequences =="
rg -nS --hidden -e 'krx_stock_info_seq|create sequence|CREATE SEQUENCE|setval|alter sequence' || true
echo
echo "== Locate KrxStockInfo.java and show its top 200 lines if present =="
file_path=$(rg -n --hidden -S -g '!**/target/**' 'class\s+KrxStockInfo' | sed -n '1s/:.*//p' || true)
if [ -n "$file_path" ]; then
echo "-- $file_path --"
sed -n '1,200p' "$file_path"
else
echo "KrxStockInfo.java not found"
fiLength of output: 11556
Add DB migrations for SEQUENCE-based IDs or switch to a DB-compatible strategy
Quick summary: I verified the repo — two entities use GenerationType.SEQUENCE but there are no DB dialect/config entries or migration scripts creating those sequences. This will fail on MySQL (which lacks SEQUENCE) and will still require explicit sequence creation on Postgres/Oracle to avoid PK collisions.
Files needing attention:
- src/main/java/naughty/tuzamate/domain/stock/entity/KrxStockInfo.java — generator "krx_stock_info_seq"
- src/main/java/naughty/tuzamate/domain/stock/entity/NasdaqStockInfo.java — generator "nasdaq_stock_info_seq"
Verification notes:
- No occurrences of spring.datasource / spring.jpa / hibernate.dialect were found in the repo (no explicit DB dialect/config detected).
- No migration SQL referencing krx_stock_info_seq / nasdaq_stock_info_seq (CREATE SEQUENCE / setval / ALTER SEQUENCE) detected.
Actionable fixes (pick one):
- If you use Postgres/Oracle: add migrations to create the sequences and set their current values >= MAX(id)+1. Example Flyway SQL for Postgres:
-- Vxxx__create_stock_sequences.sql
CREATE SEQUENCE IF NOT EXISTS krx_stock_info_seq INCREMENT BY 50;
SELECT setval('krx_stock_info_seq', GREATEST((SELECT COALESCE(MAX(id),0) FROM krx_stock_info)+1, 1), true);
CREATE SEQUENCE IF NOT EXISTS nasdaq_stock_info_seq INCREMENT BY 50;
SELECT setval('nasdaq_stock_info_seq', GREATEST((SELECT COALESCE(MAX(id),0) FROM nasdaq_stock_info)+1, 1), true); - If you use MySQL/MariaDB: SEQUENCE is unsupported — change entities to GenerationType.IDENTITY (or use a TABLE generator / application-side id allocation) instead.
- Ensure allocationSize (50) matches the sequence INCREMENT BY and that the migration start value prevents PK collisions.
Please add the migrations (or change the generation strategy) and ensure they are applied in prod/staging.
🤖 Prompt for AI Agents
In src/main/java/naughty/tuzamate/domain/stock/entity/KrxStockInfo.java (lines
~14-20) and
src/main/java/naughty/tuzamate/domain/stock/entity/NasdaqStockInfo.java, the
entities use GenerationType.SEQUENCE with named generators ("krx_stock_info_seq"
/ "nasdaq_stock_info_seq") but the repo has no DB dialect config or migration
scripts to create those sequences; either add DB migrations for your chosen DB
(e.g., Flyway/V1__create_stock_sequences.sql that creates the two sequences with
INCREMENT BY matching allocationSize and sets their start values >= MAX(id)+1)
if using Postgres/Oracle, or change the entities to a DB-compatible strategy
such as GenerationType.IDENTITY (or TABLE generator/application id allocation)
if using MySQL/MariaDB; also ensure allocationSize (50) matches the sequence
INCREMENT BY in the migration to avoid PK collisions and apply these migrations
in prod/staging.
|
MySQL은 sequence 전략이 적용되지 않아 PR 필요없다 |
1 participant
📍 PR 타입 (하나 이상 선택)
📌 개요
🔁 변경 사항
📸 스크린샷
✅ 체크리스트
💡 추가 사항 (리뷰어가 참고해야 할 것)
Summary by CodeRabbit