line / kotlin-jdsl

Kotlin library that makes it easy to build and execute queries without generated metamodel
https://kotlin-jdsl.gitbook.io/docs/
Apache License 2.0
715 stars 89 forks source link

jdsl selectFrom function #776

Open kihwankim opened 1 month ago

kihwankim commented 1 month ago

Backgrounds

Requirements

Example

    authorRepository.findAll {
          selectFrom(
              entity(Author::class),
              leftJoin(BookAuthor::class).on(path(Author::authorId).equal(path(BookAuthor::authorId))),
          ).where(
              path(BookAuthor::authorId).isNull(),
          ).orderBy(
              path(Author::authorId).asc(),
          )
      }
shouwn commented 1 month ago

Hi @kihwankim. Thanks for the suggestion!

I'm hesitant to add the selectFrom function to Jdsl because it might lead to misunderstandings about the purpose of the library. Jdsl is designed to closely reflect the target language, and adding custom expressions might make users think we're abstracting that language too much. Ideally, users should have the freedom to create their own Custom DSLs that fit their personal style.

Also, Jdsl and QueryDSL have different approaches. In QueryDSL, you can chain methods after selectFrom to add joins, but this isn't possible in Jdsl. Using variable arguments in selectFrom seems to enforce a specific way of using it, which can be confusing. For example, it's unclear whether joins are included in what selectFrom returns just by looking at the method. This confusion arises because selectFrom is a unique construct of the library, not a standard method. This could lead us to respond to various user requests, such as supporting selectFrom().join() or allowing multiple where clauses, which can complicate decision-making.

However, if many users want selectFrom, I'm open to considering it, provided we have a clear vision for the future. Otherwise, the DSL might become too complex to enhance later on. I think it would be worth discussing further if this issue gets about 20 πŸ‘, which is about 3% of our current stars.

kihwankim commented 1 month ago

@shouwn Thank you! (It's a personal opinion, so if users need the selectFrom function, please reply then) I'm leaving a comment to appeal my opinion once again in case this issue gets a lot of πŸ‘ and becomes a subject of consideration. Each individual may have a different style, but when calling the 'select' function about entity, the from function often adds the same entity. And we developed a lot by adding fetchJoin if necessary. since the select from function is repeated, people created and used a class that inherited the Jpql open class and add a selectFrom function or define it through an extended function to simplify logic. In the process of repeated work, I thought it would be better if the Kotlin-JDSL Lib could provide that function, so I uploaded issue and PR

--- korean

κ°μ‚¬ν•©λ‹ˆλ‹€! (개인적인 μ˜κ²¬μ΄λ‹ˆ λ§Žμ€ μ‚¬μš©μžλ“€μ˜ selectFrom ν•¨μˆ˜λ₯Ό ν•„μš”λ‘œ ν•˜λ©΄ κ·Έ λ•Œ λ‹΅κΈ€ λ‚¨κ²¨μ£Όμ„Έμš”!) λ‹€μ‹œ 이 issueκ°€ πŸ‘ λ₯Ό 많이 λ°›μ•„μ„œ, κ³ λ € λŒ€μƒμ΄ 될 경우λ₯Ό μœ„ν•΄μ„œ μ €μ˜ μ˜κ²¬μ„ ν•œλ²ˆ μ–΄ν•„ ν•˜κ³ μž λŒ“κΈ€μ„ λ‚¨κΉλ‹ˆλ‹€. 각 개인 λ§ˆλ‹€ μŠ€νƒ€μΌ 차이가 μžˆκ² μ§€λ§Œ, entity λ‹¨μœ„λ‘œ select ν•  경우 from도 λ™μΌν•œ entityλ₯Ό λ„£λŠ”κ²½μš°κ°€ λ§ŽμŠ΅λ‹ˆλ‹€. 그리고 ν•„μš”ν•  경우 fetchJoin을 μΆ”κ°€ν•˜λŠ” λ°©μ‹μœΌλ‘œ 많이 κ°œλ°œν•˜μ˜€μŠ΅λ‹ˆλ‹€. μœ„μ™€ 같은 μ½”λ“œκ°€ λ°˜λ³΅λ˜λ‹€ λ³΄λ‹ˆ λ‘œμ§μ„ κ°„μ†Œν™” ν•˜κΈ° μœ„ν•΄μ„œ Jpql open class λ₯Ό μƒμ†λ°›μ•„μ„œ selectFrom ν•¨μˆ˜λ₯Ό μΆ”κ°€ν•˜κ±°λ‚˜ , ν™•μž₯ν•¨μˆ˜λ₯Ό ν†΅ν•΄μ„œ μ •μ˜ν•΄μ„œ μ‚¬μš©ν•˜λŠ” κ²½μš°κ°€ 많이 λ°œμƒν•˜μ˜€μŠ΅λ‹ˆλ‹€. 반볡된 μž‘μ—…κ³Όμ •μ—μ„œ jdsl lib λ‹¨μ—μ„œ ν•΄λ‹Ή κΈ°λŠ₯을 μ œκ³΅ν•΄μ£Όλ©΄ μ’‹κ² λ‹€λŠ” 생각이 λ§Žμ΄λ“€μ–΄μ„œ issue 와 PR을 올리게 λ˜μ—ˆμŠ΅λ‹ˆλ‹€

esperar commented 3 weeks ago

This is a personal opinion!

I think it’s a reasonable decision to implement a feature once an issue receives 20 πŸ‘ reactions, especially from the perspective of managing open source projects.

However, I believe that even though Kotlin JDSL is not actively maintained as an open source project, it is used directly by a relatively larger number of people.

I’m not sure if this matter has been brought up in the Discord community, but personally, I feel that information dissemination is somewhat lacking.

I think it would be good to spread the word more broadly about the requirement for 20 πŸ‘ reactions to check whether the feature will be implemented or not. Thank you!

shouwn commented 3 weeks ago

@esperar As you said, it would be good to make an announcement on Discord or something, since this is a survey. Thank you for the good feedback.

I'll try to make an announcement on Discord sometime next week.

shouwn commented 2 weeks ago

@esperar Sorry it took so long. I left a post on Discord to ask for feedback on this issue.