-
Notifications
You must be signed in to change notification settings - Fork 0
Comment: comment 모듈 추가 및 application, api 작성 #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…radle.kts, yml, build.gradle.kts)
services/comment/driven/rdb/src/main/java/nettee/reply/entity/ReplyEntity.java
Outdated
Show resolved
Hide resolved
var comments = commentQueryRepositoryPort.findPageByBoardId(boardId, 0, 10); | ||
|
||
// comment별로 reply를 10개씩 가져옴 | ||
// 현재 N+1 이므로, 최대 11(1+10)개의 쿼리를 발생시킴 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 기능에 대한 최적화를 말씀해주셔서 저도 고민을 해볼 수 있어서 좋았습니다.
commentsId를 꺼낸 다음에, 한꺼번에 조회를 하는 방식은 어떤지 궁금합니다.
한꺼번에 조회를 한 다음 정리를 해주면 될 것 같다는 생각이 들었습니다.
List commentIds = comments.stream()
.map(Comment::getId)
.collect(Collectors.toList());
List replies = replyQueryRepositoryPort.findAllByCommentIdIn(commentIds);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 정말 좋은 고민이였습니다.
해당 기능에 현재 QueryDSL이 제공되는 문법에는 윈도우 함수 적용이 쉽지가 않습니다. 또한 댓글과 대댓글에 케이스는 하나의 댓글에 몇개의 대댓글을 가져올지 ROW 개수 제한도 포함되어야 합니다.
🤔 저는 선택적으로 Native Query를 사용해야 한다고 생각합니다!
수용님이 작성하신 부분에서 더 수정을 하자면
// limit 갯수 추가
List replies = replyQueryRepositoryPort.findAllByCommentIdIn(commentIds, 10);
SELECT *
FROM (
SELECT *,
ROW_NUMBER() OVER (PARTITION BY reply.comment_id ORDER BY reply.created_at ASC) as rn
FROM reply
WHERE reply.comment_id IN (:commentIds)
) ranked
WHERE rn <= (:limit)
이렇게 되면 하기 와 같이 DB에서 조회가 되며 (5개만 조회)
조회 후에는 Application 단에서 commentId에 맞게 매핑을 시켜주면 된다고 생각합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음.. 결국 특정 DB에 종속되는 건데 ROW_NUMBER()가 대부분의 RDBMS가 지원한다고 하더라도
특정 버전 (ex. MySQL 8.0 미만)의 경우 ROW_NUMBER()를 지원하지 않습니다.
이런 경우는 배제하고 효율을 챙기는 게 좋을까요?? @merge-simpson @sweetykr7 @silberbullet @wch-os
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*참고
관련 논의는 'JSON 필드 사용: reply 첫 페이지에 대해서만 JSON 필드에 중복으로 관리' 방식을 시도하는 것으로 이야기되었습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
많은 작업을 하시느라 고생 많으셨습니다. 작업량이 상당하셨는데 중간 중간 고민하신 포인트들 덕분에 저도 고민해볼 수 있어서 좋았습니다~!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 작업하시느라 고생하셨습니다!
현재 기본 동작들에는 문제가 없어 🚀 Approve 합니다.
다만, 코드 스타일 적으로 관련된 질의는 남겨두겠습니다!
services/comment/driven/rdb/src/main/java/nettee/reply/driven/rdb/entity/ReplyEntity.java
Outdated
Show resolved
Hide resolved
services/comment/driven/rdb/src/main/java/nettee/comment/driven/rdb/entity/CommentEntity.java
Outdated
Show resolved
Hide resolved
var comments = commentQueryRepositoryPort.findPageByBoardId(boardId, 0, 10); | ||
|
||
// comment별로 reply를 10개씩 가져옴 | ||
// 현재 N+1 이므로, 최대 11(1+10)개의 쿼리를 발생시킴 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 정말 좋은 고민이였습니다.
해당 기능에 현재 QueryDSL이 제공되는 문법에는 윈도우 함수 적용이 쉽지가 않습니다. 또한 댓글과 대댓글에 케이스는 하나의 댓글에 몇개의 대댓글을 가져올지 ROW 개수 제한도 포함되어야 합니다.
🤔 저는 선택적으로 Native Query를 사용해야 한다고 생각합니다!
수용님이 작성하신 부분에서 더 수정을 하자면
// limit 갯수 추가
List replies = replyQueryRepositoryPort.findAllByCommentIdIn(commentIds, 10);
SELECT *
FROM (
SELECT *,
ROW_NUMBER() OVER (PARTITION BY reply.comment_id ORDER BY reply.created_at ASC) as rn
FROM reply
WHERE reply.comment_id IN (:commentIds)
) ranked
WHERE rn <= (:limit)
이렇게 되면 하기 와 같이 DB에서 조회가 되며 (5개만 조회)
조회 후에는 Application 단에서 commentId에 맞게 매핑을 시켜주면 된다고 생각합니다.
Pull Request
Issues
Description
api, application, driven/rdb, driving/web 모듈 추가
postgresql migration script를 작성했습니다.
basePackage 컨벤션에 맞게 리팩토링 진행했습니다.
How Has This Been Tested?
Comment, Reply에 대한 Create, Update, Delete API를 실행한 후,
Read API를 실행시킨 결과입니다.
Additional Notes
현재 작성한 댓글 조회 기능에서 댓글과 대댓글을 10개씩 가져오는데,
이 과정에서 N+1 비효율이 발생합니다.
게시물에 대한 댓글을 최대 10개, 또한 각각의 댓글에 대한 대댓글을 최대 10개 불러와야 하지만,
QueryDSL에서 제공하는 기본 구문으로는 이 코드를 최적화 할 수 없습니다.
nettee.comment.application.service.CommentQueryService의 함수입니다.
위 코드를 NativeQuery를 써서 최적화를 할지
vs
11개의 쿼리는 N+1 문제일지라도 예측 가능한 범위 내에 있기 때문에 그대로 둘지
에 대한 고민은 해봐야 할 것 같습니다.