Skip to content
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

[BE] refactor: 상품 자동 완성 API 페이징 방식 변경 #757

Merged
merged 8 commits into from
Dec 29, 2023

Conversation

Go-Jaecheol
Copy link
Collaborator

Issue

✨ 구현한 기능

AS-IS : Offset 기반 페이지네이션
TO-BE : Cursor 기반 페이지네이션

  • page 정보 대신 RequestParam으로 lastId(0이면 첫 요청) 받도록 수정
  • page 정보 대신 Response로 hasNext(데이터 남아있는지 여부) 반환하도록 수정
  • 첫 요청이면 findAllByNameContainingFirst 메서드, 아니면 findAllByNameContaining 메서드 실행
@Query("SELECT p FROM Product p "
            + "WHERE p.name LIKE CONCAT('%', :name, '%') "
            + "ORDER BY "
            + "(CASE WHEN p.name LIKE CONCAT(:name, '%') THEN 1 ELSE 2 END), "
            + "p.id DESC")
List<Product> findAllByNameContainingFirst(@Param("name") final String name, final Pageable pageable);

@Query("SELECT p FROM Product p "
            + "JOIN Product p2 ON p2.id = :lastId "
            + "WHERE p.name LIKE CONCAT('%', :name, '%') "
            + "AND ((p2.name LIKE CONCAT(:name, '%') AND p.id < :lastId) OR (p.name NOT LIKE CONCAT(:name, '%') AND p.id < :lastId)) "
            + "ORDER BY "
            + "(CASE WHEN p.name LIKE CONCAT(:name, '%') THEN 1 ELSE 2 END), "
            + "p.id DESC")
List<Product> findAllByNameContaining(@Param("name") final String name, final Long lastId, final Pageable pageable);

📢 논의하고 싶은 내용

  • 쿼리 괜찮은지..?

    AND ((p2.name LIKE CONCAT(:name, '%') AND p.id < :lastId) OR (p.name NOT LIKE CONCAT(:name, '%') AND p.id < :lastId))

    이 부분 쿼리 간단하게 개선할 수 있을까요?? 그리고 문제는 없을까요?

  • JPQL에서 LIMIT 구문을 사용할 수 없어서

    PageRequest.of(0, PAGE_SIZE)

    다음과 같은 방식으로 PageRequest를 넘기도록 했는데 이 부분에 대해서 어떻게 생각하시나요??

🎸 기타

  • 상품 검색 결과 API와 꿀조합 검색 결과 API는 이 API와 유사하기 때문에, 이 PR 해결되면 바로 똑같이 적용할게요~

⏰ 일정

  • 추정 시간 : 3h (API 3개 기준)
  • 걸린 시간 : 2h (자동완성 API 하나)

@github-actions
Copy link

github-actions bot commented Oct 13, 2023

Test Results

279 tests   279 ✅  16s ⏱️
144 suites    0 💤
144 files      0 ❌

Results for commit 01684c2.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@wugawuga wugawuga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전반적으로 코드가 깔끔하네요 LGTM~
코멘트 한번 확인해주세요!

}

private List<Product> findAllByNameContaining(final String query, final Long lastId) {
final PageRequest size = PageRequest.of(0, PAGE_SIZE);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PageRequest.ofSize(사이즈크기);
이 메소드를 사용해도 좋을 것 같아요! 내부들어가면 똑같습니다!

@wugawuga
Copy link
Collaborator

wugawuga commented Oct 13, 2023

AND ((p2.name LIKE CONCAT(:name, '%') AND p.id < :lastId) OR (p.name NOT LIKE CONCAT(:name, '%') AND p.id < :lastId))

이 부분 개선할 방법이 떠오르지 않네여,,,

하게된다면

AND (p.id < :lastId) AND (p2.name LIKE CONCAT(:name, '%') OR p.name NOT LIKE CONCAT(:name, '%'))

이렇게도 될 것 같아요!

Copy link
Member

@70825 70825 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 개선할 방법이 떠오르지 않네요

그래서 PR 본문 중간이나 맨 아래나 쿼리문 각 줄에 대한 설명도 추가하는건 어떤가요??
왜냐하면 만약 코드를 수정하게 된다면 다른 크루들에게 물어보면서 코드를 바꿀 수 있겠지만, 만약 여건이 안된다면 남은건 문서뿐이라서 git blame을 하면 PR 번호만 있기 때문에 PR 본문에 설명이 추가되면 좋을 것 같아요. (팀노션에도 적혀있겠지만, 노션엔 다른 내용들도 많아서 찾는데 시간이 좀 걸릴 수도)

만약 토글 사용하고 싶으면 여기 참고하면 좋아보여요

고생하셨어요~

Copy link
Collaborator

@hanueleee hanueleee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어우 검색 쿼리 너무 어렵네요😵
저도 딱히 개선할 방법이 떠오르지 않네요..

고생하셨습니다. 망고!!

@Go-Jaecheol
Copy link
Collaborator Author

쿼리 수정

AS-IS

SELECT p FROM Product p 
JOIN Product p2 ON p2.id = :lastId 
WHERE p.name LIKE CONCAT('%', :name, '%') 
AND ((p2.name LIKE CONCAT(:name, '%') AND p.id < :lastId) OR (p.name NOT LIKE CONCAT(:name, '%') AND p.id < :lastId)) 
ORDER BY (CASE WHEN p.name LIKE CONCAT(:name, '%') THEN 1 ELSE 2 END), 
p.id DESC;

TO-BE

SELECT * FROM product p
JOIN product last ON last.id = :lastId
WHERE p.name LIKE CONCAT('%', '1', '%')
AND (last.name LIKE CONCAT('1', '%')
    AND ((p.name LIKE CONCAT('1', '%') AND p.id < :lastId) OR (p.name NOT LIKE CONCAT('1', '%')))
    OR (p.name NOT LIKE CONCAT('1', '%') AND p.id < :lastId))
ORDER BY (CASE WHEN p.name LIKE CONCAT('1', '%') THEN 1 ELSE 2 END), p.id DESC;

# 위와 같은 쿼리임 (CASE로 나타낸 경우)
SELECT * FROM product p
JOIN product last ON last.id = :lastId
WHERE p.name LIKE CONCAT('%', '1', '%')
AND (CASE WHEN last.name LIKE CONCAT('1', '%')
    THEN (p.name LIKE CONCAT('1', '%') AND p.id < :lastId) OR (p.name NOT LIKE CONCAT('1', '%'))
    ELSE p.name NOT LIKE CONCAT('1', '%') AND p.id < :lastId END)
ORDER BY (CASE WHEN p.name LIKE CONCAT('1', '%') THEN 1 ELSE 2 END), p.id DESC;

기존 쿼리 제대로 동작 X
=> 의도에 맞게 제대로 동작하도록 쿼리 수정


페이징 비교 (총 468,565 개 중 끝에서 10개 조회)

Offset

SELECT * FROM product p
WHERE p.name LIKE CONCAT('%', '1', '%')
ORDER BY
(CASE WHEN p.name LIKE CONCAT('1', '%') THEN 1 ELSE 2 END),
p.id DESC
LIMIT 468555, 10;

Cursor

SELECT * FROM product p
JOIN product last ON last.id = :lastId
WHERE p.name LIKE CONCAT('%', '1', '%')
AND (last.name LIKE CONCAT('1', '%')
    AND ((p.name LIKE CONCAT('1', '%') AND p.id &lt; :lastId) OR (p.name NOT LIKE CONCAT('1', '%')))
    OR (p.name NOT LIKE CONCAT('1', '%') AND p.id &lt; :lastId))
ORDER BY (CASE WHEN p.name LIKE CONCAT('1', '%') THEN 1 ELSE 2 END), p.id DESC;
Offset Cursor
5s 149ms 3s 720ms
4s 993ms 3s 583ms
4s 766ms 3s 643ms
4s 883ms 3s 662ms
5s 186ms 3s 667ms
4s 884ms 3s 688ms
4s 812ms 3s 886ms
5s 84ms 3s 784ms
4s 893ms 3s 712ms
5s 80ms 3s 761ms

=> Cursor 방식이 Offset 방식보다 성능 면에서 더 좋음

@Go-Jaecheol Go-Jaecheol merged commit bae8feb into develop Dec 29, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants