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

Fix Malfunction - only call writeParam() methods inside writeParentheses() method #524

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

waahhh
Copy link
Contributor

@waahhh waahhh commented Nov 13, 2023

Motivation

  • String functions 추가 지원을 위한 작업 중, 예상과 다른 동작을 보이는 부분이 있어서 확인해보니,
    3.0.0 부터 지원했던 비슷한 류의 upper(), lower() 등의 함수들도 이전과 다르게 아래와 같은 예기치 않은 동작을 보이는것을 확인했습니다.
  • 기존에 upper(:param1) 처럼 렌더링 되던게 upper:param1 처럼 괄호가 누락된 채로 렌더링 되는 이슈.
  • writer에 의해 writeParentheses() 내부에서 writeParam()만 호출하는 경우에 괄호가 누락 됨.
  • 원인을 확인해보니 d2cbb650 커밋의 DefaultJpqlWriter.kt 변경 이후 부터 영향을 받은듯합니다.
  • develop 브랜치는 현재 3.1.0 준비중으로 보여, 3.0.2(hotfix)를 위해 main 브랜치에 PR 올립니다. 혹시나 문제가 없다면 develop에도 반영해 주시면 감사하겠습니다.

Modifications

  • writeParam() 메서드 호출시에도 write 메서드와 마찬가지로, nodes.add(String()) 추가.
  • 이슈와 관련된 writeParentheses() 내부에서 writeParam()만 호출하는 경우에 대한 테스트 추가.

Commit Convention Rule

Commit type Description
feat New Feature
fix Fix bug
docs Documentation only changed
ci Change CI configuration
refactor Not a bug fix or add feature, just refactoring code
test Add Test case or fix wrong test case
style Only change the code style(ex. white-space, formatting)
chore It refers to minor tasks such as library version upgrade, typo correction, etc.
  • If you want to add some more commit type please describe it on the Pull Request

Result

  • 기존의 upper(), lower() 등의 함수나 앞으로 추가되는 기능 구현시, writer에 의해 writeParentheses() 내부에서 writeParam()만 호출하는 경우에도 정상적으로 괄호 렌더링이 되어 정상 작동.

Closes

  • #{issue number} (If this PR resolves an issue.)

@waahhh waahhh force-pushed the fix/malfunction-write-param-method branch from aeb2dc2 to 48ee920 Compare November 13, 2023 12:19
Copy link
Member

@shouwn shouwn left a comment

Choose a reason for hiding this comment

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

아... param을 놓쳤네요. 😢 찾아주셔서 감사합니다! 큰 버그였네요...

@shouwn shouwn merged commit ff14081 into line:main Nov 13, 2023
2 of 3 checks passed
@waahhh waahhh deleted the fix/malfunction-write-param-method branch November 22, 2023 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants