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

Documentation of why Kotlin JDSL returns a nullable list in the Spring Support module. #685

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

progress0407
Copy link
Contributor

Motivation

Modifications

  • wrote why-kotlin-jdsl-returns-nullable-list-in-the-spring-support-module.md document.

Result

  • As described in the topic above(Modifications)

Closes

@progress0407
Copy link
Contributor Author

hello!
I think the original date I was going to do the work and post the PR was 3.27~28th...
I apologize for posting it now, and thank you for your help.

@shouwn
Copy link
Member

shouwn commented Apr 12, 2024

Thanks for your help! I think the confirmation of this PR will be delayed for a week or two.

@progress0407
Copy link
Contributor Author

Thank you. I'm sure you've been busy deploying version 3.4.0 otherwise. I understand. 🥰


| Item | Nullable or not | Reason |
|----------------|-----------------|------------------------------------------------------------------------------------------------------|
| DTO Projection | X | Calling the constructor for all ROWs results in the object being created, which does not allow nulls |
Copy link
Collaborator

@cj848 cj848 Apr 17, 2024

Choose a reason for hiding this comment

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

This is inaccurate information. In case of DTO Projection, Nullable is also allowed. For example

class Entity {
   @Id
   var id: Long? = null

   @Column(nullable = true)
   var name: String? = null
}

class DTO {
  var name: String?
}

When there is an entity like the above, null may occur at the time of the query below.

select new DTO(name) from Entity;

Copy link
Member

Choose a reason for hiding this comment

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

This is correct information. When using the new keyword in JPQL, the constructor is called for every row to create the object. That's why there can't be a nullable for DTO Projection only.

image
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you mean returning a DTO object itself, that is correct.
I meant that the fields in the object are nullable, but that seems to be incorrect information. Thanks for the good point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see you got it, I'll add it to the documentation!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's nice to be able to check it with test code.

image

@progress0407
Copy link
Contributor Author

Thank you so much for the review! We will reflect it sequentially after checking :)


| Item | Nullable or not | Reason |
|----------------|-----------------|------------------------------------------------------------------------------------------------------|
| DTO Projection | X | Calling the constructor for all ROWs results in the object being created, which does not allow nulls |
Copy link
Member

Choose a reason for hiding this comment

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

This is correct information. When using the new keyword in JPQL, the constructor is called for every row to create the object. That's why there can't be a nullable for DTO Projection only.

image
image

@@ -0,0 +1,39 @@
# 왜 Kotlin JDSL은 nullable한 반환 타입을 허용하나요?

Kotlin JDSL을 사용하면 별도의 메타데이터 모델 생성이나 문자열 문법 오류 없이 쿼리를 작성할 수 있도록 도와주어 개발자가 보다 효율적으로 개발할 수 있게 도와줍니다.
Copy link
Member

Choose a reason for hiding this comment

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

메타데이터 모델 생성은 nullable 주제와는 연관이 적어 보입니다.

'Type Safe하게 쿼리를 작성할 수 있게 도와주지만, 왜 리스트 반환 시에는 non-null한 것도 nullable하게 반환하는지 궁금하실 수 있다' 는 스토리로 글을 풀어가면 좋을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음 확실히 저번에 올린 PR(메타 모델)과는 성격이 다른 것 같아요. 수정하겠습니다!

@progress0407
Copy link
Contributor Author

Thank you for your review!
I reflected!

@progress0407 progress0407 requested review from cj848 and shouwn April 22, 2024 14:34

| 항목 | null 여부 | 이유 |
|----------------|---------|-------------------------------------------------------------------------------------------------------|
| DTO Projection | X | 모든 ROW에 대한 생성자 호출하기 때문에 객체가 생성되어 null을 허용하는 결과가 나오지 않음<br/>DTO의 필드가 null일 수는 있으나 DTO 객체가 null일 수는 없음 |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
| DTO Projection | X | 모든 ROW에 대한 생성자 호출하기 때문에 객체가 생성되어 null을 허용하는 결과가 나오지 않음<br/>DTO의 필드가 null일 수는 있으나 DTO 객체가 null일 수는 없음 |
| DTO Projection | X | 모든 ROW에 대한 생성자를 호출하기 때문에 객체가 생성되어 null을 허용하는 결과가 나오지 않음<br/>DTO의 필드가 null일 수는 있으나 DTO 객체가 null일 수는 없음 |

i think this is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the detailed review! I will edit it. ☺️

@progress0407 progress0407 requested a review from cj848 April 25, 2024 15:49
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.

I apologize for the delay in reviewing, I've been busy with other things.

Thanks for your help. I'll make some changes to the documentation later!

@shouwn shouwn merged commit 08d5b2a into line:develop Apr 29, 2024
4 checks passed
@progress0407
Copy link
Contributor Author

Oh, no, I know it's not easy to take the time to review outside of work hours. I always appreciate your reviews!

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.

3 participants